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

[PATCH v2] 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 v2:
- Fix NULL pointer dereference in probe error path by checking if channels                                                                       list is initialized before cleanup
- Fix race condition by calling free_irq() before tasklet_kill() to ensure
  IRQ handlers cannot reschedule tasklets after cleanup
---
 drivers/dma/at_hdmac.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index e5b30a57c477..9048b56b082f 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1940,6 +1940,19 @@ 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;
+
+	list_for_each_entry_safe(chan, _chan, &atdma->dma_device.channels,
+			device_node) {
+		/* Disable interrupts */
+		atc_disable_chan_irq(atdma, chan->chan_id);
+		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;
@@ -2109,6 +2122,8 @@ static int __init at_dma_probe(struct platform_device *pdev)
 err_memset_pool_create:
 	dma_pool_destroy(atdma->lli_pool);
 err_desc_pool_create:
+	if (atdma->dma_device.channels.next != NULL)
+		at_dma_cleanup_channels(atdma);
 	free_irq(platform_get_irq(pdev, 0), atdma);
 err_irq:
 	clk_disable_unprepare(atdma->clk);
@@ -2125,16 +2140,11 @@ static void at_dma_remove(struct platform_device *pdev)
 		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);
+	at_dma_cleanup_channels(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);
-	}
+	dma_pool_destroy(atdma->memset_pool);
+	dma_pool_destroy(atdma->lli_pool);
 
 	clk_disable_unprepare(atdma->clk);
 }
-- 
2.25.1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect teardown order in the probe error path leaves the driver vulnerable to Use-After-Free and pool corruption.
- [Low] Unused local variables `chan` and `_chan` are left in `at_dma_remove()`, which will cause compiler warnings.
--

commit 2676b9fc85315dd8a64edbcb2460a143edbcb102
Author: Hongling Zeng <zenghongling@kylinos.cn>

dma: at_hdmac: Fix use-after-free by proper tasklet cleanup
    
This commit fixes a use-after-free vulnerability by properly ordering the
teardown path to free IRQs before killing tasklets and freeing other
resources.

Closes: https://lore.kernel.org/all/20260604073945.54B311F00898@smtp.kernel.org/

> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index e5b30a57c477a..9048b56b082f4 100644
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c
[ ... ]
> @@ -2109,6 +2122,8 @@ static int __init at_dma_probe(struct platform_device *pdev)
>  err_memset_pool_create:
>  	dma_pool_destroy(atdma->lli_pool);
>  err_desc_pool_create:
> +	if (atdma->dma_device.channels.next != NULL)
> +		at_dma_cleanup_channels(atdma);
>  	free_irq(platform_get_irq(pdev, 0), atdma);

[Severity: High]
Does this sequence in the probe error path violate the cleanup order mentioned
in the commit message?

The commit message states that free_irq() should happen first, then tasklets
should be killed, and finally other resources should be freed.

In this error path, if probe fails at dma_async_device_register(), we call
dma_pool_destroy() on both pools, then at_dma_cleanup_channels() kills the
tasklets, and free_irq() is called last:

err_dma_async_device_register:
	dma_pool_destroy(atdma->memset_pool);
err_memset_pool_create:
	dma_pool_destroy(atdma->lli_pool);
err_desc_pool_create:
	if (atdma->dma_device.channels.next != NULL)
		at_dma_cleanup_channels(atdma);
	free_irq(platform_get_irq(pdev, 0), atdma);

Could an active IRQ handler still fire and schedule tasklets, causing a
use-after-free on the already-destroyed pool memory?

>  err_irq:
>  	clk_disable_unprepare(atdma->clk);
> @@ -2125,16 +2140,11 @@ static void at_dma_remove(struct platform_device *pdev)
>  		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);

[Severity: Low]
This isn't a bug, but does at_dma_remove() still declare the local variables
chan and _chan after this refactoring?

The loop previously using these variables was moved to
at_dma_cleanup_channels(), so leaving them declared at the top of
at_dma_remove() might cause -Wunused-variable compiler warnings:

static void at_dma_remove(struct platform_device *pdev)
{
	struct at_dma		*atdma = platform_get_drvdata(pdev);
	struct dma_chan		*chan, *_chan;

> +	at_dma_cleanup_channels(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);
> -	}
> +	dma_pool_destroy(atdma->memset_pool);
> +	dma_pool_destroy(atdma->lli_pool);
>  
>  	clk_disable_unprepare(atdma->clk);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604101912.318118-1-zenghongling@kylinos.cn?part=1