As long as one skb has sock_rfree has its destructor, the socket
attached to
this skb can not be released. There is no race here.
Note that skb_clone() does not propagate the destructor.
The issue here is that in the rcu lookup, we can find a socket that
has been
dismantled, with a 0 refcount.
We must not use sock_hold() in this case, since we are not sure the
socket refcount is not already 0.
pfkey_broadcast() and pfkey_broadcast_one() violate basic RCU rules.
When in a RCU lookup, one want to increment an object refcount, it
needs
to be extra-careful, as I did in my proposal.
Note that the race could be automatically detected with
CONFIG_REFCOUNT_FULL=y
Bug was added in commit 7f6b9dbd5afb ("af_key: locking change")
Hi Eric,
I tried your refcount idea below, but it still results in the same
crash.
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -201,7 +201,9 @@ static int pfkey_broadcast_one(struct sk_buff
*skb, struct sk_buff **skb2,
{
int err = -ENOBUFS;
- sock_hold(sk);
+ if (!refcount_inc_not_zero(&sk->sk_refcnt))
+ return -ENOENT;
+
if (*skb2 == NULL) {
if (refcount_read(&skb->users) != 1) {
*skb2 = skb_clone(skb, allocation);
I also tried reverting 7f6b9dbd5afb ("af_key: locking change") and
running the
test there and I still see the crash, so it doesn't seem to be an RCU
specific
issue.
Is there anything else that could be causing this?
