generic: backport phylink fix for a PHY recorded before bringup fails

phylink_bringup_phy() stores the PHY in pl->phydev before its last step that can fail. If that step fails, the pointer outlives the failed connect: a later phylink_connect_phy() gets a permanent -EBUSY, the SFP path is left with a pointer to a freed phy_device, and a disconnect detaches the same PHY twice. The fix publishes the pointer only after the last call that can fail. It is also the base the pending phylink late-PHY series relies on: retrying a connect after a failed bringup is what makes this window reachable. The fix is in net: https://git.kernel.org/netdev/net/c/a940003f44e7 The phylink patches that apply on top of it are refreshed for hunk offsets. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> Link: https://github.com/openwrt/openwrt/pull/25459 Signed-off-by: Jonas Jelonek <jonas@jonasjelonek.de>
2026-10-14 02:19:06 +04:00 · 2026-09-28 23:58:48 +02:00
1 parent 9df055123c
commit 5dcf6a3b3d
@@ -0,0 +1,83 @@
+From a940003f44e7e441c228151dd212642152700ec8 Mon Sep 17 00:00:00 2001
+From: Aleksei Sviridkin <f@lex.la>
+Date: Mon, 21 Sep 2026 01:20:44 +0300
+Subject: [PATCH] net: phylink: record the PHY only once bringup cannot fail
+
+phylink_bringup_phy() stores 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.
+
+Publish the pointer after the last call that can fail instead of
+unwinding it afterwards. Nothing between the two points reads
+pl->phydev, and the registration that follows cannot fail:
+phy_request_interrupt() falls back to polling on its own. The PHY-side
+state keeps the order it had, so no MDIO operation moves relative to
+another.
+
+Fixes: 03abf2a7c654 ("net: phylink: add EEE management")
+Signed-off-by: Aleksei Sviridkin <f@lex.la>
+Link: https://patch.msgid.link/20260920222044.1752860-1-f@lex.la
+Signed-off-by: Jakub Kicinski <kuba@kernel.org>
+---
+ drivers/net/phy/phylink.c | 20 +++++++++++++++++---
+ 1 file changed, 17 insertions(+), 3 deletions(-)
+
+--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
+@@ -2119,7 +2119,6 @@ static int phylink_bringup_phy(struct ph
+ 	mutex_lock(&pl->phydev_mutex);
+ 	mutex_lock(&phy->lock);
+ 	mutex_lock(&pl->state_mutex);
+-	pl->phydev = phy;
+ 	pl->phy_state.interface = interface;
+ 	pl->phy_state.pause = MLO_PAUSE_NONE;
+ 	pl->phy_state.speed = SPEED_UNKNOWN;
+@@ -2186,10 +2185,25 @@ static int phylink_bringup_phy(struct ph
+ 			ret = 0;
+ 	}
+ 
+-	if (ret == 0 && phy_interrupt_is_valid(phy))
+	if (ret)
+		return ret;
+
+	/* Nothing below can fail, so the PHY can be recorded now. Doing it
+	 * here rather than above keeps a failed bringup from leaving
+	 * pl->phydev pointing at a PHY the caller is about to detach.
+	 */
+	mutex_lock(&pl->phydev_mutex);
+	mutex_lock(&phy->lock);
+	mutex_lock(&pl->state_mutex);
+	pl->phydev = phy;
+	mutex_unlock(&pl->state_mutex);
+	mutex_unlock(&phy->lock);
+	mutex_unlock(&pl->phydev_mutex);
+
+	if (phy_interrupt_is_valid(phy))
+ 		phy_request_interrupt(phy);
+ 
+-	return ret;
+	return 0;
+ }
+ 
+ static int phylink_attach_phy(struct phylink *pl, struct phy_device *phy,
@@ -41,7 +41,7 @@ Signed-off-by: Jakub Kicinski <kuba@kernel.org>
 		return false;
 	return true;
 }
-@@ -2197,9 +2195,7 @@ static int phylink_attach_phy(struct phy
+@@ -2211,9 +2209,7 @@ static int phylink_attach_phy(struct phy
 {
 	u32 flags = 0;
 
@@ -20,7 +20,7 @@ Signed-off-by: Daniel Golle <daniel@makrotopia.org>

 --- a/drivers/net/phy/phylink.c
 +++ b/drivers/net/phy/phylink.c
-@@ -2280,7 +2280,7 @@ int phylink_fwnode_phy_connect(struct ph
+@@ -2294,7 +2294,7 @@ int phylink_fwnode_phy_connect(struct ph
 {
 	struct fwnode_handle *phy_fwnode;
 	struct phy_device *phy_dev;
@@ -29,7 +29,7 @@ Signed-off-by: Daniel Golle <daniel@makrotopia.org>
 
 	/* Fixed links and 802.3z are handled without needing a PHY */
 	if (pl->cfg_link_an_mode == MLO_AN_FIXED ||
-@@ -2310,6 +2310,25 @@ int phylink_fwnode_phy_connect(struct ph
+@@ -2324,6 +2324,25 @@ int phylink_fwnode_phy_connect(struct ph
 	if (pl->config->mac_requires_rxc)
 		flags |= PHY_F_RXC_ALWAYS_ON;
 
@@ -62,7 +62,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 
 		if (phy_interface_empty(interfaces)) {
 			phylink_err(pl, "PHY has no common interfaces\n");
-@@ -2755,12 +2763,12 @@ static phy_interface_t phylink_sfp_selec
+@@ -2769,12 +2777,12 @@ static phy_interface_t phylink_sfp_selec
 		return interface;
 	}
 
@@ -77,7 +77,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 		return PHY_INTERFACE_MODE_NA;
 	}
 
-@@ -3688,14 +3696,14 @@ static int phylink_sfp_config_optical(st
+@@ -3702,14 +3710,14 @@ static int phylink_sfp_config_optical(st
 
 	phylink_dbg(pl, "optical SFP: interfaces=[mac=%*pbl, sfp=%*pbl]\n",
 		    (int)PHY_INTERFACE_MODE_MAX,
@@ -94,7 +94,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 			  pl->sfp_interfaces);
 	if (phy_interface_empty(pl->sfp_interfaces)) {
 		phylink_err(pl, "unsupported SFP module: no common interface modes\n");
-@@ -3866,7 +3874,7 @@ static int phylink_sfp_connect_phy(void
+@@ -3880,7 +3888,7 @@ static int phylink_sfp_connect_phy(void
 
 	/* Set the PHY's host supported interfaces */
 	phy_interface_and(phy->host_interfaces, phylink_sfp_interfaces,
@@ -383,7 +383,7 @@ Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
 	kfree(pl);
 }
 EXPORT_SYMBOL_GPL(phylink_destroy);
-@@ -2421,8 +2569,15 @@ void phylink_pcs_change(struct phylink_p
+@@ -2435,8 +2583,15 @@ void phylink_pcs_change(struct phylink_p
 {
 	struct phylink *pl = pcs->phylink;