[PATCH] dmaengine: moxart: Fix use-after-free by proper tasklet cleanup

[PATCH] dmaengine: moxart: Fix use-after-free by proper tasklet cleanup

[Hongling Zeng]

The moxart DMA driver has a use-after-free vulnerability:
- vchan_init() creates tasklets that access moxart_chan memory
- Neither probe error paths nor remove() call tasklet_kill()
- devm_free_irq() only waits for IRQ handler, NOT tasklets
- mdc is devm-managed and freed after probe/remove
- Running tasklets accessing freed memory → Use-After-Free!

Fix by adding moxart_dma_free_channels() helper that calls
tasklet_kill() for each channel, and ensuring proper teardown order:

In remove():
- devm_free_irq() FIRST to stop the IRQ handler (implies
  synchronize_irq(), preventing new tasklets from being scheduled)
- moxart_dma_free_channels() to kill already-scheduled tasklets
- Then of_dma_controller_free() and dma_async_device_unregister()
  to safely unregister the device

In probe error path:
- moxart_dma_free_channels() to kill tasklets created by vchan_init()
- devm_request_irq() is automatically released by devres, so no
  explicit devm_free_irq() is needed

Fixes: 5f9e685a0d46 ("dmaengine: Add MOXA ART DMA engine driver")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 drivers/dma/moxart-dma.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/moxart-dma.c b/drivers/dma/moxart-dma.c
index 442f5aa16031..d01eaa6b964b 100644
--- a/drivers/dma/moxart-dma.c
+++ b/drivers/dma/moxart-dma.c
@@ -553,6 +553,22 @@ static irqreturn_t moxart_dma_interrupt(int irq, void *devid)
 	return IRQ_HANDLED;
 }
 
+static void moxart_dma_free_channels(struct moxart_dmadev *mdc)
+{
+	struct moxart_chan *ch;
+	int i;
+
+	for (i = 0; i < APB_DMA_MAX_CHANNEL; i++) {
+		ch = &mdc->slave_chans[i];
+		/*
+		 * Wait for any scheduled tasklet to complete before channel
+		 * memory is freed by devres.
+		 */
+		tasklet_kill(&ch->vc.task);
+		list_del(&ch->vc.chan.device_node);
+	}
+}
+
 static int moxart_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -609,19 +625,23 @@ static int moxart_probe(struct platform_device *pdev)
 	ret = dma_async_device_register(&mdc->dma_slave);
 	if (ret) {
 		dev_err(dev, "dma_async_device_register failed\n");
-		return ret;
+		goto err_dma_register;
 	}
 
 	ret = of_dma_controller_register(node, moxart_of_xlate, mdc);
 	if (ret) {
 		dev_err(dev, "of_dma_controller_register failed\n");
 		dma_async_device_unregister(&mdc->dma_slave);
-		return ret;
+		goto err_dma_register;
 	}
 
 	dev_dbg(dev, "%s: IRQ=%u\n", __func__, irq);
 
 	return 0;
+
+err_dma_register:
+	moxart_dma_free_channels(mdc);
+	return ret;
 }
 
 static void moxart_remove(struct platform_device *pdev)
@@ -630,10 +650,12 @@ static void moxart_remove(struct platform_device *pdev)
 
 	devm_free_irq(&pdev->dev, m->irq, m);
 
-	dma_async_device_unregister(&m->dma_slave);
+	moxart_dma_free_channels(m);
 
 	if (pdev->dev.of_node)
 		of_dma_controller_free(pdev->dev.of_node);
+
+	dma_async_device_unregister(&m->dma_slave);
 }
 
 static const struct of_device_id moxart_dma_match[] = {
-- 
2.25.1

Re: [PATCH] dmaengine: moxart: 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:

New issues:
- [High] Calling list_del() on channel nodes in moxart_dma_free_channels() causes dma_async_device_unregister() to silently skip channel cleanup, permanently leaking sysfs entries, IDA tags, and per-cpu memory.

Pre-existing issues:
- [High] The DMA hardware is not halted during device removal, which can lead to memory corruption.
--

--- Patch [1]: [PATCH] dmaengine: moxart: Fix use-after-free by proper tasklet cleanup ---
commit 17dd1a2edfd46732b7bad62b5a03240b9b705283
Author: Hongling Zeng <zenghongling@kylinos.cn>

dmaengine: moxart: Fix use-after-free by proper tasklet cleanup

This commit attempts to fix a use-after-free vulnerability in the moxart
DMA driver by explicitly killing tasklets before freeing the memory. It
introduces a helper to stop tasklets and reorganizes the remove and probe
error paths.

> diff --git a/drivers/dma/moxart-dma.c b/drivers/dma/moxart-dma.c
> index 442f5aa16031f..d01eaa6b964bc 100644
> --- a/drivers/dma/moxart-dma.c
> +++ b/drivers/dma/moxart-dma.c
> @@ -553,6 +553,22 @@ static irqreturn_t moxart_dma_interrupt(int irq, void *devid)
>  	return IRQ_HANDLED;
>  }
>  
> +static void moxart_dma_free_channels(struct moxart_dmadev *mdc)
> +{
> +	struct moxart_chan *ch;
> +	int i;
> +
> +	for (i = 0; i < APB_DMA_MAX_CHANNEL; i++) {
> +		ch = &mdc->slave_chans[i];
> +		/*
> +		 * Wait for any scheduled tasklet to complete before channel
> +		 * memory is freed by devres.
> +		 */
> +		tasklet_kill(&ch->vc.task);
> +		list_del(&ch->vc.chan.device_node);

[Severity: High]
Does this code skip channel cleanup in the core DMA engine?

By calling list_del() on the channel nodes here, the channels are removed
from the DMA controller's device->channels list. When
dma_async_device_unregister() is called later in moxart_remove(), it
iterates over an empty list and silently skips calling
__dma_async_device_channel_unregister().

Won't this permanently leak sysfs entries, IDA tags, and per-cpu memory
for every DMA channel?

> +	}
> +}
> +

[ ... ]

> @@ -627,10 +647,12 @@ static int moxart_probe(struct platform_device *pdev)
>  static void moxart_remove(struct platform_device *pdev)
>  {
>  	struct moxart_dmadev *m = platform_get_drvdata(pdev);

[Severity: High]
This is a pre-existing issue, but does this remove sequence leave the DMA
hardware running?

The moxart_remove() callback does not appear to explicitly stop ongoing
hardware transfers before tearing down the driver. If a transfer is active
when the driver is unbound, the IRQ is freed and the client driver might
time out and free its DMA buffer.

Without explicitly clearing the enable bits (like APB_DMA_ENABLE) in the
hardware control registers, won't the runaway DMA hardware continue writing
asynchronously and potentially corrupt memory?

>  
>  	devm_free_irq(&pdev->dev, m->irq, m);
>  
> -	dma_async_device_unregister(&m->dma_slave);
> +	moxart_dma_free_channels(m);
>  
>  	if (pdev->dev.of_node)
>  		of_dma_controller_free(pdev->dev.of_node);
> +
> +	dma_async_device_unregister(&m->dma_slave);
>  }

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