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

[PATCH] 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:
- Disable interrupts FIRST to stop new tasklets from being scheduled
- Then kill tasklets to wait for already-scheduled ones to complete
- Only then free IRQs and 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>
---
 drivers/dma/at_hdmac.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index e5b30a57c477..ac45e88e16da 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,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:
+	at_dma_cleanup_channels(atdma);
 	free_irq(platform_get_irq(pdev, 0), atdma);
 err_irq:
 	clk_disable_unprepare(atdma->clk);
@@ -2125,17 +2139,12 @@ static void at_dma_remove(struct platform_device *pdev)
 		of_dma_controller_free(pdev->dev.of_node);
 	dma_async_device_unregister(&atdma->dma_device);
 
+	at_dma_cleanup_channels(atdma);
+
 	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);
-	}
-
 	clk_disable_unprepare(atdma->clk);
 }
 
-- 
2.25.1

Re: [PATCH] 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:
- [Critical] The patch introduces a NULL pointer dereference in the error handling paths of `at_dma_probe()` by calling `at_dma_cleanup_channels()` before the channel list is initialized.
- [High] The patch introduces a flawed teardown order by calling `tasklet_kill()` before `free_irq()`, leaving a race condition open that allows an interrupt handler to schedule a tasklet after it has been killed.
--

commit 616bebad1ba493e9e69b6cbd4fcc4afc4e20c2f9
Author: Hongling Zeng <zenghongling@kylinos.cn>

dma: at_hdmac: Fix use-after-free by proper tasklet cleanup

This commit aims to fix a use-after-free vulnerability by introducing
a cleanup function that explicitly kills tasklets during device teardown.
It modifies both the probe error path and the remove function.

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
> --- a/drivers/dma/at_hdmac.c
> +++ b/drivers/dma/at_hdmac.c

[ ... ]

> @@ -2109,6 +2122,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:
> +	at_dma_cleanup_channels(atdma);
>  	free_irq(platform_get_irq(pdev, 0), atdma);

[Severity: Critical]
Does this cause a NULL pointer dereference during a probe failure?

The atdma structure is allocated earlier via devm_kzalloc(), meaning the
dma_device.channels list head is initially zeroed out.

If dma_pool_create() fails, we jump to err_desc_pool_create and call
at_dma_cleanup_channels(). However, INIT_LIST_HEAD() hasn't been called yet.

When at_dma_cleanup_channels() executes list_for_each_entry_safe() on
the uninitialized list, it will dereference the NULL pointers, resulting in
a kernel panic.

[ ... ]

> @@ -2125,17 +2139,12 @@ static void at_dma_remove(struct platform_device *pdev)
>  		of_dma_controller_free(pdev->dev.of_node);
>  	dma_async_device_unregister(&atdma->dma_device);
>  
> +	at_dma_cleanup_channels(atdma);
> +
>  	dma_pool_destroy(atdma->memset_pool);
>  	dma_pool_destroy(atdma->lli_pool);
>  	free_irq(platform_get_irq(pdev, 0), atdma);

[Severity: High]
Is it safe to place at_dma_cleanup_channels() before free_irq() here?

The commit message indicates we need to disable interrupts, kill tasklets,
and then free the IRQ. However, atc_disable_chan_irq() inside the cleanup
function only disables the hardware interrupt; it doesn't synchronize with
an already-executing IRQ handler on another CPU.

If an interrupt handler is currently running and delayed, it can schedule
a tasklet after tasklet_kill() has already returned. 

Because free_irq() is the function that actually synchronizes with running
handlers, wouldn't free_irq() need to be called before tasklet_kill() to
prevent the handler from re-queueing the tasklet and triggering the exact
use-after-free this patch is trying to fix?

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