linux-kernel - Re: [PATCH net-next v3] net: phy: Fix suspicious rcu

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: <Z49wsE4A94PGVes1@shell.armlinux.org.uk>
Date: Tue, 21 Jan 2025 10:02:24 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Kory Maincent <kory.maincent@...tlin.com>
Cc: Jakub Kicinski <kuba@...nel.org>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>,
	thomas.petazzoni@...tlin.com, Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next v3] net: phy: Fix suspicious rcu_dereference
 usage

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@...nel.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!