Re: [PATCHv2 2/9] dmaengine: mv_xor: fix use-after-free in probe error path

[sashiko-bot@kernel.org]

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

New issues:
- [High] Calling `tasklet_kill()` before `free_irq()` in `mv_xor_channel_remove()` leaves a race window where an already-executing interrupt handler can reschedule the tasklet, resulting in a use-after-free.

Pre-existing issues:
- [High] Missing `tasklet_kill()` in the `mv_xor_channel_add()` error path leaves a pending tasklet that can access freed memory if initialization fails.
--

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

dmaengine: mv_xor: fix use-after-free in probe error path

This commit attempts to fix a use-after-free in the probe error path by
masking hardware interrupts and calling tasklet_kill() at the start of
mv_xor_channel_remove(). This is intended to ensure no new interrupts can
schedule the tasklet during teardown.

> diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
> index 25ed61f1b0899..8e8902bbda9ac 100644
> --- a/drivers/dma/mv_xor.c
> +++ b/drivers/dma/mv_xor.c
[ ... ]
> @@ -1011,6 +1019,9 @@ static int mv_xor_channel_remove(struct mv_xor_chan *mv_chan)
>  	struct dma_chan *chan, *_chan;
>  	struct device *dev = mv_chan->dmadev.dev;
>  
> +	mv_chan_mask_interrupts(mv_chan);
> +	tasklet_kill(&mv_chan->irq_tasklet);

[Severity: High]
Could calling tasklet_kill() here, before free_irq(), leave a race window?

While mv_chan_mask_interrupts() prevents new interrupts from being raised by
the hardware, it does not stop an interrupt handler that is already
executing on another CPU.

If tasklet_kill() runs concurrently while the ISR is executing but before
it calls tasklet_schedule(), tasklet_kill() will see the tasklet is neither
scheduled nor running, and return immediately. The ISR will then schedule
the tasklet.

Later in mv_xor_channel_remove():

	free_irq(mv_chan->irq, mv_chan);

free_irq() waits for the ISR to finish but does not cancel the newly
scheduled tasklet. When the mv_chan structure is subsequently freed via
devres, might the pending tasklet execute on freed memory?

Should tasklet_kill() be called after free_irq() ensures no ISR is running?

> +
>  	dma_async_device_unregister(&mv_chan->dmadev);
>  
>  	dma_free_wc(dev, MV_XOR_POOL_SIZE,

[Severity: High]
This is a pre-existing issue, but does the mv_xor_channel_add() error path
also need a tasklet_kill() call?

During mv_xor_channel_add(), the self-test functions submit DMA transactions
that trigger real hardware interrupts, scheduling mv_chan->irq_tasklet.

If a self-test times out or dma_async_device_register() fails, the code
jumps to the err_free_irq label, which calls free_irq() but omits
tasklet_kill().

Since the mv_chan structure is devm-allocated, it will be automatically
freed when the probe function returns an error. Could the pending tasklet
then execute on freed memory?

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