Re: [PATCH] dma: fsldma: convert to platform_get_irq_optional()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing tasklet_kill() in channel removal causes use-after-free of channel structure.
- [High] Ignored return value of dma_async_device_register() causes silent probe failure and later kernel panic.
--

commit a1cff40cb09109b8d2ad16457c9eff5f0adadc62
Author: Rosen Penev <rosenp@gmail.com>

dma: fsldma: convert to platform_get_irq_optional()

Replace the per-controller irq_of_parse_and_map() call with
platform_get_irq_optional(). The controller IRQ is optional — when
absent (-ENXIO) the driver falls back to per-channel IRQs.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..a98a7798fd180 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c

[ ... ]

> @@ -1300,7 +1309,6 @@ static int fsldma_of_probe(struct platform_device *op)
>  		if (fdev->chan[i])
>  			fsl_dma_chan_remove(fdev->chan[i]);
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does fsl_dma_chan_remove()
safely handle scheduled tasklets?

During driver removal, the driver calls fsldma_free_irqs() which executes
free_irq(). While free_irq() waits for currently executing interrupt
handlers to finish, it does not stop or wait for scheduled tasklets.

Immediately after, fsl_dma_chan_remove() frees the channel structure:

drivers/dma/fsldma.c:fsl_dma_chan_remove() {
	irq_dispose_mapping(chan->irq);
	list_del(&chan->common.device_node);
	iounmap(chan->regs);
	kfree(chan);
}

If an interrupt scheduled the tasklet just before free_irq() was called,
the tasklet could execute asynchronously on freed memory, leading to a
use-after-free when dma_do_tasklet() dereferences the chan pointer.

Should there be a call to tasklet_kill(&chan->tasklet) before freeing
the channel?

[Severity: High]
This is also a pre-existing issue, but is it safe to ignore the return
value of dma_async_device_register() during the probe?

Looking at fsldma_of_probe(), the driver unconditionally returns 0
after attempting registration:

drivers/dma/fsldma.c:fsldma_of_probe() {
	...
	err = fsldma_request_irqs(fdev);
	if (err) {
		dev_err(fdev->dev, "unable to request IRQs\n");
		goto out_free_fdev;
	}

	dma_async_device_register(&fdev->common);
	return 0;
}

If dma_async_device_register() fails, the global_node list head remains
zero-initialized, but the driver still successfully binds.

Later during device teardown, fsldma_of_remove() blindly unregisters:

drivers/dma/fsldma.c:fsldma_of_remove() {
	...
	dma_async_device_unregister(&fdev->common);
	...
}

Would this cause dma_async_device_unregister() to call list_del_rcu() on
a NULL pointer, resulting in a kernel panic?

> -	irq_dispose_mapping(fdev->irq);
>  	iounmap(fdev->regs);
>  out_free:
>  	kfree(fdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603191951.5729-1-rosenp@gmail.com?part=1