Re: [PATCHv4 07/15] dmaengine: fsldma: fix request_irqs unwind freeing unregistered IRQ

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The error cleanup path in `fsl_dma_prep_memcpy` causes a Use-After-Free read when using `fsldma_free_desc_list_reverse` to free a list whose head is embedded in one of the freed elements.
- [High] Missing `tasklet_kill()` during channel teardown leads to Use-After-Free if a tasklet is pending or running.
--

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

dmaengine: fsldma: fix request_irqs unwind freeing unregistered IRQ

When fsldma_request_irqs() fails on a per-channel IRQ, the unwind
loop starts at the current index i, which calls free_irq() on the
IRQ that request_irq() just failed to register. Decrement i before
the loop to skip the failed channel.

> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index 22d62d958abd9..285c5e284d425 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -1085,7 +1085,7 @@ static int fsldma_request_irqs(struct fsldma_device *fdev)
>  	return 0;
>  
>  out_unwind:
> -	for (/* none */; i >= 0; i--) {
> +	for (i--; i >= 0; i--) {
>  		chan = fdev->chan[i];
>  		if (!chan)
>  			continue;

[Severity: High]
This is a pre-existing issue, but while reviewing the error handling I noticed
a potential use-after-free in drivers/dma/fsldma.c.

In fsl_dma_prep_memcpy(), if a descriptor allocation fails, the error path
cleans up by calling fsldma_free_desc_list_reverse() and passing the list
head embedded in the `first` descriptor:

	fsldma_free_desc_list_reverse(chan, &first->tx_list);

In fsldma_free_desc_list_reverse(), the traversal is done using:

	list_for_each_entry_safe_reverse(desc, _desc, list, node)
		fsl_dma_free_descriptor(chan, desc);

Since the list head `&first->tx_list` is embedded within the memory that will
be freed, does the list traversal macro's increment step cause a read from
freed memory when it evaluates the previous node?

[Severity: High]
This isn't a bug introduced by this patch, but I found a potential missing
cleanup in the channel teardown path in drivers/dma/fsldma.c.

In fsldma_chan_irq(), a tasklet is scheduled:

	tasklet_schedule(&chan->tasklet);

During driver unbind or error handling, fsl_dma_chan_remove() frees the DMA
channel:

	kfree(chan);

Does fsl_dma_chan_remove() need to call tasklet_kill(&chan->tasklet) before
freeing the channel memory? Even though fsldma_free_irqs() unregisters the
interrupt, if the tasklet is scheduled right before the IRQ is freed, could
the tasklet run after `chan` has been freed and cause a use-after-free?

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