Re: [PATCH net-next v3] net: phy: Fix suspicious rcu_dereference usage

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Jan 21, 2025 at 10:38:45AM +0100, Kory Maincent wrote:
> On Mon, 20 Jan 2025 11:12:28 -0800
> Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > On Mon, 20 Jan 2025 15:19:25 +0100 Kory Maincent wrote:
> > > The path reported to not having RTNL lock acquired is the suspend path of
> > > the ravb MAC driver. Without this fix we got this warning:  
> > 
> > I maintain that ravb is buggy, plenty of drivers take rtnl_lock 
> > from the .suspend callback. We need _some_ write protection here,
> > the patch as is only silences a legitimate warning.
> 
> Indeed if the suspend path is buggy we should fix it. Still there is lots of
> ethernet drivers calling phy_disconnect without rtnl (IIUC) if probe return an
> error or in the remove path. What should we do about it?

They could trigger the same warning, although I think they would be
relatively safe because register_netdev() hasn't been called, and thus
nothing that the netdev provides should be used. (If it can be used, as
the driver has not completed initialisation, then it's probably racy
anyway.)

I don't think throwing ASSERT_RTNL() into phy_detach() will do anything
to solve this. If the RCU warning doesn't trigger (because phy_detach()
only gets called on error which practically never happens) then
ASSERT_RTNL() isn't going to trigger either. Warnings in functions will
only work when they're called in a context that will trigger the
warning!

So, I think it's something that can only be addressed by reviewing
drivers and patching.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!