netdev - Re: [PATCH] net af_key: Fix RCU splat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1440113810.6610.70.camel@edumazet-glaptop2.roam.corp.google.com>
Date:	Thu, 20 Aug 2015 16:36:50 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Ahern <dsa@...ulusnetworks.com>
Cc:	steffen.klassert@...unet.com, netdev@...r.kernel.org,
	Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH] net af_key: Fix RCU splat

On Thu, 2015-08-20 at 15:57 -0700, David Ahern wrote:
> On 8/20/15 9:51 AM, Eric Dumazet wrote:
> > On Thu, 2015-08-20 at 08:51 -0700, David Ahern wrote:
> >> Hit the following splat testing VRF change for ipsec:

...

> >>
> >> diff --git a/net/key/af_key.c b/net/key/af_key.c
> >> index b397f0aa9005..73527e7dd247 100644
> >> --- a/net/key/af_key.c
> >> +++ b/net/key/af_key.c
> >> @@ -1670,7 +1670,7 @@ static int pfkey_register(struct sock *sk, struct sk_buff *skb, const struct sad
> >>   		return -ENOBUFS;
> >>   	}
> >>
> >> -	pfkey_broadcast(supp_skb, GFP_KERNEL, BROADCAST_REGISTERED, sk, sock_net(sk));
> >> +	pfkey_broadcast(supp_skb, GFP_ATOMIC, BROADCAST_REGISTERED, sk, sock_net(sk));
> >>
> >>   	return 0;
> >>   }
> >
> > I would rather remove the useless rcu locking from pfkey_broadcast() if
> > a mutex properly protects the thing.
> 
> rcu_read_lock was added by Stephen with 7f6b9dbd5afbd. It does not 
> appear the net->xfrm.xfrm_cfg_mutex mutex added by 283bc9f35bbbc 
> properly covers the locking. ie., the rcu_read_lock is needed.

Then please cook a complete patch, and add a 'Fixes: ...' tag


# git grep -n pfkey_broadcast|grep GFP_KERNEL
net/key/af_key.c:336:	pfkey_broadcast(skb, GFP_KERNEL, BROADCAST_ONE, sk, sock_net(sk));
net/key/af_key.c:1368:	pfkey_broadcast(resp_skb, GFP_KERNEL, BROADCAST_ONE, sk, net);
net/key/af_key.c:1673:	pfkey_broadcast(supp_skb, GFP_KERNEL, BROADCAST_REGISTERED, sk, sock_net(sk));
net/key/af_key.c:1850:	pfkey_broadcast(skb, GFP_KERNEL, BROADCAST_ALL, NULL, sock_net(sk));
net/key/af_key.c:2773:	pfkey_broadcast(skb_clone(skb, GFP_KERNEL), GFP_KERNEL,

Presumably we should remove "gfp_t allocation" pfkey_broadcast() argument
if we need to use GFP_ATOMIC in all cases.



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html