Skip to content

Commit

Permalink
net: phy: Fix PHY module checks and NULL deref in phy_attach_direct()
Browse files Browse the repository at this point in the history
The Generic PHY drivers gets assigned after we checked that the current
PHY driver is NULL, so we need to check a few things before we can
safely dereference d->driver. This would be causing a NULL deference to
occur when a system binds to the Generic PHY driver. Update
phy_attach_direct() to do the following:

- grab the driver module reference after we have assigned the Generic
  PHY drivers accordingly, and remember we came from the generic PHY
  path

- update the error path to clean up the module reference in case the
  Generic PHY probe function fails

- split the error path involving phy_detacht() to avoid double free/put
  since phy_detach() does all the clean up

- finally, have phy_detach() drop the module reference count before we
  call device_release_driver() for the Generic PHY driver case

Fixes: cafe8df ("net: phy: Fix lack of reference count on PHY driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Florian Fainelli authored and David S. Miller committed Feb 9, 2017
1 parent 075ad76 commit 6d9f66a
Showing 1 changed file with 20 additions and 8 deletions.
28 changes: 20 additions & 8 deletions drivers/net/phy/phy_device.c
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,7 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
struct module *ndev_owner = dev->dev.parent->driver->owner;
struct mii_bus *bus = phydev->mdio.bus;
struct device *d = &phydev->mdio.dev;
bool using_genphy = false;
int err;

/* For Ethernet device drivers that register their own MDIO bus, we
Expand All @@ -920,11 +921,6 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
return -EIO;
}

if (!try_module_get(d->driver->owner)) {
dev_err(&dev->dev, "failed to get the device driver module\n");
return -EIO;
}

get_device(d);

/* Assume that if there is no driver, that it doesn't
Expand All @@ -938,12 +934,22 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
d->driver =
&genphy_driver[GENPHY_DRV_1G].mdiodrv.driver;

using_genphy = true;
}

if (!try_module_get(d->driver->owner)) {
dev_err(&dev->dev, "failed to get the device driver module\n");
err = -EIO;
goto error_put_device;
}

if (using_genphy) {
err = d->driver->probe(d);
if (err >= 0)
err = device_bind_driver(d);

if (err)
goto error;
goto error_module_put;
}

if (phydev->attached_dev) {
Expand Down Expand Up @@ -980,9 +986,14 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
return err;

error:
/* phy_detach() does all of the cleanup below */
phy_detach(phydev);
put_device(d);
return err;

error_module_put:
module_put(d->driver->owner);
error_put_device:
put_device(d);
if (ndev_owner != bus->owner)
module_put(bus->owner);
return err;
Expand Down Expand Up @@ -1045,6 +1056,8 @@ void phy_detach(struct phy_device *phydev)

phy_led_triggers_unregister(phydev);

module_put(phydev->mdio.dev.driver->owner);

/* If the device had no specific driver before (i.e. - it
* was using the generic driver), we unbind the device
* from the generic driver so that there's a chance a
Expand All @@ -1065,7 +1078,6 @@ void phy_detach(struct phy_device *phydev)
bus = phydev->mdio.bus;

put_device(&phydev->mdio.dev);
module_put(phydev->mdio.dev.driver->owner);
if (ndev_owner != bus->owner)
module_put(bus->owner);
}
Expand Down

0 comments on commit 6d9f66a

Please sign in to comment.