| Message ID | 1358952191-3374-1-git-send-email-xiyou.wangcong@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Jan 23, 2013 at 10:43:11PM +0800, Cong Wang wrote: > From: Cong Wang <xiyou.wangcong@gmail.com> > > Yannick reported [1] we probably forgot to account ->sk_rmem_alloc > before moving the skb to other->sk_receive_queue. I believe > he is right. So, just call skb_set_owner_r() before every time > we queuing skb into other->sk_receive_queue. I think Eric's comment on a possible DOS is correct. Having a quick look, it seems unix_recvq_full needs to check ->sk_rmem_alloc at a minimum. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2013-01-23 at 22:43 +0800, Cong Wang wrote: > From: Cong Wang <xiyou.wangcong@gmail.com> > > Yannick reported [1] we probably forgot to account ->sk_rmem_alloc > before moving the skb to other->sk_receive_queue. I believe > he is right. So, just call skb_set_owner_r() before every time > we queuing skb into other->sk_receive_queue. > > 1. http://marc.info/?l=linux-netdev&m=135882012924930&w=2 > > Reported-by: Yannick Koehler <yannick@koehler.name> > Cc: Yannick Koehler <yannick@koehler.name> > Cc: Eric Dumazet <eric.dumazet@gmail.com> > Cc: David S. Miller <davem@davemloft.net> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> > > --- > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c > index 5b5c876..7e9dba3 100644 > --- a/net/unix/af_unix.c > +++ b/net/unix/af_unix.c > @@ -1205,6 +1205,7 @@ restart: > > unix_state_unlock(sk); > > + skb_set_owner_r(skb, other); > /* take ten and and send info to listening sock */ > spin_lock(&other->sk_receive_queue.lock); > __skb_queue_tail(&other->sk_receive_queue, skb); > @@ -1579,6 +1580,7 @@ restart: > if (sock_flag(other, SOCK_RCVTSTAMP)) > __net_timestamp(skb); > maybe_add_creds(skb, sock, other); > + skb_set_owner_r(skb, other); > skb_queue_tail(&other->sk_receive_queue, skb); > if (max_level > unix_sk(other)->recursion_level) > unix_sk(other)->recursion_level = max_level; > @@ -1694,6 +1696,7 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock, > goto pipe_err_free; > > maybe_add_creds(skb, sock, other); > + skb_set_owner_r(skb, other); > skb_queue_tail(&other->sk_receive_queue, skb); > if (max_level > unix_sk(other)->recursion_level) > unix_sk(other)->recursion_level = max_level; I Nack this patch, for reasons already given. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 5b5c876..7e9dba3 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -1205,6 +1205,7 @@ restart: unix_state_unlock(sk); + skb_set_owner_r(skb, other); /* take ten and and send info to listening sock */ spin_lock(&other->sk_receive_queue.lock); __skb_queue_tail(&other->sk_receive_queue, skb); @@ -1579,6 +1580,7 @@ restart: if (sock_flag(other, SOCK_RCVTSTAMP)) __net_timestamp(skb); maybe_add_creds(skb, sock, other); + skb_set_owner_r(skb, other); skb_queue_tail(&other->sk_receive_queue, skb); if (max_level > unix_sk(other)->recursion_level) unix_sk(other)->recursion_level = max_level; @@ -1694,6 +1696,7 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock, goto pipe_err_free; maybe_add_creds(skb, sock, other); + skb_set_owner_r(skb, other); skb_queue_tail(&other->sk_receive_queue, skb); if (max_level > unix_sk(other)->recursion_level) unix_sk(other)->recursion_level = max_level;