Re: [PATCH net v2] net: phylink: unwind the PHY binding when bringup fails late

[Andrew Lunn <andrew@lunn.ch>]

On Sat, Sep 19, 2026 at 04:53:38AM +0300, Aleksei Sviridkin wrote:
> phylink_bringup_phy() records the PHY in pl->phydev before its last
> fallible step: on a MAC whose phylink ops implement LPI,
> phy_eee_rx_clock_stop() can fail with a real MDIO error. The callers
> unwind with phy_detach(), which knows nothing about pl->phydev, so a
> pointer to a PHY that is no longer attached outlives the failed
> connect.
> 
> What that costs depends on how the caller got here.
> phylink_connect_phy() goes through phylink_attach_phy(), which refuses
> to attach while pl->phydev is set, turning a transient MDIO error into
> a permanent -EBUSY. The SFP path is worse than that: sfp_sm_probe_phy()
> answers the failure with phy_device_remove() and phy_device_free(), and
> it assigns sfp->mod_phy only past that error return, so nothing clears
> pl->phydev and it is left pointing at a freed phy_device that
> phylink_resolve() and the ethtool helpers go on reading.
> phylink_fwnode_phy_connect() has no such check, so a later connect
> overwrites the stale pointer and hides the problem. A disconnect does
> not: phylink_disconnect_phy() hands that pointer to phy_disconnect(),
> and the second phy_detach() on the same PHY drops references the first
> one already released.
> 
> Found while making a DSA port survive a PHY whose driver arrives after
> the switch probes: keeping the port across a failed connect and
> retrying is what makes this window reachable.
> 
> Clear the binding on the failure path. This is the same operation
> phylink_disconnect_phy() performs, so both now share a helper. The
> PHY-side fields are left to phy_detach(), which every caller already
> runs on this path.

What is missing here is an explanation why you cannot record the PHY
in pl->phydev later, once all calls which can fail have been
performed. That seems like a simpler and more logical fix. But maybe
i'm missing something.

	Andrew