Re: [PATCH] hwmon: Fix possible memleak in __hwmon_device_register()

[Guenter Roeck]

On Tue, Oct 12, 2021 at 07:27:58PM +0800, Yang Yingliang wrote:
> I got memory leak as follows when doing fault injection test:
> 
> unreferenced object 0xffff888102740438 (size 8):
>   comm "27", pid 859, jiffies 4295031351 (age 143.992s)
>   hex dump (first 8 bytes):
>     68 77 6d 6f 6e 30 00 00                          hwmon0..
>   backtrace:
>     [<00000000544b5996>] __kmalloc_track_caller+0x1a6/0x300
>     [<00000000df0d62b9>] kvasprintf+0xad/0x140
>     [<00000000d3d2a3da>] kvasprintf_const+0x62/0x190
>     [<000000005f8f0f29>] kobject_set_name_vargs+0x56/0x140
>     [<00000000b739e4b9>] dev_set_name+0xb0/0xe0
>     [<0000000095b69c25>] __hwmon_device_register+0xf19/0x1e50 [hwmon]
>     [<00000000a7e65b52>] hwmon_device_register_with_info+0xcb/0x110 [hwmon]
>     [<000000006f181e86>] devm_hwmon_device_register_with_info+0x85/0x100 [hwmon]
>     [<0000000081bdc567>] tmp421_probe+0x2d2/0x465 [tmp421]
>     [<00000000502cc3f8>] i2c_device_probe+0x4e1/0xbb0
>     [<00000000f90bda3b>] really_probe+0x285/0xc30
>     [<000000007eac7b77>] __driver_probe_device+0x35f/0x4f0
>     [<000000004953d43d>] driver_probe_device+0x4f/0x140
>     [<000000002ada2d41>] __device_attach_driver+0x24c/0x330
>     [<00000000b3977977>] bus_for_each_drv+0x15d/0x1e0
>     [<000000005bf2a8e3>] __device_attach+0x267/0x410
> 
> When device_register() returns an error, the name allocated in
> dev_set_name() will be leaked, the put_device() should be used
> instead of calling hwmon_dev_release() to give up the device
> reference, then the name will be freed in kobject_cleanup().
> 
> Reported-by: Hulk Robot <hulkci@xxxxxxxxxx>
> Fixes: bab2243ce189 ("hwmon: Introduce hwmon_device_register_with_groups")
> Signed-off-by: Yang Yingliang <yangyingliang@xxxxxxxxxx>

Good catch. Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/hwmon.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 8d3b1dae31df..3501a3ead4ba 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -796,8 +796,10 @@ __hwmon_device_register(struct device *dev, const char *name, void *drvdata,
>  	dev_set_drvdata(hdev, drvdata);
>  	dev_set_name(hdev, HWMON_ID_FORMAT, id);
>  	err = device_register(hdev);
> -	if (err)
> -		goto free_hwmon;
> +	if (err) {
> +		put_device(hdev);
> +		goto ida_remove;
> +	}
>  
>  	INIT_LIST_HEAD(&hwdev->tzdata);
>