linux-kernel - Re: [PATCH] net: phy: Fix premature resume by a PHY driver

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: <20250719062550.652-1-dev.nuvorolabs@gmail.com>
Date: Sat, 19 Jul 2025 06:25:47 +0000
From: Abid Ali <dev.nuvorolabs@...il.com>
To: Abid Ali <dev.nuvorolabs@...il.com>
Cc: Russell King <linux@...linux.org.uk>,
	Andrew Lunn <andrew@...n.ch>,
	Heiner Kallweit <hkallweit1@...il.com>,
	"David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: phy: Fix premature resume by a PHY driver

On Fri, Jul 18, 2025 at 05:10:54 PM +0100, Russell King (Oracle) wrote:
> Sorry but no. The PHY will be "resumed" from boot, even if it wasn't
> "suspended". So the idea that resume should only be called if it was
> previously suspended is incorrect.

> E.g. .ndo_open -> ... -> phy_attach_direct() -> phy_resume() ->
> 	phydrv->resume()

I do point this path out and there is also a second call
(2) .ndo_open -> phylink_start -> phy_start -> __phy_resume
This would mean 2 calls to the PHY resume every time an interface is
taken UP is expected behaviour?.

> During this path, the PHY may or may not be suspended, depending on
> the state of the hardware when control was passed to the kernel,
> which includes kexec().

> PHY drivers must cope with an already functional PHY when their
> resume() method is called.

This is not what I expected, but if it is by design I do not see a
need to fight it. Just to make it clear, if we need to reset a PHY
after it returns from suspend(or any code thats dependant), the driver`s
callback should provide this guarantee?.
if yes, this was just a misconception of relating it to resume callbacks
provided by the PM subsystem where such behaviour is not exepcted

Obvious logic error in the patch but this patch is not required as it stands.
-	if (!phydrv || !phydrv->resume && phydev->suspended)
+	if (!phydrv || !phydrv->resume || !phydev->suspended)