Re: assertion (!atomic_read(&sk->sk_rmem_alloc)) failed at net/netlink/af_netlink.c (122)

[Tommy Christensen <tommy.christensen@tpack.net>]

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

Herbert Xu wrote:
> I think I understand your patch now.  What's happening is that
> 
> 1) The skb is sent to socket 1.
> 2) Someone does a recvmsg on socket 1 and drops the ref on the skb.
>    Note that the rmalloc is not returned at this point since the
>    skb is still referenced.
> 3) The same skb is now sent to socket 2.

Ahh, even I get the point now.
I actually thought this was caused by another race. More on that later.

> I agree with your solution except that we should still do the skb_get
> if we can.  Here is my version where we only do the skb_get at the
> start.

What about an alternative fix, that avoids even more cloning (where
possible)? This resurrects the skb_orphan call that was moved out, last
time we had 'shared-skb troubles'. It is practically a no-op in the
common case, but still prevents the possible race with recvmsg.
(And I have a weakness for one-line-fixes). :-)


Signed-off-by: Tommy S. Christensen <tommy.christensen@tpack.net>

[-- Attachment #2: netlink-1.patch --]
[-- Type: text/plain, Size: 506 bytes --]

diff -ru linux-2.6.12-rc4/net/netlink/af_netlink.c linux-2.6.12-work/net/netlink/af_netlink.c
--- linux-2.6.12-rc4/net/netlink/af_netlink.c	2005-05-11 11:10:20.000000000 +0200
+++ linux-2.6.12-work/net/netlink/af_netlink.c	2005-05-12 00:08:33.634344658 +0200
@@ -697,6 +697,7 @@
 
 	if (atomic_read(&sk->sk_rmem_alloc) <= sk->sk_rcvbuf &&
 	    !test_bit(0, &nlk->state)) {
+		skb_orphan(skb);
 		skb_set_owner_r(skb, sk);
 		skb_queue_tail(&sk->sk_receive_queue, skb);
 		sk->sk_data_ready(sk, skb->len);