dma: at_hdmac: Fix IRQ leak in at_dma_probe()

[PATCH] dma: at_hdmac: Fix IRQ leak in at_dma_probe()

Posted by Hongling Zeng 1 month, 1 week ago

When request_irq() succeeds but a later error occurs in at_dma_probe(),
the error handling path attempts to free the IRQ by calling
platform_get_irq() again instead of using the already stored IRQ number
in the local variable 'irq'.

Fix this by using the stored 'irq' variable directly in free_irq().

Fixes: dc78baa2b90b2 ("dmaengine: Atmel HDMAC driver")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/dma/at_hdmac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index e5b30a57c477..2a860679b9e1 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -2109,7 +2109,7 @@ static int __init at_dma_probe(struct platform_device *pdev)
 err_memset_pool_create:
 	dma_pool_destroy(atdma->lli_pool);
 err_desc_pool_create:
-	free_irq(platform_get_irq(pdev, 0), atdma);
+	free_irq(irq, atdma);
 err_irq:
 	clk_disable_unprepare(atdma->clk);
 	return err;
-- 
2.25.1

Re: [PATCH] dma: at_hdmac: Fix IRQ leak in at_dma_probe()

Posted by Frank Li 1 month ago

On Thu, May 07, 2026 at 03:57:50PM +0800, Hongling Zeng wrote:
> When request_irq() succeeds but a later error occurs in at_dma_probe(),
> the error handling path attempts to free the IRQ by calling
> platform_get_irq() again instead of using the already stored IRQ number
> in the local variable 'irq'.
>
> Fix this by using the stored 'irq' variable directly in free_irq().
>
> Fixes: dc78baa2b90b2 ("dmaengine: Atmel HDMAC driver")

Any actual problem do you meet? suppose it should be the same as 'irq'.

of course using varible irq is correct. but this patch should belong code
cleanup, not fix.

Frank

> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  drivers/dma/at_hdmac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index e5b30a57c477..2a860679b9e1 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -2109,7 +2109,7 @@ static int __init at_dma_probe(struct platform_device *pdev)
>  err_memset_pool_create:
>  	dma_pool_destroy(atdma->lli_pool);
>  err_desc_pool_create:
> -	free_irq(platform_get_irq(pdev, 0), atdma);
> +	free_irq(irq, atdma);
>  err_irq:
>  	clk_disable_unprepare(atdma->clk);
>  	return err;
> --
> 2.25.1
>

Re:Re: [PATCH] dma: at_hdmac: Fix IRQ leak in at_dma_probe()

Posted by dd 1 month ago



Hi Frank,

Thanks for your review.

You're right that in the normal case platform_get_irq() returns the

same value as 'irq'. However, this pattern triggers a smatch warning:

           drivers/dma/at_hdmac.c:2110 at_dma_probe()

           warn: 'irq' from request_irq() not released on lines: 2110.

Static analysis tools cannot guarantee that platform_get_irq() will

always match the previously requested IRQ, so they treat it as a

potential resource leak.

Using the stored 'irq' makes the error path unambiguous and silences

the warning. Therefore I think it qualifies as a small bug fix rather

than just cleanup.

Thanks,



At 2026-05-08 02:24:52, "Frank Li" <Frank.li@nxp.com> wrote:
>On Thu, May 07, 2026 at 03:57:50PM +0800, Hongling Zeng wrote:
>> When request_irq() succeeds but a later error occurs in at_dma_probe(),
>> the error handling path attempts to free the IRQ by calling
>> platform_get_irq() again instead of using the already stored IRQ number
>> in the local variable 'irq'.
>>
>> Fix this by using the stored 'irq' variable directly in free_irq().
>>
>> Fixes: dc78baa2b90b2 ("dmaengine: Atmel HDMAC driver")
>
>Any actual problem do you meet? suppose it should be the same as 'irq'.
>
>of course using varible irq is correct. but this patch should belong code
>cleanup, not fix.
>
>Frank
>
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>  drivers/dma/at_hdmac.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
>> index e5b30a57c477..2a860679b9e1 100644
>> --- a/drivers/dma/at_hdmac.c
>> +++ b/drivers/dma/at_hdmac.c
>> @@ -2109,7 +2109,7 @@ static int __init at_dma_probe(struct platform_device *pdev)
>>  err_memset_pool_create:
>>  	dma_pool_destroy(atdma->lli_pool);
>>  err_desc_pool_create:
>> -	free_irq(platform_get_irq(pdev, 0), atdma);
>> +	free_irq(irq, atdma);
>>  err_irq:
>>  	clk_disable_unprepare(atdma->clk);
>>  	return err;
>> --
>> 2.25.1
>>

Re: Re: [PATCH] dma: at_hdmac: Fix IRQ leak in at_dma_probe()

Posted by Frank Li 1 month ago

On Fri, May 08, 2026 at 09:33:19AM +0800, dd wrote:
>
>
>
> Hi Frank,
>
> Thanks for your review.

Avoid top post

>
> You're right that in the normal case platform_get_irq() returns the
>
> same value as 'irq'. However, this pattern triggers a smatch warning:
>
>            drivers/dma/at_hdmac.c:2110 at_dma_probe()
>
>            warn: 'irq' from request_irq() not released on lines: 2110.
>
> Static analysis tools cannot guarantee that platform_get_irq() will
>
> always match the previously requested IRQ, so they treat it as a
>
> potential resource leak.

It is false alarm from static analysis tools.

>
> Using the stored 'irq' makes the error path unambiguous and silences
>
> the warning. Therefore I think it qualifies as a small bug fix rather
>
> than just cleanup.

So it still belong code cleanup, no user visible impact.


Frank

>
> Thanks,
>
>
>
> At 2026-05-08 02:24:52, "Frank Li" <Frank.li@nxp.com> wrote:
> >On Thu, May 07, 2026 at 03:57:50PM +0800, Hongling Zeng wrote:
> >> When request_irq() succeeds but a later error occurs in at_dma_probe(),
> >> the error handling path attempts to free the IRQ by calling
> >> platform_get_irq() again instead of using the already stored IRQ number
> >> in the local variable 'irq'.
> >>
> >> Fix this by using the stored 'irq' variable directly in free_irq().
> >>
> >> Fixes: dc78baa2b90b2 ("dmaengine: Atmel HDMAC driver")
> >
> >Any actual problem do you meet? suppose it should be the same as 'irq'.
> >
> >of course using varible irq is correct. but this patch should belong code
> >cleanup, not fix.
> >
> >Frank
> >
> >> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> >> ---
> >>  drivers/dma/at_hdmac.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> >> index e5b30a57c477..2a860679b9e1 100644
> >> --- a/drivers/dma/at_hdmac.c
> >> +++ b/drivers/dma/at_hdmac.c
> >> @@ -2109,7 +2109,7 @@ static int __init at_dma_probe(struct platform_device *pdev)
> >>  err_memset_pool_create:
> >>  	dma_pool_destroy(atdma->lli_pool);
> >>  err_desc_pool_create:
> >> -	free_irq(platform_get_irq(pdev, 0), atdma);
> >> +	free_irq(irq, atdma);
> >>  err_irq:
> >>  	clk_disable_unprepare(atdma->clk);
> >>  	return err;
> >> --
> >> 2.25.1
> >>