Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

net: dsa: bcm_sf2: Fix possible memory leak in bcm_sf2_mdio_register()

In bcm_sf2_mdio_register(), the class_find_device() will call get_device()
to increment reference count for priv->master_mii_bus->dev if
of_mdio_find_bus() succeeds. If mdiobus_alloc() or mdiobus_register()
fails, it will call get_device() twice without decrement reference count
for the device. And it is the same if bcm_sf2_mdio_register() succeeds but
fails in bcm_sf2_sw_probe(), or if bcm_sf2_sw_probe() succeeds. If the
reference count has not decremented to zero, the dev related resource will
not be freed.

So remove the get_device() in bcm_sf2_mdio_register(), and call
put_device() if mdiobus_alloc() or mdiobus_register() fails and in
bcm_sf2_mdio_unregister() to solve the issue.

And as Simon suggested, unwind from errors for bcm_sf2_mdio_register() and
just return 0 if it succeeds to make it cleaner.

Fixes: 461cd1b03e32 ("net: dsa: bcm_sf2: Register our slave MDIO bus")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
Suggested-by: Simon Horman <horms@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://lore.kernel.org/r/20231011032419.2423290-1-ruanjinjie@huawei.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Jinjie Ruan and committed by
Jakub Kicinski
61b40cef dda5e1ee

+15 -9
+15 -9
drivers/net/dsa/bcm_sf2.c
··· 617 617 dn = of_find_compatible_node(NULL, NULL, "brcm,unimac-mdio"); 618 618 priv->master_mii_bus = of_mdio_find_bus(dn); 619 619 if (!priv->master_mii_bus) { 620 - of_node_put(dn); 621 - return -EPROBE_DEFER; 620 + err = -EPROBE_DEFER; 621 + goto err_of_node_put; 622 622 } 623 623 624 - get_device(&priv->master_mii_bus->dev); 625 624 priv->master_mii_dn = dn; 626 625 627 626 priv->slave_mii_bus = mdiobus_alloc(); 628 627 if (!priv->slave_mii_bus) { 629 - of_node_put(dn); 630 - return -ENOMEM; 628 + err = -ENOMEM; 629 + goto err_put_master_mii_bus_dev; 631 630 } 632 631 633 632 priv->slave_mii_bus->priv = priv; ··· 683 684 } 684 685 685 686 err = mdiobus_register(priv->slave_mii_bus); 686 - if (err && dn) { 687 - mdiobus_free(priv->slave_mii_bus); 688 - of_node_put(dn); 689 - } 687 + if (err && dn) 688 + goto err_free_slave_mii_bus; 690 689 690 + return 0; 691 + 692 + err_free_slave_mii_bus: 693 + mdiobus_free(priv->slave_mii_bus); 694 + err_put_master_mii_bus_dev: 695 + put_device(&priv->master_mii_bus->dev); 696 + err_of_node_put: 697 + of_node_put(dn); 691 698 return err; 692 699 } 693 700 ··· 701 696 { 702 697 mdiobus_unregister(priv->slave_mii_bus); 703 698 mdiobus_free(priv->slave_mii_bus); 699 + put_device(&priv->master_mii_bus->dev); 704 700 of_node_put(priv->master_mii_dn); 705 701 } 706 702