[PATCH net-next v6 0/2] net: phy: fix a second attach of an attached PHY and the module put on detach

[PATCH net-next v6 0/2] net: phy: fix a second attach of an attached PHY and the module put on detach

[Aleksei Sviridkin]

Two phylib fixes found on a Keenetic KN-1012 (MT7981) while chasing a
PHY driver unbind race. A second attach of an attached PHY tears down
the first consumer, and a detach after a driver unbind leaks the module
reference.

1: refuse a second attach before the driver module is taken or the
   generic driver bound.
2: put the module the attach pinned, not whatever driver is bound at
   detach.

Patch 1 goes first: with patch 2 alone, the second attach error path
leaves the first consumer's module reference pinned.

Changes in v6 (since v5):
https://lore.kernel.org/netdev/20261009180551.1234107-1-f@lex.la/
- Dropped patches 3 and 4 (the bind_lock and the unbind wait), as
  Andrew suggested: unbinding a PHY driver is a development tool and
  not worth that complexity.
- Dropped the Fixes tags, the series targets net-next.
- Patch 2: added Andrew's Reviewed-by.

Aleksei Sviridkin (2):
  net: phy: refuse a second attach before touching the PHY
  net: phy: put the driver module the attach took

 drivers/net/phy/phy_device.c | 25 +++++++++++++++----------
 include/linux/phy.h          |  2 ++
 2 files changed, 17 insertions(+), 10 deletions(-)

-- 
2.53.0

[PATCH net-next v6 1/2] net: phy: refuse a second attach before touching the PHY

[Aleksei Sviridkin]

phy_attach_direct() tests phydev->attached_dev only after it has taken
the driver module and, for a PHY with no driver, bound the generic one,
and then leaves through the error path that calls
phy_detach_internal(). That call works on the PHY's current attachment,
so a second attach of a PHY in use tears down the first consumer: it
clears that netdev's phydev pointer and the PHY's attached_dev, removes
the sysfs links and suspends the PHY, while the first consumer goes on
using it. A PHY on the generic driver never reaches the test: the
attach runs its probe again and, when the bind fails, clears d->driver
under the first consumer.

Found in review of a change that makes phy_detach() put only the
module the attach recorded: with it, this path leaves the first
consumer's reference pinned. On a Keenetic KN-1012, a test module
attaching a second netdev to the EN8811H that the lan4 DSA port holds
got -EBUSY and left lan4 with no PHY; with this change lan4 kept it.

Test whether the PHY is attached before the driver module is taken or
the generic driver bound, and refuse from there. A PHY attached without
a netdev, as DSA does for its CPU and link ports, has no attached_dev,
so test phy_link_change, which every attach sets and every detach
clears. A second attach of such a PHY used to succeed and share it;
with that change it would also leak a module reference.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
v6: dropped the Fixes tag.

 drivers/net/phy/phy_device.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index bfce8b893644..6bc9cdb7fa69 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1811,6 +1811,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 
 	get_device(d);
 
+	/* Set by every attach, with or without a netdev */
+	if (phydev->phy_link_change) {
+		phydev_err(phydev, "PHY already attached\n");
+		err = -EBUSY;
+		goto error_put_device;
+	}
+
 	/* Assume that if there is no driver, that it doesn't
 	 * exist, and we should use the genphy driver.
 	 */
@@ -1838,12 +1845,6 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 			goto error_module_put;
 	}
 
-	if (phydev->attached_dev) {
-		dev_err(&dev->dev, "PHY already attached\n");
-		err = -EBUSY;
-		goto error;
-	}
-
 	phydev->phy_link_change = phy_link_change;
 	if (dev) {
 		phydev->attached_dev = dev;
-- 
2.53.0

Re: [PATCH net-next v6 1/2] net: phy: refuse a second attach before touching the PHY

[Andrew Lunn]

On Tue, Oct 13, 2026 at 06:11:59PM +0300, Aleksei Sviridkin wrote:
> phy_attach_direct() tests phydev->attached_dev only after it has taken
> the driver module and, for a PHY with no driver, bound the generic one,
> and then leaves through the error path that calls
> phy_detach_internal(). That call works on the PHY's current attachment,
> so a second attach of a PHY in use tears down the first consumer: it
> clears that netdev's phydev pointer and the PHY's attached_dev, removes
> the sysfs links and suspends the PHY, while the first consumer goes on
> using it. A PHY on the generic driver never reaches the test: the
> attach runs its probe again and, when the bind fails, clears d->driver
> under the first consumer.
> 
> Found in review of a change that makes phy_detach() put only the
> module the attach recorded: with it, this path leaves the first
> consumer's reference pinned. On a Keenetic KN-1012, a test module
> attaching a second netdev to the EN8811H that the lan4 DSA port holds
> got -EBUSY and left lan4 with no PHY; with this change lan4 kept it.
> 
> Test whether the PHY is attached before the driver module is taken or
> the generic driver bound, and refuse from there. A PHY attached without
> a netdev, as DSA does for its CPU and link ports, has no attached_dev,
> so test phy_link_change, which every attach sets and every detach
> clears. A second attach of such a PHY used to succeed and share it;
> with that change it would also leak a module reference.
> 
> Assisted-by: LLM
> Signed-off-by: Aleksei Sviridkin <f@lex.la>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

[PATCH net-next v6 2/2] net: phy: put the driver module the attach took

[Aleksei Sviridkin]

phy_attach_direct() pins the PHY driver module through d->driver, and
phy_detach() releases it by reading d->driver again. Unbinding the PHY
driver while the PHY is attached clears that pointer, so a detach that
runs while the driver is still unbound skips the put and the module can
no longer be unloaded; if a different driver binds in between, the put
lands on a module that was never pinned. The NULL test added by
commit c2b727df7caa ("net: phy: Avoid NPD upon phy_detach() when driver
is unbound") avoids the oops but skips the put.

Found by reading phy_detach() while chasing a PHY driver unbind race on
a Keenetic KN-1012 (MT7981, air_en8811h built as a module). The leak
itself was not observed there: unbinding air_en8811h under the attached
lan4 DSA port and then unbinding the switch faults earlier on that
board, in phy_free_interrupt() or under phy_stop(), before
phy_detach() gets to the put.

Remember which module was pinned and release that one.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
v6: dropped the Fixes tag, added Andrew's Reviewed-by.

 drivers/net/phy/phy_device.c | 12 ++++++++----
 include/linux/phy.h          |  2 ++
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 6bc9cdb7fa69..68691c487eee 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1728,8 +1728,8 @@ static void phy_detach_internal(struct phy_device *phydev, bool notify_bus)
 	phydev->phy_link_change = NULL;
 	phydev->phylink = NULL;
 
-	if (phydev->mdio.dev.driver)
-		module_put(phydev->mdio.dev.driver->owner);
+	module_put(phydev->drv_owner);
+	phydev->drv_owner = NULL;
 
 	/* If the device had no specific driver before (i.e. - it
 	 * was using the generic driver), we unbind the device
@@ -1794,6 +1794,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	struct mii_bus *bus = phydev->mdio.bus;
 	struct device *d = &phydev->mdio.dev;
 	struct module *ndev_owner = NULL;
+	struct module *drv_owner;
 	int irq = phydev->irq;
 	int err;
 
@@ -1830,11 +1831,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 		phydev->is_genphy_driven = 1;
 	}
 
-	if (!try_module_get(d->driver->owner)) {
+	drv_owner = d->driver->owner;
+	if (!try_module_get(drv_owner)) {
 		phydev_err(phydev, "failed to get the device driver module\n");
 		err = -EIO;
 		goto error_put_device;
 	}
+	phydev->drv_owner = drv_owner;
 
 	if (phydev->is_genphy_driven) {
 		err = d->driver->probe(d);
@@ -1942,7 +1945,8 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
 	return err;
 
 error_module_put:
-	module_put(d->driver->owner);
+	module_put(phydev->drv_owner);
+	phydev->drv_owner = NULL;
 	phydev->irq = irq;
 	phydev->is_genphy_driven = 0;
 	d->driver = NULL;
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7c5098a0dd6c..a5a419bc400e 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -578,6 +578,7 @@ struct phy_oatc14_sqi_capability {
  *
  * @mdio: MDIO bus this PHY is on
  * @drv: Pointer to the driver for this PHY instance
+ * @drv_owner: Driver module phy_attach_direct() took a reference on
  * @devlink: Create a link between phy dev and mac dev, if the external phy
  *           used by current mac interface is managed by another mac interface.
  * @phyindex: Unique id across the phy's parent tree of phys to address the PHY
@@ -689,6 +690,7 @@ struct phy_device {
 	/* Information about the PHY type */
 	/* And management functions */
 	const struct phy_driver *drv;
+	struct module *drv_owner;
 
 	struct device_link *devlink;
 
-- 
2.53.0

Re: [PATCH net-next v6 0/2] net: phy: fix a second attach of an attached PHY and the module put on detach

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 13 Oct 2026 18:11:58 +0300 you wrote:
> Two phylib fixes found on a Keenetic KN-1012 (MT7981) while chasing a
> PHY driver unbind race. A second attach of an attached PHY tears down
> the first consumer, and a detach after a driver unbind leaks the module
> reference.
> 
> 1: refuse a second attach before the driver module is taken or the
>    generic driver bound.
> 2: put the module the attach pinned, not whatever driver is bound at
>    detach.
> 
> [...]

Here is the summary with links:
  - [net-next,v6,1/2] net: phy: refuse a second attach before touching the PHY
    https://git.kernel.org/netdev/net-next/c/fa38468cd26b
  - [net-next,v6,2/2] net: phy: put the driver module the attach took
    https://git.kernel.org/netdev/net-next/c/7151b86654fa

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html