| Message ID | 20231126141046.3505343-2-claudiu.beznea@tuxon.dev (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: macb: Fixes for macb driver | expand |
On Sun, Nov 26, 2023 at 04:10:45PM +0200, Claudiu Beznea wrote: > The macb driver calls mdiobus_unregister() and mdiobus_free() in its remove > function before calling unregister_netdev(). unregister_netdev() calls the > driver-specific struct net_device_ops::ndo_stop function (macb_close()), > and macb_close() calls phylink_disconnect_phy(). This, in turn, will call: > > phy_disconnect() -> > phy_free_interrupt() -> > phy_disable_interrupts() -> > phy_config_interrupt() > > which dereference phydev->drv, which was already freed by: > mdiobus_unregister() -> > phy_mdio_device_remove() -> > device_del() -> > bus_remove_device() -> > device_release_driver_internal() -> > phy_remove() > > from macb_close(). > > Although the sequence in the macb driver is not correct, check phydev->drv > before dereferencing it in phy_config_interrupt() to avoid scenarios > like the one described. I don't know why I've ended up with two copies of this series, but as said in the other posting of this patch (where details of why can be found)... NAK.
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index a5fa077650e8..dd98a4b3ef81 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -165,7 +165,7 @@ EXPORT_SYMBOL_GPL(phy_get_rate_matching); static int phy_config_interrupt(struct phy_device *phydev, bool interrupts) { phydev->interrupts = interrupts ? 1 : 0; - if (phydev->drv->config_intr) + if (phydev->drv && phydev->drv->config_intr) return phydev->drv->config_intr(phydev); return 0;
The macb driver calls mdiobus_unregister() and mdiobus_free() in its remove function before calling unregister_netdev(). unregister_netdev() calls the driver-specific struct net_device_ops::ndo_stop function (macb_close()), and macb_close() calls phylink_disconnect_phy(). This, in turn, will call: phy_disconnect() -> phy_free_interrupt() -> phy_disable_interrupts() -> phy_config_interrupt() which dereference phydev->drv, which was already freed by: mdiobus_unregister() -> phy_mdio_device_remove() -> device_del() -> bus_remove_device() -> device_release_driver_internal() -> phy_remove() from macb_close(). Although the sequence in the macb driver is not correct, check phydev->drv before dereferencing it in phy_config_interrupt() to avoid scenarios like the one described. Fixes: 00db8189d984 ("This patch adds a PHY Abstraction Layer to the Linux Kernel") Signed-off-by: Claudiu Beznea <claudiu.beznea@tuxon.dev> --- drivers/net/phy/phy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)