[PATCH] media: imagination: e5010: release m2m device on probe failure

[PATCH] media: imagination: e5010: release m2m device on probe failure

[박명훈]

From: Myeonghun Pak <mhun512@gmail.com>

The probe path allocates the mem2mem device with v4l2_m2m_init(),
but several later failures jump directly to the v4l2_device_unregister()
unwind path. Since the driver core does not call .remove() after a failed
probe, those paths leak the m2m device.

Route post-v4l2_m2m_init() failures through the existing m2m release step
before unregistering the v4l2 device.

Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
Cc: stable@vger.kernel.org
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/media/platform/imagination/e5010-jpeg-enc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
index 42ad9ee399..4c250e2275 100644
--- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
+++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
@@ -1072,14 +1072,14 @@ static int e5010_probe(struct platform_device *pdev)
 	if (IS_ERR(e5010->core_base)) {
 		ret = PTR_ERR(e5010->core_base);
 		dev_err_probe(dev, ret, "Missing 'core' resources area\n");
-		goto fail_after_v4l2_register;
+		goto fail_after_m2m_init;
 	}
 
 	e5010->mmu_base = devm_platform_ioremap_resource_byname(pdev, "mmu");
 	if (IS_ERR(e5010->mmu_base)) {
 		ret = PTR_ERR(e5010->mmu_base);
 		dev_err_probe(dev, ret, "Missing 'mmu' resources area\n");
-		goto fail_after_v4l2_register;
+		goto fail_after_m2m_init;
 	}
 
 	e5010->last_context_run = NULL;
@@ -1089,14 +1089,14 @@ static int e5010_probe(struct platform_device *pdev)
 			       E5010_MODULE_NAME, e5010);
 	if (ret) {
 		dev_err_probe(dev, ret, "failed to register IRQ %d\n", irq);
-		goto fail_after_v4l2_register;
+		goto fail_after_m2m_init;
 	}
 
 	e5010->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(e5010->clk)) {
 		ret = PTR_ERR(e5010->clk);
 		dev_err_probe(dev, ret, "failed to get clock\n");
-		goto fail_after_v4l2_register;
+		goto fail_after_m2m_init;
 	}
 
 	pm_runtime_enable(dev);
@@ -1113,6 +1113,7 @@ static int e5010_probe(struct platform_device *pdev)
 	return 0;
 
 fail_after_video_register_device:
+fail_after_m2m_init:
 	v4l2_m2m_release(e5010->m2m_dev);
 fail_after_v4l2_register:
 	v4l2_device_unregister(&e5010->v4l2_dev);
-- 
2.50.0

Re: [PATCH] media: imagination: e5010: release m2m device on probe failure

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2706 bytes --]

Hi,

sorry for the long delay, finally getting to this one, comment below...

Le samedi 25 avril 2026 à 01:58 +0900, 박명훈 a écrit :
> From: Myeonghun Pak <mhun512@gmail.com>
> 
> The probe path allocates the mem2mem device with v4l2_m2m_init(),
> but several later failures jump directly to the v4l2_device_unregister()
> unwind path. Since the driver core does not call .remove() after a failed
> probe, those paths leak the m2m device.
> 
> Route post-v4l2_m2m_init() failures through the existing m2m release step
> before unregistering the v4l2 device.
> 
> Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
>  drivers/media/platform/imagination/e5010-jpeg-enc.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> index 42ad9ee399..4c250e2275 100644
> --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
> +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> @@ -1072,14 +1072,14 @@ static int e5010_probe(struct platform_device *pdev)
>  	if (IS_ERR(e5010->core_base)) {
>  		ret = PTR_ERR(e5010->core_base);
>  		dev_err_probe(dev, ret, "Missing 'core' resources area\n");
> -		goto fail_after_v4l2_register;
> +		goto fail_after_m2m_init;
>  	}
>  
>  	e5010->mmu_base = devm_platform_ioremap_resource_byname(pdev, "mmu");
>  	if (IS_ERR(e5010->mmu_base)) {
>  		ret = PTR_ERR(e5010->mmu_base);
>  		dev_err_probe(dev, ret, "Missing 'mmu' resources area\n");
> -		goto fail_after_v4l2_register;
> +		goto fail_after_m2m_init;
>  	}
>  
>  	e5010->last_context_run = NULL;
> @@ -1089,14 +1089,14 @@ static int e5010_probe(struct platform_device *pdev)
>  			       E5010_MODULE_NAME, e5010);
>  	if (ret) {
>  		dev_err_probe(dev, ret, "failed to register IRQ %d\n", irq);
> -		goto fail_after_v4l2_register;
> +		goto fail_after_m2m_init;
>  	}
>  
>  	e5010->clk = devm_clk_get(dev, NULL);
>  	if (IS_ERR(e5010->clk)) {
>  		ret = PTR_ERR(e5010->clk);
>  		dev_err_probe(dev, ret, "failed to get clock\n");
> -		goto fail_after_v4l2_register;
> +		goto fail_after_m2m_init;
>  	}
>  
>  	pm_runtime_enable(dev);
> @@ -1113,6 +1113,7 @@ static int e5010_probe(struct platform_device *pdev)
>  	return 0;
>  
>  fail_after_video_register_device:
> +fail_after_m2m_init:

Can't you simply use fail_after_video_register_device label ?

Nicolas

>  	v4l2_m2m_release(e5010->m2m_dev);
>  fail_after_v4l2_register:
>  	v4l2_device_unregister(&e5010->v4l2_dev);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] media: imagination: e5010: release m2m device on probe failure

[Devarsh Thakkar]

Hi Myeonghun, Nicolas,

On 29/09/26 00:58, Nicolas Dufresne wrote:
> Hi,
> 
> sorry for the long delay, finally getting to this one, comment below...
> 

Apologies from my side too. I too completely missed it.

> Le samedi 25 avril 2026 à 01:58 +0900, 박명훈 a écrit :
>> From: Myeonghun Pak <mhun512@gmail.com>
>>
>> The probe path allocates the mem2mem device with v4l2_m2m_init(),
>> but several later failures jump directly to the v4l2_device_unregister()
>> unwind path. Since the driver core does not call .remove() after a failed
>> probe, those paths leak the m2m device.
>>
>> Route post-v4l2_m2m_init() failures through
Brandonm
Could you drop the existing m2m release step
>> before unregistering the v4l2 device.
>>
>> Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
>> ---
>>   drivers/media/platform/imagination/e5010-jpeg-enc.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
>> index 42ad9ee399..4c250e2275 100644
>> --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
>> +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
>> @@ -1072,14 +1072,14 @@ static int e5010_probe(struct platform_device *pdev)
>>   	if (IS_ERR(e5010->core_base)) {
>>   		ret = PTR_ERR(e5010->core_base);
>>   		dev_err_probe(dev, ret, "Missing 'core' resources area\n");
>> -		goto fail_after_v4l2_register;
>> +		goto fail_after_m2m_init;

I'd prefer naming convention as goto fail_after_v4l2_m2m_init to be 
consistent w.r.t naming for other labels (goto fail_after_v4l2_register)
>>   	}
>>   
>>   	e5010->mmu_base = devm_platform_ioremap_resource_byname(pdev, "mmu");
>>   	if (IS_ERR(e5010->mmu_base)) {
>>   		ret = PTR_ERR(e5010->mmu_base);
>>   		dev_err_probe(dev, ret, "Missing 'mmu' resources area\n");
>> -		goto fail_after_v4l2_register;
>> +		goto fail_after_m2m_init;

goto fail_after_v4l2_m2m_init;

>>   	}
>>   
>>   	e5010->last_context_run = NULL;
>> @@ -1089,14 +1089,14 @@ static int e5010_probe(struct platform_device *pdev)
>>   			       E5010_MODULE_NAME, e5010);                            >>   	if (ret) {
>>   		dev_err_probe(dev, ret, "failed to register IRQ %d\n", irq);
>> -		goto fail_after_v4l2_register;
>> +		goto fail_after_m2m_init;
>>   	}
>>   
>>   	e5010->clk = devm_clk_get(dev, NULL);
>>   	if (IS_ERR(e5010->clk)) {
>>   		ret = PTR_ERR(e5010->clk);
>>   		dev_err_probe(dev, ret, "failed to get clock\n");
>> -		goto fail_after_v4l2_register;
>> +		goto fail_after_m2m_init;

goto fail_after_v4l2_m2m_init;

>>   	}
>>   
>>   	pm_runtime_enable(dev);
>> @@ -1113,6 +1113,7 @@ static int e5010_probe(struct platform_device *pdev)
>>   	return 0;
>>   
>>   fail_after_video_register_device:

I don't think we need above label anymore.

>> +fail_after_m2m_init:

fail_after_v4l2_m2m_init;
> 
> Can't you simply use fail_after_video_register_device label ?
> 

Actually it was a bit of mislabeling from my end when I authored this 
originally as below :

         ret = video_register_device(e5010->vdev, VFL_TYPE_VIDEO, 0); 

          if (ret) { 

                 dev_err_probe(dev, ret, "failed to register video 
device\n");
                 goto fail_after_video_register_device; -> This should 
be fail_after_v4l2_m2_init too
         }

I think same thing I commented here too [0] which seems to address 
similar problem.

Basically you don't need fail_after_video_register_device label here, as 
there is nothing much after it which needs and intead user here goto 
fail_after_v4l2_m2m_init

[0]:https://lore.kernel.org/all/39a03dcb-bebd-4794-a2bf-ff1b56cb632b@ti.com/#t

Regards
Devarsh


> Nicolas
> 
>>   	v4l2_m2m_release(e5010->m2m_dev);
>>   fail_after_v4l2_register:
>>   	v4l2_device_unregister(&e5010->v4l2_dev);