phy_attach_direct() tests phydev->attached_dev only after it has taken
its references and, for a PHY with no driver, bound the generic one, and
then leaves through the error path that calls phy_detach(). That
phy_detach() 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,
suspends the PHY and, for the generic driver, unbinds it, while the
first consumer goes on using the PHY.
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 attached_dev before anything is taken and return from there.
Fixes: a7dac9f9c169 ("phy: fix error case of phy_led_triggers_(un)register")
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <redacted>
---
drivers/net/phy/phy_device.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 94b2e85e00a3..7046976c5b6a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1757,6 +1757,11 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
struct module *ndev_owner = NULL;
int err;
+ if (phydev->attached_dev) {
+ phydev_err(phydev, "PHY already attached\n");
+ return -EBUSY;
+ }
+
/* For Ethernet device drivers that register their own MDIO bus, we
* will have bus->owner match ndev_mod, so we do not want to increment
* our own module->refcnt here, otherwise we would not be able to@@ -1798,12 +1803,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