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

[PATCH v3] 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
---
Change in v3:
- Reorder probe error path to match remove() cleanup order
- Keep defensive NULL check for channels cleanup
- Remove unused variables in at_dma_remove()
- Ensure consistent order: free_irq → cleanup_channels → dma_pool_destroy
---
 drivers/dma/at_hdmac.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index e5b30a57c477..ac1ba21e3428 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;
@@ -2105,11 +2118,15 @@ 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);
 err_memset_pool_create:
 	dma_pool_destroy(atdma->lli_pool);
 err_desc_pool_create:
 	free_irq(platform_get_irq(pdev, 0), atdma);
+	if (atdma->dma_device.channels.next != NULL)
+		at_dma_cleanup_channels(atdma);
 err_irq:
 	clk_disable_unprepare(atdma->clk);
 	return err;
@@ -2118,23 +2135,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);
+	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 v3] 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] The probe error path executes `free_irq()` and `at_dma_cleanup_channels()` twice due to a missing return or goto statement after the `err_dma_async_device_register` label.
- [High] `at_dma_cleanup_channels()` relies on uninitialized `chan->chan_id` values on early probe error paths, causing it to fail to disable interrupts.
--

commit 393b3802c91585b2038e624d6fb8dec3e442b9db
Author: Hongling Zeng <zenghongling@kylinos.cn>

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

This commit reorders the cleanup paths in the at_hdmac driver to fix a
use-after-free vulnerability. It ensures free_irq() is called before
killing tasklets and freeing memory to prevent running tasklets from
accessing freed memory.

Link: 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..ac1ba21e34287 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);

[Severity: High]
Will this successfully disable interrupts for all channels if probe fails
early?

It looks like chan->chan_id is populated by the DMA engine core inside
dma_async_device_register(). If probe fails before registration is complete,
chan->chan_id will still be zero from the allocation.

This would result in repeatedly attempting to disable the interrupt for
channel 0, leaving the other channels enabled. Since the global IRQ handler
was just removed by free_irq(), could pending interrupts on these active
channels cause an unhandled interrupt storm?

> +		tasklet_kill(&to_at_dma_chan(chan)->vc.task);
> +		list_del(&chan->device_node);
> +	}
> +}

[ ... ]

> @@ -2105,11 +2118,15 @@ 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);

[Severity: High]
Is a goto or return statement missing here?

If execution reaches err_dma_async_device_register, it appears it will fall
through to err_desc_pool_create below, where free_irq() and
at_dma_cleanup_channels() are called a second time.

Could calling free_irq() twice for the same dev_id trigger a kernel warning
for trying to free an already-free IRQ?

>  	dma_pool_destroy(atdma->memset_pool);
>  err_memset_pool_create:
>  	dma_pool_destroy(atdma->lli_pool);
>  err_desc_pool_create:
>  	free_irq(platform_get_irq(pdev, 0), atdma);
> +	if (atdma->dma_device.channels.next != NULL)
> +		at_dma_cleanup_channels(atdma);
>  err_irq:
>  	clk_disable_unprepare(atdma->clk);
>  	return err;

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

[PATCH v3] 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
---
Change in v3:
- Reorder probe error path to match remove() cleanup order
- Keep defensive NULL check for channels cleanup
- Remove unused variables in at_dma_remove()
- Ensure consistent order: free_irq → cleanup_channels → dma_pool_destroy
---
 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

[PATCH v3] 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
---
Change in v3:
- Fix double free_irq() in probe error path by using goto to skip
  lower error labels after upper cleanup handles IRQ and channels
- Fix double at_dma_cleanup_channels() call in probe error path
- Replace incorrect channels.next != NULL check (always true after
  INIT_LIST_HEAD) with proper list_empty() check
- Consolidate at_dma_cleanup_channels() helper used by both probe
---
 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 v3] dma: at_hdmac: Fix use-after-free by proper tasklet cleanup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Meaningful code and commit message mismatch: The v3 changelog explicitly claims to keep a defensive NULL check, but no such check exists in the code.
--

--- Patch [1]: [PATCH v3] dma: at_hdmac: Fix use-after-free by proper tasklet  cleanup ---
commit e05118b092320af0ba75bbb64dc2f10dd2eff233
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 reordering the probe and
remove cleanup paths. It ensures that free_irq is called before killing
tasklets, preventing IRQ handlers from scheduling new tasklets on freed memory.

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

> Change in v3:
> - Reorder probe error path to match remove() cleanup order
> - Keep defensive NULL check for channels cleanup

[Severity: Medium]
This isn't a bug in the code, but the changelog states a defensive NULL
check was kept for the channels cleanup.

Looking at the newly added cleanup function in the diff:

> 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
[ ... ]
> +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) {

Was the defensive check accidentally dropped in at_dma_cleanup_channels,
or does the commit message just need to be updated?

While the code appears safe because the error paths were correctly reordered
to avoid reaching this function with an uninitialized list, the changelog
mismatch might cause confusion.

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Mismatch between commit message and implementation regarding list_empty() check
--

--- Patch [1]: [PATCH v3] dma: at_hdmac: Fix use-after-free by proper tasklet  cleanup ---
commit e5bf642ad51db020b1e407d27b314f3478d2609a
Author: Hongling Zeng <zenghongling@kylinos.cn>

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

This commit addresses a use-after-free vulnerability in the cleanup and probe
error paths of the at_hdmac driver. It ensures free_irq() is called before
killing tasklets and freeing other resources, properly synchronizing with
running IRQ handlers.

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

> Change in v3:
[ ... ]
> - Replace incorrect channels.next != NULL check (always true after
>   INIT_LIST_HEAD) with proper list_empty() check
[ ... ]
> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
> index e5b30a57c477a..044a0fb38b7a9 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) {

[Severity: Medium]
This isn't a bug, but the v3 changelog explicitly mentions replacing an
incorrect channels.next check with a proper list_empty() check. Does this
function actually implement that check?

While list_for_each_entry_safe() inherently handles empty lists correctly,
making an explicit check functionally unnecessary, there appears to be a
direct contradiction between the stated changes and the final code.

Should the commit message be updated to reflect that the check was simply
removed entirely, or was the list_empty() check accidentally omitted here?

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