| Message ID | 20230720000231.1939689-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1c613beaf877c0c0d755853dc62687e2013e55c4 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: phy: prevent stale pointer dereference in phy_init() | expand |
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 20 Jul 2023 03:02:31 +0300 you wrote: > mdio_bus_init() and phy_driver_register() both have error paths, and if > those are ever hit, ethtool will have a stale pointer to the > phy_ethtool_phy_ops stub structure, which references memory from a > module that failed to load (phylib). > > It is probably hard to force an error in this code path even manually, > but the error teardown path of phy_init() should be the same as > phy_exit(), which is now simply not the case. > > [...] Here is the summary with links: - [net] net: phy: prevent stale pointer dereference in phy_init() https://git.kernel.org/netdev/net/c/1c613beaf877 You are awesome, thank you!
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 0c2014accba7..61921d4dbb13 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -3451,23 +3451,30 @@ static int __init phy_init(void) { int rc; + ethtool_set_ethtool_phy_ops(&phy_ethtool_phy_ops); + rc = mdio_bus_init(); if (rc) - return rc; + goto err_ethtool_phy_ops; - ethtool_set_ethtool_phy_ops(&phy_ethtool_phy_ops); features_init(); rc = phy_driver_register(&genphy_c45_driver, THIS_MODULE); if (rc) - goto err_c45; + goto err_mdio_bus; rc = phy_driver_register(&genphy_driver, THIS_MODULE); - if (rc) { - phy_driver_unregister(&genphy_c45_driver); + if (rc) + goto err_c45; + + return 0; + err_c45: - mdio_bus_exit(); - } + phy_driver_unregister(&genphy_c45_driver); +err_mdio_bus: + mdio_bus_exit(); +err_ethtool_phy_ops: + ethtool_set_ethtool_phy_ops(NULL); return rc; }
mdio_bus_init() and phy_driver_register() both have error paths, and if those are ever hit, ethtool will have a stale pointer to the phy_ethtool_phy_ops stub structure, which references memory from a module that failed to load (phylib). It is probably hard to force an error in this code path even manually, but the error teardown path of phy_init() should be the same as phy_exit(), which is now simply not the case. Fixes: 55d8f053ce1b ("net: phy: Register ethtool PHY operations") Link: https://lore.kernel.org/netdev/ZLaiJ4G6TaJYGJyU@shell.armlinux.org.uk/ Suggested-by: Russell King (Oracle) <linux@armlinux.org.uk> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/phy/phy_device.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)