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

[Frank Li]

On Tue, Jul 14, 2026 at 11:30:46AM +0800, Hongling Zeng wrote:
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Hongling Zeng <zenghongling@xxxxxxxxxx>
>
> ---
>  Change in v2:
>  - The DMA core's dma_async_device_unregister() handles all channel
>    cleanup including sysfs entries, IDA tags, and per-cpu memory, so
>    we don't call list_del() on the channel nodes.
> ---
>  drivers/dma/moxart-dma.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/moxart-dma.c b/drivers/dma/moxart-dma.c
> index 442f5aa16031..a2481960e870 100644
> --- a/drivers/dma/moxart-dma.c
> +++ b/drivers/dma/moxart-dma.c
> @@ -553,6 +553,21 @@ 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);
> +	}
> +}
> +
>  static int moxart_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -609,19 +624,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 +649,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);

Please use  dmaenginem_async_device_register() to fix this problem

Frank

>  }
>
>  static const struct of_device_id moxart_dma_match[] = {
> --
> 2.25.1
>