Hi Dave:
On Wed, Mar 30, 2005 at 05:02:36PM -0800, David S. Miller wrote:
> On Wed, 30 Mar 2005 18:26:40 +1000
> Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
>
> > The solution is to hold a ref count on the socket before we drop
> > the cb lock.
>
> Applied, thanks Herbert.
Unfortunately my patch only closed half the race. There is still
a chunk of code between netlink_dump_start and netlink_dump that runs
outside the cb lock which isn't protected by an sk reference.
Here is a better patch which protects the entire netlink_dump function
with a sk reference.
The other call to netlink_dump by recvmsg is safe as the open file
descriptor already holds a reference. As such the final sock_put
in netlink_dump can be turned into a __sock_put since there is at
least one reference held by the caller.
Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
===== net/netlink/af_netlink.c 1.73 vs edited =====
--- 1.73/net/netlink/af_netlink.c 2005-03-31 10:58:04 +10:00
+++ edited/net/netlink/af_netlink.c 2005-04-01 09:06:20 +10:00
@@ -1080,11 +1080,9 @@
len = cb->dump(skb, cb);
if (len > 0) {
- sock_hold(sk);
spin_unlock(&nlk->cb_lock);
skb_queue_tail(&sk->sk_receive_queue, skb);
sk->sk_data_ready(sk, len);
- sock_put(sk);
return 0;
}
@@ -1099,7 +1097,7 @@
spin_unlock(&nlk->cb_lock);
netlink_destroy_callback(cb);
- sock_put(sk);
+ __sock_put(sk);
return 0;
}
@@ -1138,9 +1136,11 @@
return -EBUSY;
}
nlk->cb = cb;
+ sock_hold(sk);
spin_unlock(&nlk->cb_lock);
netlink_dump(sk);
+ sock_put(sk);
return 0;
}
