[PATCH] unix_diag: Fix incoming connections nla length

[PATCH] unix_diag: Fix incoming connections nla length

[Pavel Emelyanov]

The NLA_PUT macro should accept the actual attribute length, not
the amount of elements in array :(

Signed-off-by: Pavel Emelyanov <xemul@parallels.com>

---
 net/unix/diag.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/unix/diag.c b/net/unix/diag.c
index 91d5782..39e44c9 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -72,7 +72,8 @@ static int sk_diag_dump_icons(struct sock *sk, struct sk_buff *nlskb)
 
 	if (sk->sk_state == TCP_LISTEN) {
 		spin_lock(&sk->sk_receive_queue.lock);
-		buf = UNIX_DIAG_PUT(nlskb, UNIX_DIAG_ICONS, sk->sk_receive_queue.qlen);
+		buf = UNIX_DIAG_PUT(nlskb, UNIX_DIAG_ICONS,
+				sk->sk_receive_queue.qlen * sizeof(u32));
 		i = 0;
 		skb_queue_walk(&sk->sk_receive_queue, skb) {
 			struct sock *req, *peer;
-- 
1.5.5.6

Re: [PATCH] unix_diag: Fix incoming connections nla length

[David Miller]

From: Pavel Emelyanov <xemul@parallels.com>
Date: Sun, 25 Dec 2011 23:58:05 +0400

> The NLA_PUT macro should accept the actual attribute length, not
> the amount of elements in array :(
> 
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>

Applied, thanks.

Re: [PATCH] unix_diag: Fix incoming connections nla length

[Eric Dumazet]

Le dimanche 25 décembre 2011 à 23:58 +0400, Pavel Emelyanov a écrit :
> The NLA_PUT macro should accept the actual attribute length, not
> the amount of elements in array :(
> 
> Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
> 
> ---
>  net/unix/diag.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/unix/diag.c b/net/unix/diag.c
> index 91d5782..39e44c9 100644
> --- a/net/unix/diag.c
> +++ b/net/unix/diag.c
> @@ -72,7 +72,8 @@ static int sk_diag_dump_icons(struct sock *sk, struct sk_buff *nlskb)
>  
>  	if (sk->sk_state == TCP_LISTEN) {
>  		spin_lock(&sk->sk_receive_queue.lock);
> -		buf = UNIX_DIAG_PUT(nlskb, UNIX_DIAG_ICONS, sk->sk_receive_queue.qlen);
> +		buf = UNIX_DIAG_PUT(nlskb, UNIX_DIAG_ICONS,
> +				sk->sk_receive_queue.qlen * sizeof(u32));
>  		i = 0;
>  		skb_queue_walk(&sk->sk_receive_queue, skb) {
>  			struct sock *req, *peer;

Hmm, I must say sk_diag_dump_icons() looks buggy, since it does :


if (peer)
	buf[i++] = sock_i_ino(peer);

So we probably leak kernel memory content to user for the (!peer) case,
since we did :

UNIX_DIAG_PUT(nlskb, UNIX_DIAG_ICONS,
		sk->sk_receive_queue.qlen * sizeof(u32));

Re: [PATCH] unix_diag: Fix incoming connections nla length

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 26 Dec 2011 20:36:12 +0100

> if (peer)
> 	buf[i++] = sock_i_ino(peer);
> 
> So we probably leak kernel memory content to user for the (!peer) case,
> since we did :
> 
> UNIX_DIAG_PUT(nlskb, UNIX_DIAG_ICONS,
> 		sk->sk_receive_queue.qlen * sizeof(u32));

I just commited the following fix for this, it probably takes less
effort to post a patch for this kind of bug than explain it don't
you think? :)

--------------------
unix: If we happen to find peer NULL when diag dumping, write zero.

Otherwise we leave uninitialized kernel memory in there.

Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/unix/diag.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/net/unix/diag.c b/net/unix/diag.c
index 39e44c9..c5bdbcb 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -86,8 +86,7 @@ static int sk_diag_dump_icons(struct sock *sk, struct sk_buff *nlskb)
 			 */
 			unix_state_lock_nested(req);
 			peer = unix_sk(req)->peer;
-			if (peer)
-				buf[i++] = sock_i_ino(peer);
+			buf[i++] = (peer ? sock_i_ino(peer) : 0);
 			unix_state_unlock(req);
 		}
 		spin_unlock(&sk->sk_receive_queue.lock);
-- 
1.7.7.4