netdev - Re: [PATCH net-next] net: bonding: correctly annotate RCU in bond_should_notify

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: <310c8cfd1dc1747cf8ffc1f5be8994d0c87a008d.camel@sipsolutions.net>
Date: Tue, 23 Jul 2024 13:30:53 +0200
From: Johannes Berg <johannes@...solutions.net>
To: Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Cc: Jay Vosburgh <j.vosburgh@...il.com>, Andy Gospodarek
 <andy@...yhouse.net>,  Jiri Pirko <jiri@...dia.com>
Subject: Re: [PATCH net-next] net: bonding: correctly annotate RCU in
 bond_should_notify_peers()

On Tue, 2024-07-23 at 12:25 +0200, Paolo Abeni wrote:
> 
> On 7/19/24 18:41, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@...el.com>
> > 
> > RCU use in bond_should_notify_peers() looks wrong, since it does
> > rcu_dereference(), leaves the critical section, and uses the
> > pointer after that.
> > 
> > Luckily, it's called either inside a nested RCU critical section
> > or with the RTNL held.
> > 
> > Annotate it with rcu_dereference_rtnl() instead, and remove the
> > inner RCU critical section.
> > 
> > Fixes: 4cb4f97b7e36 ("bonding: rebuild the lock use for bond_mii_monitor()")
> > Reviewed-by: Jiri Pirko <jiri@...dia.com>
> > Signed-off-by: Johannes Berg <johannes.berg@...el.com>
> 
> Any special reasons to target net-next? this looks like a legit net fix 
> to me. If you want to target net, no need to re-post, otherwise it will 
> have to wait the merge window end.

Well, I guess it's kind of a fix, but functionally all it really does is
remove the RCU critical section which isn't necessary because either we
hold the lock or there's already one around it. So locally the function
_looks_ wrong (using the pointer outside the section it uses to deref
it), but because of other reasons in how the function is used, it's not
really wrong.

I'd really prefer not to have to resend it though ;-)

johannes