| Message ID | 20221130091759.682841-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: mdiobus: remove unneccessary fwnode_handle_put() in the error path | expand |
On Wed, Nov 30, 2022 at 05:17:59PM +0800, Yang Yingliang wrote: > If phy_device_register() or fwnode_mdiobus_phy_device_register() > fail, phy_device_free() is called, the device refcount is decreased > to 0, then fwnode_handle_put() is called in phy_device_release(), > so the fwnode_handle_put() in the error path can be removed to avoid > double put. > > Fixes: cdde1560118f ("net: mdiobus: fix unbalanced node reference count") > Reported-by: Zeng Heng <zengheng4@huawei.com> > Tested-by: Zeng Heng <zengheng4@huawei.com> > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/net/mdio/fwnode_mdio.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c > index eb344f6d4a7b..e584abda585b 100644 > --- a/drivers/net/mdio/fwnode_mdio.c > +++ b/drivers/net/mdio/fwnode_mdio.c > @@ -97,10 +97,8 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio, > * register it > */ > rc = phy_device_register(phy); > - if (rc) { > - fwnode_handle_put(child); > + if (rc) > return rc; > - } The current code is: /* Associate the fwnode with the device structure so it * can be looked up later */ fwnode_handle_get(child); device_set_node(&phy->mdio.dev, child); /* All data is now stored in the phy struct; * register it */ rc = phy_device_register(phy); if (rc) { fwnode_handle_put(child); return rc; } The fwnode_handle_put(child) balances the earlier fwnode_handle_get(child). The code will look wrong without this balance. Please find a better fix which actually looks correct. Andrew
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c index eb344f6d4a7b..e584abda585b 100644 --- a/drivers/net/mdio/fwnode_mdio.c +++ b/drivers/net/mdio/fwnode_mdio.c @@ -97,10 +97,8 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio, * register it */ rc = phy_device_register(phy); - if (rc) { - fwnode_handle_put(child); + if (rc) return rc; - } dev_dbg(&mdio->dev, "registered phy %p fwnode at address %i\n", child, addr); @@ -152,10 +150,8 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus, /* All data is now stored in the phy struct, so register it */ rc = phy_device_register(phy); - if (rc) { - fwnode_handle_put(phy->mdio.dev.fwnode); + if (rc) goto clean_phy; - } } else if (is_of_node(child)) { rc = fwnode_mdiobus_phy_device_register(bus, phy, child, addr); if (rc)
If phy_device_register() or fwnode_mdiobus_phy_device_register() fail, phy_device_free() is called, the device refcount is decreased to 0, then fwnode_handle_put() is called in phy_device_release(), so the fwnode_handle_put() in the error path can be removed to avoid double put. Fixes: cdde1560118f ("net: mdiobus: fix unbalanced node reference count") Reported-by: Zeng Heng <zengheng4@huawei.com> Tested-by: Zeng Heng <zengheng4@huawei.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/net/mdio/fwnode_mdio.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)