[PATCH v4] dma: at_hdmac: Fix use-after-free by proper tasklet cleanup

[PATCH v4] dma: at_hdmac: Fix use-after-free by proper tasklet cleanup

[Hongling Zeng]

Current cleanup paths have a use-after-free vulnerability:
- vchan_init() creates tasklets that access at_dma_chan memory
- free_irq() only waits for IRQ handler, NOT tasklets
- atdma is devm-managed and freed after probe/remove
- Running tasklets accessing freed memory → Use-After-Free!

The fix requires careful ordering:
- free_irq() FIRST to synchronize with running IRQ handlers and prevent
  them from scheduling new tasklets
- Then kill tasklets to wait for already-scheduled ones to complete
- Only then free other resources

Fixes: ac803b56860f ("dmaengine: at_hdmac: Convert driver to use virt-dma")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260604073945.54B311F00898@smtp.kernel.org/
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
Change in v4:
  - Fix error path fallthrough causing double-free_irq()
  - Use channel iteration index (chan_id not initialized before registration)
  - Remove unnecessary defensive checks
---
 drivers/dma/at_hdmac.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index e5b30a57c477..044a0fb38b7a 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1940,6 +1940,20 @@ static void at_dma_off(struct at_dma *atdma)
 		cpu_relax();
 }
 
+static void at_dma_cleanup_channels(struct at_dma *atdma)
+{
+	struct dma_chan *chan, *_chan;
+	int i = 0;
+
+	list_for_each_entry_safe(chan, _chan, &atdma->dma_device.channels,
+			device_node) {
+		/* Disable interrupts */
+		atc_disable_chan_irq(atdma, i++);
+		tasklet_kill(&to_at_dma_chan(chan)->vc.task);
+		list_del(&chan->device_node);
+	}
+}
+
 static int __init at_dma_probe(struct platform_device *pdev)
 {
 	struct at_dma		*atdma;
@@ -2105,12 +2119,17 @@ static int __init at_dma_probe(struct platform_device *pdev)
 err_of_dma_controller_register:
 	dma_async_device_unregister(&atdma->dma_device);
 err_dma_async_device_register:
+	free_irq(platform_get_irq(pdev, 0), atdma);
+	at_dma_cleanup_channels(atdma);
 	dma_pool_destroy(atdma->memset_pool);
+	dma_pool_destroy(atdma->lli_pool);
+	goto err_clk;
 err_memset_pool_create:
 	dma_pool_destroy(atdma->lli_pool);
 err_desc_pool_create:
 	free_irq(platform_get_irq(pdev, 0), atdma);
 err_irq:
+err_clk:
 	clk_disable_unprepare(atdma->clk);
 	return err;
 }
@@ -2118,23 +2137,17 @@ static int __init at_dma_probe(struct platform_device *pdev)
 static void at_dma_remove(struct platform_device *pdev)
 {
 	struct at_dma		*atdma = platform_get_drvdata(pdev);
-	struct dma_chan		*chan, *_chan;
 
 	at_dma_off(atdma);
 	if (pdev->dev.of_node)
 		of_dma_controller_free(pdev->dev.of_node);
 	dma_async_device_unregister(&atdma->dma_device);
 
-	dma_pool_destroy(atdma->memset_pool);
-	dma_pool_destroy(atdma->lli_pool);
 	free_irq(platform_get_irq(pdev, 0), atdma);
 
-	list_for_each_entry_safe(chan, _chan, &atdma->dma_device.channels,
-			device_node) {
-		/* Disable interrupts */
-		atc_disable_chan_irq(atdma, chan->chan_id);
-		list_del(&chan->device_node);
-	}
+	at_dma_cleanup_channels(atdma);
+	dma_pool_destroy(atdma->memset_pool);
+	dma_pool_destroy(atdma->lli_pool);
 
 	clk_disable_unprepare(atdma->clk);
 }
-- 
2.25.1

Re: [PATCH v4] dma: at_hdmac: Fix use-after-free by proper tasklet cleanup

[Frank Li]

On Wed, Jul 08, 2026 at 10:59:59AM +0800, Hongling Zeng wrote:
> Current cleanup paths have a use-after-free vulnerability:
> - vchan_init() creates tasklets that access at_dma_chan memory
> - free_irq() only waits for IRQ handler, NOT tasklets
> - atdma is devm-managed and freed after probe/remove
> - Running tasklets accessing freed memory → Use-After-Free!
>
> The fix requires careful ordering:
> - free_irq() FIRST to synchronize with running IRQ handlers and prevent
>   them from scheduling new tasklets
> - Then kill tasklets to wait for already-scheduled ones to complete
> - Only then free other resources
>
> Fixes: ac803b56860f ("dmaengine: at_hdmac: Convert driver to use virt-dma")
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/all/20260604073945.54B311F00898@smtp.kernel.org/
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>
> ---
> Change in v4:
>   - Fix error path fallthrough causing double-free_irq()
>   - Use channel iteration index (chan_id not initialized before registration)
>   - Remove unnecessary defensive checks
> ---
>  drivers/dma/at_hdmac.c | 31 ++++++++++++++++++++++---------
>  1 file changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index e5b30a57c477..044a0fb38b7a 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
> @@ -1940,6 +1940,20 @@ static void at_dma_off(struct at_dma *atdma)
>  		cpu_relax();
>  }
>
> +static void at_dma_cleanup_channels(struct at_dma *atdma)
> +{
> +	struct dma_chan *chan, *_chan;
> +	int i = 0;
> +
> +	list_for_each_entry_safe(chan, _chan, &atdma->dma_device.channels,
> +			device_node) {
> +		/* Disable interrupts */
> +		atc_disable_chan_irq(atdma, i++);
> +		tasklet_kill(&to_at_dma_chan(chan)->vc.task);
> +		list_del(&chan->device_node);
> +	}
> +}
> +
>  static int __init at_dma_probe(struct platform_device *pdev)
>  {
>  	struct at_dma		*atdma;
> @@ -2105,12 +2119,17 @@ static int __init at_dma_probe(struct platform_device *pdev)
>  err_of_dma_controller_register:
>  	dma_async_device_unregister(&atdma->dma_device);
>  err_dma_async_device_register:
> +	free_irq(platform_get_irq(pdev, 0), atdma);
> +	at_dma_cleanup_channels(atdma);
>  	dma_pool_destroy(atdma->memset_pool);
> +	dma_pool_destroy(atdma->lli_pool);
> +	goto err_clk;

I forget the reason why need goto here. Can you call disable_irq() or
disable hardware irq and call synchronize_irq() at free_irq() place. then
goto can fallback to below clean up code

Frank

>  err_memset_pool_create:
>  	dma_pool_destroy(atdma->lli_pool);
>  err_desc_pool_create:
>  	free_irq(platform_get_irq(pdev, 0), atdma);
>  err_irq:
> +err_clk:
>  	clk_disable_unprepare(atdma->clk);
>  	return err;
>  }
> @@ -2118,23 +2137,17 @@ static int __init at_dma_probe(struct platform_device *pdev)
>  static void at_dma_remove(struct platform_device *pdev)
>  {
>  	struct at_dma		*atdma = platform_get_drvdata(pdev);
> -	struct dma_chan		*chan, *_chan;
>
>  	at_dma_off(atdma);
>  	if (pdev->dev.of_node)
>  		of_dma_controller_free(pdev->dev.of_node);
>  	dma_async_device_unregister(&atdma->dma_device);
>
> -	dma_pool_destroy(atdma->memset_pool);
> -	dma_pool_destroy(atdma->lli_pool);
>  	free_irq(platform_get_irq(pdev, 0), atdma);
>
> -	list_for_each_entry_safe(chan, _chan, &atdma->dma_device.channels,
> -			device_node) {
> -		/* Disable interrupts */
> -		atc_disable_chan_irq(atdma, chan->chan_id);
> -		list_del(&chan->device_node);
> -	}
> +	at_dma_cleanup_channels(atdma);
> +	dma_pool_destroy(atdma->memset_pool);
> +	dma_pool_destroy(atdma->lli_pool);
>
>  	clk_disable_unprepare(atdma->clk);
>  }
> --
> 2.25.1
>

Re: [PATCH v4] dma: at_hdmac: Fix use-after-free by proper tasklet cleanup

[Hongling Zeng]

在 2026年07月09日 05:32, Frank Li 写道:
> On Wed, Jul 08, 2026 at 10:59:59AM +0800, Hongling Zeng wrote:
>> Current cleanup paths have a use-after-free vulnerability:
>> - vchan_init() creates tasklets that access at_dma_chan memory
>> - free_irq() only waits for IRQ handler, NOT tasklets
>> - atdma is devm-managed and freed after probe/remove
>> - Running tasklets accessing freed memory → Use-After-Free!
>>
>> The fix requires careful ordering:
>> - free_irq() FIRST to synchronize with running IRQ handlers and prevent
>>    them from scheduling new tasklets
>> - Then kill tasklets to wait for already-scheduled ones to complete
>> - Only then free other resources
>>
>> Fixes: ac803b56860f ("dmaengine: at_hdmac: Convert driver to use virt-dma")
>> Reported-by: sashiko-bot@kernel.org
>> Closes: https://lore.kernel.org/all/20260604073945.54B311F00898@smtp.kernel.org/
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>>
>> ---
>> Change in v4:
>>    - Fix error path fallthrough causing double-free_irq()
>>    - Use channel iteration index (chan_id not initialized before registration)
>>    - Remove unnecessary defensive checks
>> ---
>>   drivers/dma/at_hdmac.c | 31 ++++++++++++++++++++++---------
>>   1 file changed, 22 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
>> index e5b30a57c477..044a0fb38b7a 100644
>> --- a/drivers/dma/at_hdmac.c
>> +++ b/drivers/dma/at_hdmac.c
>> @@ -1940,6 +1940,20 @@ static void at_dma_off(struct at_dma *atdma)
>>   		cpu_relax();
>>   }
>>
>> +static void at_dma_cleanup_channels(struct at_dma *atdma)
>> +{
>> +	struct dma_chan *chan, *_chan;
>> +	int i = 0;
>> +
>> +	list_for_each_entry_safe(chan, _chan, &atdma->dma_device.channels,
>> +			device_node) {
>> +		/* Disable interrupts */
>> +		atc_disable_chan_irq(atdma, i++);
>> +		tasklet_kill(&to_at_dma_chan(chan)->vc.task);
>> +		list_del(&chan->device_node);
>> +	}
>> +}
>> +
>>   static int __init at_dma_probe(struct platform_device *pdev)
>>   {
>>   	struct at_dma		*atdma;
>> @@ -2105,12 +2119,17 @@ static int __init at_dma_probe(struct platform_device *pdev)
>>   err_of_dma_controller_register:
>>   	dma_async_device_unregister(&atdma->dma_device);
>>   err_dma_async_device_register:
>> +	free_irq(platform_get_irq(pdev, 0), atdma);
>> +	at_dma_cleanup_channels(atdma);
>>   	dma_pool_destroy(atdma->memset_pool);
>> +	dma_pool_destroy(atdma->lli_pool);
>> +	goto err_clk;
> I forget the reason why need goto here. Can you call disable_irq() or
> disable hardware irq and call synchronize_irq() at free_irq() place. then
> goto can fallback to below clean up code
>
> Frank
   Thank you for the suggestion. I've updated to v6 following your advice:
   - Replaced free_irq() with disable_irq() to allow fallthrough without 
goto
   - free_irq() is now called only once at err_desc_pool_create label
   The new approach ensures IRQ handler cannot schedule tasklets before 
tasklet_kill(), then free_irq() handles the actual release.

    Please help to review again.
Thanks!

>
>>   err_memset_pool_create:
>>   	dma_pool_destroy(atdma->lli_pool);
>>   err_desc_pool_create:
>>   	free_irq(platform_get_irq(pdev, 0), atdma);
>>   err_irq:
>> +err_clk:
>>   	clk_disable_unprepare(atdma->clk);
>>   	return err;
>>   }
>> @@ -2118,23 +2137,17 @@ static int __init at_dma_probe(struct platform_device *pdev)
>>   static void at_dma_remove(struct platform_device *pdev)
>>   {
>>   	struct at_dma		*atdma = platform_get_drvdata(pdev);
>> -	struct dma_chan		*chan, *_chan;
>>
>>   	at_dma_off(atdma);
>>   	if (pdev->dev.of_node)
>>   		of_dma_controller_free(pdev->dev.of_node);
>>   	dma_async_device_unregister(&atdma->dma_device);
>>
>> -	dma_pool_destroy(atdma->memset_pool);
>> -	dma_pool_destroy(atdma->lli_pool);
>>   	free_irq(platform_get_irq(pdev, 0), atdma);
>>
>> -	list_for_each_entry_safe(chan, _chan, &atdma->dma_device.channels,
>> -			device_node) {
>> -		/* Disable interrupts */
>> -		atc_disable_chan_irq(atdma, chan->chan_id);
>> -		list_del(&chan->device_node);
>> -	}
>> +	at_dma_cleanup_channels(atdma);
>> +	dma_pool_destroy(atdma->memset_pool);
>> +	dma_pool_destroy(atdma->lli_pool);
>>
>>   	clk_disable_unprepare(atdma->clk);
>>   }
>> --
>> 2.25.1
>>