[PATCH] drivers: hv: vmbus: Fix possible memory leak when device_register() failed

[PATCH] drivers: hv: vmbus: Fix possible memory leak when device_register() failed

[Nir Levy]

The reference must be released when device_register(&child_device_obj->device) failed.
Fix this by adding 'put_device()' in the error handling path.
---
 drivers/hv/vmbus_drv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 8b2e413bf19c..e592c481f7ae 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -2082,6 +2082,7 @@ int vmbus_device_register(struct hv_device *child_device_obj)
 	ret = device_register(&child_device_obj->device);
 	if (ret) {
 		pr_err("Unable to register child device\n");
+		put_device(&child_device_obj->device);
 		return ret;
 	}
 
-- 
2.34.1

Re: [PATCH] drivers: hv: vmbus: Fix possible memory leak when device_register() failed

[Praveen Kumar]

On 11/22/2022 5:47 AM, Nir Levy wrote:
> The reference must be released when device_register(&child_device_obj->device) failed.
> Fix this by adding 'put_device()' in the error handling path.
> ---
>  drivers/hv/vmbus_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 8b2e413bf19c..e592c481f7ae 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -2082,6 +2082,7 @@ int vmbus_device_register(struct hv_device *child_device_obj)
>  	ret = device_register(&child_device_obj->device);
>  	if (ret) {
>  		pr_err("Unable to register child device\n");
> +		put_device(&child_device_obj->device);

I think this patch is not required, as device_add (device_register->device_add) will put_device for any failure within. 
Please do share the specific flow which you think might need this fix. Thanks. 

>  		return ret;
>  	}
>  

Regards,

~Praveen.

RE: [PATCH] drivers: hv: vmbus: Fix possible memory leak when device_register() failed

[Michael Kelley (LINUX)]

From: Praveen Kumar <kumarpraveen@linux.microsoft.com> Sent: Tuesday, November 22, 2022 4:01 AM
> 
> On 11/22/2022 5:47 AM, Nir Levy wrote:
> > The reference must be released when device_register(&child_device_obj->device)
> failed.
> > Fix this by adding 'put_device()' in the error handling path.
> > ---
> >  drivers/hv/vmbus_drv.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index 8b2e413bf19c..e592c481f7ae 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -2082,6 +2082,7 @@ int vmbus_device_register(struct hv_device
> *child_device_obj)
> >  	ret = device_register(&child_device_obj->device);
> >  	if (ret) {
> >  		pr_err("Unable to register child device\n");
> > +		put_device(&child_device_obj->device);
> 
> I think this patch is not required, as device_add (device_register->device_add) will
> put_device for any failure within.
> Please do share the specific flow which you think might need this fix. Thanks.
> 

The put_device() done in the exit paths for device_add() pairs with the
get_device() done at the very beginning of device_add().   The put_device()
proposed by this patch *is* needed to properly clean up.

But someone else (Yang Yingliang <yangyingliang@huawei.com>) has already
submitted a patch to add the put_device().   It's currently in the hyperv-fixes
tree waiting to merge with Linus' tree.  See https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git/commit/?h=hyperv-fixes&id=25c94b051592c010abe92c85b0485f1faedc83f3.

Michael