From db61ecc3352d72513c1b07805bd6f760e30c001b Mon Sep 17 00:00:00 2001 From: "Tommy S. Christensen" Date: Thu, 19 May 2005 12:46:59 -0700 Subject: [NETLINK]: Fix race with recvmsg(). This bug causes: assertion (!atomic_read(&sk->sk_rmem_alloc)) failed at net/netlink/af_netlink.c (122) 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. This version of the fix 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. Signed-off-by: Tommy S. Christensen Acked-by: Herbert Xu Signed-off-by: David S. Miller --- net/netlink/af_netlink.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/netlink') diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 733bf52cef3e..b40c9a976969 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -697,6 +697,7 @@ static __inline__ int netlink_broadcast_deliver(struct sock *sk, struct sk_buff 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); -- cgit v1.2.2 From 68acc024ea7391e03c2c695ba0b9fb31baa974bf Mon Sep 17 00:00:00 2001 From: "Tommy S. Christensen" Date: Thu, 19 May 2005 13:06:35 -0700 Subject: [NETLINK]: Move broadcast skb_orphan to the skb_get path. Cloned packets don't need the orphan call. Signed-off-by: Tommy S. Christensen Acked-by: Herbert Xu Signed-off-by: David S. Miller --- net/netlink/af_netlink.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'net/netlink') diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index b40c9a976969..4b91f4b84cb7 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -697,7 +697,6 @@ static __inline__ int netlink_broadcast_deliver(struct sock *sk, struct sk_buff 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); @@ -736,11 +735,15 @@ static inline int do_one_broadcast(struct sock *sk, sock_hold(sk); if (p->skb2 == NULL) { - if (atomic_read(&p->skb->users) != 1) { + if (skb_shared(p->skb)) { p->skb2 = skb_clone(p->skb, p->allocation); } else { - p->skb2 = p->skb; - atomic_inc(&p->skb->users); + p->skb2 = skb_get(p->skb); + /* + * skb ownership may have been set when + * delivered to a previous socket. + */ + skb_orphan(p->skb2); } } if (p->skb2 == NULL) { -- cgit v1.2.2 From aa1c6a6f7f0518b42994d02756a41cbfdcac1916 Mon Sep 17 00:00:00 2001 From: "Tommy S. Christensen" Date: Thu, 19 May 2005 13:07:32 -0700 Subject: [NETLINK]: Defer socket destruction a bit In netlink_broadcast() we're sending shared skb's to netlink listeners when possible (saves some copying). This is OK, since we hold the only other reference to the skb. However, this implies that we must drop our reference on the skb, before allowing a receiving socket to disappear. Otherwise, the socket buffer accounting is disrupted. Signed-off-by: Tommy S. Christensen Acked-by: Herbert Xu Signed-off-by: David S. Miller --- net/netlink/af_netlink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/netlink') diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 4b91f4b84cb7..e41ce458c2a9 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -789,11 +789,12 @@ int netlink_broadcast(struct sock *ssk, struct sk_buff *skb, u32 pid, sk_for_each_bound(sk, node, &nl_table[ssk->sk_protocol].mc_list) do_one_broadcast(sk, &info); + kfree_skb(skb); + netlink_unlock_table(); if (info.skb2) kfree_skb(info.skb2); - kfree_skb(skb); if (info.delivered) { if (info.congested && (allocation & __GFP_WAIT)) -- cgit v1.2.2