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

[Masahiro Yamada]

On Mon, Mar 16, 2020 at 1:59 AM Marek Vasut <marek.vasut at 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 at gmail.com>
> Cc: Joe Hershberger <joe.hershberger at ni.com>
> Cc: Masahiro Yamada <yamada.masahiro at 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
>

-- 
Best Regards
Masahiro Yamada