Re: [PATCHv4 05/15] dmaengine: fsldma: check dma_async_device_register() return value

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Missing `fsldma_free_irqs()` in the new `dma_async_device_register()` error path causes an IRQ leak and a critical Use-After-Free vulnerability.

Pre-existing issues:
- [High] `fsl_dma_chan_remove` frees the channel structure without killing its associated tasklet.
--

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

dmaengine: fsldma: check dma_async_device_register() return value

This commit adds a check for the return value of dma_async_device_register()
in the probe path. It propagates the error instead of silently succeeding,
fixing a null pointer dereference during removal on registration failure.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1292,7 +1292,11 @@ static int fsldma_of_probe(struct platform_device *op)
>  		goto out_free_fdev;
>  	}
>  
> -	dma_async_device_register(&fdev->common);
> +	err = dma_async_device_register(&fdev->common);
> +	if (err) {
> +		dev_err(fdev->dev, "unable to register DMA device\n");
> +		goto out_free_fdev;

[Severity: Critical]
If dma_async_device_register() fails here, we jump to out_free_fdev, but
what happens to the IRQs requested just prior in fsldma_request_irqs()?

The out_free_fdev cleanup path unmaps memory and frees the fdev structure,
but doesn't appear to call fsldma_free_irqs() or free_irq(). Will this leave
active IRQ handlers pointing to freed memory, leading to a use-after-free
if an interrupt fires on the shared line?

> +	}
>  	return 0;
>  
>  out_free_fdev:

[Severity: High]
This is a pre-existing issue, but does fsl_dma_chan_remove() need to kill
the channel's tasklet before freeing the channel structure?

In the cleanup loop immediately following this label, fsl_dma_chan_remove()
is called to tear down each channel. Looking at fsl_dma_chan_remove(), it
unmaps memory and frees the chan struct via kfree() without calling
tasklet_kill(&chan->tasklet).

If a scheduled tasklet executes concurrently or immediately after the
channel is freed, will the tasklet handler dereference the freed chan
pointer?

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