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 <redacted>
---
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