diff mbox series

media: platform: allgro-dvt: unregister v4l2_device on the error path

Message ID 20241205020621.2051079-1-joe@pf.is.s.u-tokyo.ac.jp (mailing list archive)
State New
Headers show
Series media: platform: allgro-dvt: unregister v4l2_device on the error path | expand

Commit Message

Joe Hattori Dec. 5, 2024, 2:06 a.m. UTC
In allegro_probe(), the v4l2 device is not unregistered in the error
path, which results in a memory leak. Fix it by calling
v4l2_device_unregister() before returning error.

Fixes: d74d4e2359ec ("media: allegro: move driver out of staging")
Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp>
---
 drivers/media/platform/allegro-dvt/allegro-core.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Michael Tretter Dec. 5, 2024, 8:51 a.m. UTC | #1
Hi Joe,

On Thu, 05 Dec 2024 11:06:21 +0900, Joe Hattori wrote:
> In allegro_probe(), the v4l2 device is not unregistered in the error
> path, which results in a memory leak. Fix it by calling
> v4l2_device_unregister() before returning error.

Thanks for the patch!

Out of curiosity: How did you find the memory leak? Did you use some
specific tooling for that?

> 
> Fixes: d74d4e2359ec ("media: allegro: move driver out of staging")
> Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp>

Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>

> ---
>  drivers/media/platform/allegro-dvt/allegro-core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
> index e491399afcc9..eb03df0d8652 100644
> --- a/drivers/media/platform/allegro-dvt/allegro-core.c
> +++ b/drivers/media/platform/allegro-dvt/allegro-core.c
> @@ -3912,6 +3912,7 @@ static int allegro_probe(struct platform_device *pdev)
>  	if (ret < 0) {
>  		v4l2_err(&dev->v4l2_dev,
>  			 "failed to request firmware: %d\n", ret);
> +		v4l2_device_unregister(&dev->v4l2_dev);
>  		return ret;
>  	}
>  
> -- 
> 2.34.1
> 
>
Joe Hattori Dec. 5, 2024, 12:32 p.m. UTC | #2
Hi Michael,

On 12/5/24 17:51, Michael Tretter wrote:
> Hi Joe,
> 
> On Thu, 05 Dec 2024 11:06:21 +0900, Joe Hattori wrote:
>> In allegro_probe(), the v4l2 device is not unregistered in the error
>> path, which results in a memory leak. Fix it by calling
>> v4l2_device_unregister() before returning error.
> 
> Thanks for the patch!
> 
> Out of curiosity: How did you find the memory leak? Did you use some
> specific tooling for that?

I am currently building a tool to automatically detect refcount bugs as 
a part of my academic work, and it successfully found this one too.

It will be published soon :)
> 
>>
>> Fixes: d74d4e2359ec ("media: allegro: move driver out of staging")
>> Signed-off-by: Joe Hattori <joe@pf.is.s.u-tokyo.ac.jp>
> 
> Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>
> 
>> ---
>>   drivers/media/platform/allegro-dvt/allegro-core.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
>> index e491399afcc9..eb03df0d8652 100644
>> --- a/drivers/media/platform/allegro-dvt/allegro-core.c
>> +++ b/drivers/media/platform/allegro-dvt/allegro-core.c
>> @@ -3912,6 +3912,7 @@ static int allegro_probe(struct platform_device *pdev)
>>   	if (ret < 0) {
>>   		v4l2_err(&dev->v4l2_dev,
>>   			 "failed to request firmware: %d\n", ret);
>> +		v4l2_device_unregister(&dev->v4l2_dev);
>>   		return ret;
>>   	}
>>   
>> -- 
>> 2.34.1
>>
>>

Best,
Joe
diff mbox series

Patch

diff --git a/drivers/media/platform/allegro-dvt/allegro-core.c b/drivers/media/platform/allegro-dvt/allegro-core.c
index e491399afcc9..eb03df0d8652 100644
--- a/drivers/media/platform/allegro-dvt/allegro-core.c
+++ b/drivers/media/platform/allegro-dvt/allegro-core.c
@@ -3912,6 +3912,7 @@  static int allegro_probe(struct platform_device *pdev)
 	if (ret < 0) {
 		v4l2_err(&dev->v4l2_dev,
 			 "failed to request firmware: %d\n", ret);
+		v4l2_device_unregister(&dev->v4l2_dev);
 		return ret;
 	}