public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] netlink: fix races after skb queueing
@ 2012-04-06  8:17 Eric Dumazet
  2012-04-06  8:21 ` David Miller
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2012-04-06  8:17 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

As soon as an skb is queued into socket receive_queue, another thread
can consume it, so we are not allowed to reference skb anymore, or risk
use after free.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/netlink/af_netlink.c |   24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 32bb753..faa48f7 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -829,12 +829,19 @@ int netlink_attachskb(struct sock *sk, struct sk_buff *skb,
 	return 0;
 }
 
-int netlink_sendskb(struct sock *sk, struct sk_buff *skb)
+static int __netlink_sendskb(struct sock *sk, struct sk_buff *skb)
 {
 	int len = skb->len;
 
 	skb_queue_tail(&sk->sk_receive_queue, skb);
 	sk->sk_data_ready(sk, len);
+	return len;
+}
+
+int netlink_sendskb(struct sock *sk, struct sk_buff *skb)
+{
+	int len = __netlink_sendskb(sk, skb);
+
 	sock_put(sk);
 	return len;
 }
@@ -957,8 +964,7 @@ static int netlink_broadcast_deliver(struct sock *sk, struct sk_buff *skb)
 	if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf &&
 	    !test_bit(0, &nlk->state)) {
 		skb_set_owner_r(skb, sk);
-		skb_queue_tail(&sk->sk_receive_queue, skb);
-		sk->sk_data_ready(sk, skb->len);
+		__netlink_sendskb(sk, skb);
 		return atomic_read(&sk->sk_rmem_alloc) > (sk->sk_rcvbuf >> 1);
 	}
 	return -1;
@@ -1698,10 +1704,8 @@ static int netlink_dump(struct sock *sk)
 
 		if (sk_filter(sk, skb))
 			kfree_skb(skb);
-		else {
-			skb_queue_tail(&sk->sk_receive_queue, skb);
-			sk->sk_data_ready(sk, skb->len);
-		}
+		else
+			__netlink_sendskb(sk, skb);
 		return 0;
 	}
 
@@ -1715,10 +1719,8 @@ static int netlink_dump(struct sock *sk)
 
 	if (sk_filter(sk, skb))
 		kfree_skb(skb);
-	else {
-		skb_queue_tail(&sk->sk_receive_queue, skb);
-		sk->sk_data_ready(sk, skb->len);
-	}
+	else
+		__netlink_sendskb(sk, skb);
 
 	if (cb->done)
 		cb->done(cb);

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] netlink: fix races after skb queueing
  2012-04-06  8:17 [PATCH] netlink: fix races after skb queueing Eric Dumazet
@ 2012-04-06  8:21 ` David Miller
  2012-04-06  8:34   ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2012-04-06  8:21 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 06 Apr 2012 10:17:46 +0200

> As soon as an skb is queued into socket receive_queue, another thread
> can consume it, so we are not allowed to reference skb anymore, or risk
> use after free.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Good catch Eric.

Applied and queued up for -stable.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] netlink: fix races after skb queueing
  2012-04-06  8:21 ` David Miller
@ 2012-04-06  8:34   ` Eric Dumazet
  0 siblings, 0 replies; 3+ messages in thread
From: Eric Dumazet @ 2012-04-06  8:34 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

On Fri, 2012-04-06 at 04:21 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 06 Apr 2012 10:17:46 +0200
> 
> > As soon as an skb is queued into socket receive_queue, another thread
> > can consume it, so we are not allowed to reference skb anymore, or risk
> > use after free.
> > 
> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> 
> Good catch Eric.
> 
> Applied and queued up for -stable.

Same problem with sock_queue_err_skb(), I'll send a patch for it

Thanks

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-04-06  8:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-06  8:17 [PATCH] netlink: fix races after skb queueing Eric Dumazet
2012-04-06  8:21 ` David Miller
2012-04-06  8:34   ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox