Re: [PATCH] net-sysfs: Fix memory leak in netdev_register_kobject

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Mar 19, 2019 at 08:17:01PM +0800, wanghai (M) wrote:
> 在 2019/3/19 18:30, Andy Shevchenko 写道:
> > On Tue, Mar 19, 2019 at 11:19:24AM +0800, wanghai (M) wrote:
> > > 在 2019/3/19 0:19, Andy Shevchenko 写道:

> > > If device_add(dev) or register_queue_kobjects(ndev) fails,
> > > In register_netdevice(), dev-> reg_state = NETREG_UNINITIALIZED and returns
> > > an error, causing put_device(&dev-> dev) -> ..-> kobject_cleanup() not to be
> > > called.
> > OK, that's true, but your patch is wrong.
> > See error handling in device_create_groups_vargs() for example.

> Hi Andy
> Thanks for your advice. I understand the problem of my patch.
> Can you help me see if it can be fixed like this?
> 
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index 4ff661f..6fe5b8e 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -1745,17 +1745,21 @@ int netdev_register_kobject(struct net_device *ndev)
> 
>         error = device_add(dev);
>         if (error)
> -               return error;
> +               goto device_add_error;

This part seems correct now.

>         error = register_queue_kobjects(ndev);
> -       if (error) {
> -               device_del(dev);
> -               return error;
> -       }
> +       if (error)
> +               goto register_error;

Yes, seems correct order here, i.e. device_del() followed by put_device().

> 
>         pm_runtime_set_memalloc_noio(dev, true);
> 

> +out:

Better

	return 0;

>         return error;
> +register_error:

Better to describe what you will do here, i.e.

error_device_del:

> +       device_del(dev);

> +device_add_error:

Ditto.

error_put_device:

> +       put_device(dev);

> +       goto out;

Better

	return error;

>  }

-- 
With Best Regards,
Andy Shevchenko