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

[Andy Shevchenko]

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