* [PATCH] net af_key: Fix RCU splat
@ 2015-08-20 15:51 David Ahern
2015-08-20 16:51 ` Eric Dumazet
2015-08-23 23:48 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: David Ahern @ 2015-08-20 15:51 UTC (permalink / raw)
To: steffen.klassert, netdev; +Cc: David Ahern
Hit the following splat testing VRF change for ipsec:
[ 113.475692] ===============================
[ 113.476194] [ INFO: suspicious RCU usage. ]
[ 113.476667] 4.2.0-rc6-1+deb7u2+clUNRELEASED #3.2.65-1+deb7u2+clUNRELEASED Not tainted
[ 113.477545] -------------------------------
[ 113.478013] /work/monster-14/dsa/kernel.git/include/linux/rcupdate.h:568 Illegal context switch in RCU read-side critical section!
[ 113.479288]
[ 113.479288] other info that might help us debug this:
[ 113.479288]
[ 113.480207]
[ 113.480207] rcu_scheduler_active = 1, debug_locks = 1
[ 113.480931] 2 locks held by setkey/6829:
[ 113.481371] #0: (&net->xfrm.xfrm_cfg_mutex){+.+.+.}, at: [<ffffffff814e9887>] pfkey_sendmsg+0xfb/0x213
[ 113.482509] #1: (rcu_read_lock){......}, at: [<ffffffff814e767f>] rcu_read_lock+0x0/0x6e
[ 113.483509]
[ 113.483509] stack backtrace:
[ 113.484041] CPU: 0 PID: 6829 Comm: setkey Not tainted 4.2.0-rc6-1+deb7u2+clUNRELEASED #3.2.65-1+deb7u2+clUNRELEASED
[ 113.485422] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org 04/01/2014
[ 113.486845] 0000000000000001 ffff88001d4c7a98 ffffffff81518af2 ffffffff81086962
[ 113.487732] ffff88001d538480 ffff88001d4c7ac8 ffffffff8107ae75 ffffffff8180a154
[ 113.488628] 0000000000000b30 0000000000000000 00000000000000d0 ffff88001d4c7ad8
[ 113.489525] Call Trace:
[ 113.489813] [<ffffffff81518af2>] dump_stack+0x4c/0x65
[ 113.490389] [<ffffffff81086962>] ? console_unlock+0x3d6/0x405
[ 113.491039] [<ffffffff8107ae75>] lockdep_rcu_suspicious+0xfa/0x103
[ 113.491735] [<ffffffff81064032>] rcu_preempt_sleep_check+0x45/0x47
[ 113.492442] [<ffffffff8106404d>] ___might_sleep+0x19/0x1c8
[ 113.493077] [<ffffffff81064268>] __might_sleep+0x6c/0x82
[ 113.493681] [<ffffffff81133190>] cache_alloc_debugcheck_before.isra.50+0x1d/0x24
[ 113.494508] [<ffffffff81134876>] kmem_cache_alloc+0x31/0x18f
[ 113.495149] [<ffffffff814012b5>] skb_clone+0x64/0x80
[ 113.495712] [<ffffffff814e6f71>] pfkey_broadcast_one+0x3d/0xff
[ 113.496380] [<ffffffff814e7b84>] pfkey_broadcast+0xb5/0x11e
[ 113.497024] [<ffffffff814e82d1>] pfkey_register+0x191/0x1b1
[ 113.497653] [<ffffffff814e9770>] pfkey_process+0x162/0x17e
[ 113.498274] [<ffffffff814e9895>] pfkey_sendmsg+0x109/0x213
In pfkey_sendmsg the net mutex is taken and then pfkey_broadcast takes
the RCU lock. Fix by using GFP_ATOMIC for the allocation flag.
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
net/key/af_key.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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;
}
--
2.3.2 (Apple Git-55)
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] net af_key: Fix RCU splat 2015-08-20 15:51 [PATCH] net af_key: Fix RCU splat David Ahern @ 2015-08-20 16:51 ` Eric Dumazet 2015-08-20 22:57 ` David Ahern 2015-08-23 23:48 ` David Miller 1 sibling, 1 reply; 5+ messages in thread From: Eric Dumazet @ 2015-08-20 16:51 UTC (permalink / raw) To: David Ahern; +Cc: steffen.klassert, netdev On Thu, 2015-08-20 at 08:51 -0700, David Ahern wrote: > Hit the following splat testing VRF change for ipsec: > > [ 113.475692] =============================== > [ 113.476194] [ INFO: suspicious RCU usage. ] > [ 113.476667] 4.2.0-rc6-1+deb7u2+clUNRELEASED #3.2.65-1+deb7u2+clUNRELEASED Not tainted > [ 113.477545] ------------------------------- > [ 113.478013] /work/monster-14/dsa/kernel.git/include/linux/rcupdate.h:568 Illegal context switch in RCU read-side critical section! > [ 113.479288] > [ 113.479288] other info that might help us debug this: > [ 113.479288] > [ 113.480207] > [ 113.480207] rcu_scheduler_active = 1, debug_locks = 1 > [ 113.480931] 2 locks held by setkey/6829: > [ 113.481371] #0: (&net->xfrm.xfrm_cfg_mutex){+.+.+.}, at: [<ffffffff814e9887>] pfkey_sendmsg+0xfb/0x213 > [ 113.482509] #1: (rcu_read_lock){......}, at: [<ffffffff814e767f>] rcu_read_lock+0x0/0x6e > [ 113.483509] > [ 113.483509] stack backtrace: > [ 113.484041] CPU: 0 PID: 6829 Comm: setkey Not tainted 4.2.0-rc6-1+deb7u2+clUNRELEASED #3.2.65-1+deb7u2+clUNRELEASED > [ 113.485422] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org 04/01/2014 > [ 113.486845] 0000000000000001 ffff88001d4c7a98 ffffffff81518af2 ffffffff81086962 > [ 113.487732] ffff88001d538480 ffff88001d4c7ac8 ffffffff8107ae75 ffffffff8180a154 > [ 113.488628] 0000000000000b30 0000000000000000 00000000000000d0 ffff88001d4c7ad8 > [ 113.489525] Call Trace: > [ 113.489813] [<ffffffff81518af2>] dump_stack+0x4c/0x65 > [ 113.490389] [<ffffffff81086962>] ? console_unlock+0x3d6/0x405 > [ 113.491039] [<ffffffff8107ae75>] lockdep_rcu_suspicious+0xfa/0x103 > [ 113.491735] [<ffffffff81064032>] rcu_preempt_sleep_check+0x45/0x47 > [ 113.492442] [<ffffffff8106404d>] ___might_sleep+0x19/0x1c8 > [ 113.493077] [<ffffffff81064268>] __might_sleep+0x6c/0x82 > [ 113.493681] [<ffffffff81133190>] cache_alloc_debugcheck_before.isra.50+0x1d/0x24 > [ 113.494508] [<ffffffff81134876>] kmem_cache_alloc+0x31/0x18f > [ 113.495149] [<ffffffff814012b5>] skb_clone+0x64/0x80 > [ 113.495712] [<ffffffff814e6f71>] pfkey_broadcast_one+0x3d/0xff > [ 113.496380] [<ffffffff814e7b84>] pfkey_broadcast+0xb5/0x11e > [ 113.497024] [<ffffffff814e82d1>] pfkey_register+0x191/0x1b1 > [ 113.497653] [<ffffffff814e9770>] pfkey_process+0x162/0x17e > [ 113.498274] [<ffffffff814e9895>] pfkey_sendmsg+0x109/0x213 > > In pfkey_sendmsg the net mutex is taken and then pfkey_broadcast takes > the RCU lock. Fix by using GFP_ATOMIC for the allocation flag. > > Signed-off-by: David Ahern <dsa@cumulusnetworks.com> > --- > net/key/af_key.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net af_key: Fix RCU splat 2015-08-20 16:51 ` Eric Dumazet @ 2015-08-20 22:57 ` David Ahern 2015-08-20 23:36 ` Eric Dumazet 0 siblings, 1 reply; 5+ messages in thread From: David Ahern @ 2015-08-20 22:57 UTC (permalink / raw) To: Eric Dumazet; +Cc: steffen.klassert, netdev, Stephen Hemminger 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: >> >> [ 113.475692] =============================== >> [ 113.476194] [ INFO: suspicious RCU usage. ] >> [ 113.476667] 4.2.0-rc6-1+deb7u2+clUNRELEASED #3.2.65-1+deb7u2+clUNRELEASED Not tainted >> [ 113.477545] ------------------------------- >> [ 113.478013] /work/monster-14/dsa/kernel.git/include/linux/rcupdate.h:568 Illegal context switch in RCU read-side critical section! >> [ 113.479288] >> [ 113.479288] other info that might help us debug this: >> [ 113.479288] >> [ 113.480207] >> [ 113.480207] rcu_scheduler_active = 1, debug_locks = 1 >> [ 113.480931] 2 locks held by setkey/6829: >> [ 113.481371] #0: (&net->xfrm.xfrm_cfg_mutex){+.+.+.}, at: [<ffffffff814e9887>] pfkey_sendmsg+0xfb/0x213 >> [ 113.482509] #1: (rcu_read_lock){......}, at: [<ffffffff814e767f>] rcu_read_lock+0x0/0x6e >> [ 113.483509] >> [ 113.483509] stack backtrace: >> [ 113.484041] CPU: 0 PID: 6829 Comm: setkey Not tainted 4.2.0-rc6-1+deb7u2+clUNRELEASED #3.2.65-1+deb7u2+clUNRELEASED >> [ 113.485422] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org 04/01/2014 >> [ 113.486845] 0000000000000001 ffff88001d4c7a98 ffffffff81518af2 ffffffff81086962 >> [ 113.487732] ffff88001d538480 ffff88001d4c7ac8 ffffffff8107ae75 ffffffff8180a154 >> [ 113.488628] 0000000000000b30 0000000000000000 00000000000000d0 ffff88001d4c7ad8 >> [ 113.489525] Call Trace: >> [ 113.489813] [<ffffffff81518af2>] dump_stack+0x4c/0x65 >> [ 113.490389] [<ffffffff81086962>] ? console_unlock+0x3d6/0x405 >> [ 113.491039] [<ffffffff8107ae75>] lockdep_rcu_suspicious+0xfa/0x103 >> [ 113.491735] [<ffffffff81064032>] rcu_preempt_sleep_check+0x45/0x47 >> [ 113.492442] [<ffffffff8106404d>] ___might_sleep+0x19/0x1c8 >> [ 113.493077] [<ffffffff81064268>] __might_sleep+0x6c/0x82 >> [ 113.493681] [<ffffffff81133190>] cache_alloc_debugcheck_before.isra.50+0x1d/0x24 >> [ 113.494508] [<ffffffff81134876>] kmem_cache_alloc+0x31/0x18f >> [ 113.495149] [<ffffffff814012b5>] skb_clone+0x64/0x80 >> [ 113.495712] [<ffffffff814e6f71>] pfkey_broadcast_one+0x3d/0xff >> [ 113.496380] [<ffffffff814e7b84>] pfkey_broadcast+0xb5/0x11e >> [ 113.497024] [<ffffffff814e82d1>] pfkey_register+0x191/0x1b1 >> [ 113.497653] [<ffffffff814e9770>] pfkey_process+0x162/0x17e >> [ 113.498274] [<ffffffff814e9895>] pfkey_sendmsg+0x109/0x213 >> >> In pfkey_sendmsg the net mutex is taken and then pfkey_broadcast takes >> the RCU lock. Fix by using GFP_ATOMIC for the allocation flag. >> >> Signed-off-by: David Ahern <dsa@cumulusnetworks.com> >> --- >> net/key/af_key.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> 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. David ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net af_key: Fix RCU splat 2015-08-20 22:57 ` David Ahern @ 2015-08-20 23:36 ` Eric Dumazet 0 siblings, 0 replies; 5+ messages in thread From: Eric Dumazet @ 2015-08-20 23:36 UTC (permalink / raw) To: David Ahern; +Cc: steffen.klassert, netdev, Stephen Hemminger 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] net af_key: Fix RCU splat 2015-08-20 15:51 [PATCH] net af_key: Fix RCU splat David Ahern 2015-08-20 16:51 ` Eric Dumazet @ 2015-08-23 23:48 ` David Miller 1 sibling, 0 replies; 5+ messages in thread From: David Miller @ 2015-08-23 23:48 UTC (permalink / raw) To: dsa; +Cc: steffen.klassert, netdev From: David Ahern <dsa@cumulusnetworks.com> Date: Thu, 20 Aug 2015 08:51:40 -0700 > @@ -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)); As Eric alluded to, the gfp_t argument is totally pointless. It is used inside of pfkey_broadcast() via calls to pfkey_broadcast_one() inside of an RCU protected area _CREATED_ by pfkey_broadcast() itself. Therefore it could never possibly honor a sleeping gfp_t flag, and GFP_ATOMIC must always be used. So just get rid of it and use GFP_ATOMIC unconditionally. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-08-23 23:48 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-20 15:51 [PATCH] net af_key: Fix RCU splat David Ahern 2015-08-20 16:51 ` Eric Dumazet 2015-08-20 22:57 ` David Ahern 2015-08-20 23:36 ` Eric Dumazet 2015-08-23 23:48 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).