Re: [PATCH 05/12] net: smc911x: Fix potential memleak() in init fail path

On Mon, Mar 16, 2020 at 1:59 AM Marek Vasut marek.vasut@gmail.com wrote:

Fix memleak in the init fail path, where if allocation or registration of MDIO bus fails, then ethernet interface is not unregistered and the private data are not freed, yet the probe function reports a failure.

Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Joe Hershberger joe.hershberger@ni.com Cc: Masahiro Yamada yamada.masahiro@socionext.com

---

drivers/net/smc911x.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c index 81f8f0d017..44cb45af61 100644 --- a/drivers/net/smc911x.c +++ b/drivers/net/smc911x.c @@ -249,7 +249,7 @@ int smc911x_initialize(u8 dev_num, int base_addr)

    dev = calloc(1, sizeof(*dev));
    if (!dev) {

•           return -1;
  

•           return -ENODEV;
    }
  

Our convention is to return -ENOMEM when memory allocation fails.

If you like to do some additional cleanups here, you can remove the unneeded braces around the single statement, which will fix the checkpatch warning.

    dev->iobase = base_addr;

@@ -283,15 +283,23 @@ int smc911x_initialize(u8 dev_num, int base_addr) #if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) int retval; struct mii_dev *mdiodev = mdio_alloc();

•   if (!mdiodev)
  

•   if (!mdiodev) {
  

•           eth_unregister(dev);
  

•           free(dev);
            return -ENOMEM;
  

•   }
  

•   strncpy(mdiodev->name, dev->name, MDIO_NAME_LEN);
    mdiodev->read = smc911x_miiphy_read;
    mdiodev->write = smc911x_miiphy_write;
  
    retval = mdio_register(mdiodev);
  

•   if (retval < 0)
  

•   if (retval < 0) {
  

•           mdio_free(mdiodev);
  

•           eth_unregister(dev);
  

•           free(dev);
            return retval;
  

Using "goto <label>" is a general tip to simplify the failure path.

•   }
  

#endif

    return 1;

-- 2.25.0