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] [day] [month] [year] [list]

Message-ID: <85496569-9ecc-447d-8b76-659c68aee3ea@paulmck-laptop>
Date: Tue, 4 Feb 2025 15:25:15 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Jakub Kicinski <kuba@...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, Feb 04, 2025 at 01:30:25PM -0800, Jakub Kicinski wrote:
> 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());

That should do it!

							Thanx, Paul

>  #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 :)