* [Patch net] unix: correct sk_rmem_alloc accounting
@ 2013-01-23 14:43 Cong Wang
2013-01-23 14:55 ` Hannes Frederic Sowa
2013-01-23 15:03 ` Eric Dumazet
0 siblings, 2 replies; 3+ messages in thread
From: Cong Wang @ 2013-01-23 14:43 UTC (permalink / raw)
To: netdev; +Cc: Yannick Koehler, Eric Dumazet, David S. Miller, Cong Wang
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;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch net] unix: correct sk_rmem_alloc accounting
2013-01-23 14:43 [Patch net] unix: correct sk_rmem_alloc accounting Cong Wang
@ 2013-01-23 14:55 ` Hannes Frederic Sowa
2013-01-23 15:03 ` Eric Dumazet
1 sibling, 0 replies; 3+ messages in thread
From: Hannes Frederic Sowa @ 2013-01-23 14:55 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Yannick Koehler, Eric Dumazet, David S. Miller
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch net] unix: correct sk_rmem_alloc accounting
2013-01-23 14:43 [Patch net] unix: correct sk_rmem_alloc accounting Cong Wang
2013-01-23 14:55 ` Hannes Frederic Sowa
@ 2013-01-23 15:03 ` Eric Dumazet
1 sibling, 0 replies; 3+ messages in thread
From: Eric Dumazet @ 2013-01-23 15:03 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, Yannick Koehler, David S. Miller
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-01-23 15:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-23 14:43 [Patch net] unix: correct sk_rmem_alloc accounting Cong Wang
2013-01-23 14:55 ` Hannes Frederic Sowa
2013-01-23 15:03 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).