netdev - Re: [PATCH v3 net 11/16] ipv6: input: convert to dev_net

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: <20250204133025.78c466ec@kernel.org>
Date: Tue, 4 Feb 2025 13:30:25 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: "Paul E. McKenney" <paulmck@...nel.org>
Cc: Eric Dumazet <edumazet@...gle.com>, "David S . Miller"
 <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>,
 netdev@...r.kernel.org, Kuniyuki Iwashima <kuniyu@...zon.com>, Simon Horman
 <horms@...nel.org>, eric.dumazet@...il.com, rcu@...r.kernel.org
Subject: Re: [PATCH v3 net 11/16] ipv6: input: convert to dev_net_rcu()

On Tue, 4 Feb 2025 13:17:08 -0800 Paul E. McKenney wrote:
> > > TBH I'm slightly confused by this, and the previous warnings.
> > >
> > > The previous one was from a timer callback.
> > >
> > > This one is with BH disabled.
> > >
> > > I thought BH implies RCU protection. We certainly depend on that
> > > in NAPI for XDP. And threaded NAPI does the exact same thing as
> > > xfrm_trans_reinject(), a bare local_bh_disable().
> > >
> > > RCU folks, did something change or is just holes in my brain again?  
> > 
> > Nope, BH does not imply rcu_read_lock()  
> 
> You are both right?  ;-)
> 
> The synchronize_rcu() function will wait for all types of RCU readers,
> including BH-disabled regions of code.  However, lockdep can distinguish
> between the various sorts of readers.  So for example
> 
> 	lockdep_assert_in_rcu_read_lock_bh();
> 
> will complain unless you did rcu_read_lock_bh(), even if you did something
> like disable_bh().  If you don't want to distinguish and are happy with
> any type of RCU reader, you can use
> 
> 	lockdep_assert_in_rcu_reader();
> 
> I have been expecting that CONFIG_PREEMPT_RT=y kernels will break this
> any day now, but so far so good.  ;-)

Thanks Paul! So IIUC in this case we could:

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 0f5eb9db0c62..58ec1eb9ae6a 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -401,7 +401,7 @@ static inline struct net *read_pnet(const possible_net_t *pnet)
 static inline struct net *read_pnet_rcu(possible_net_t *pnet)
 {
 #ifdef CONFIG_NET_NS
-	return rcu_dereference(pnet->net);
+	return rcu_dereference_check(pnet->net, rcu_read_lock_bh_held());
 #else
 	return &init_net;
 #endif

Sorry for the sideline, Eric, up to you how to proceed..
I'll try to remember the details better next time :)