Re: [PATCH v2 30/64] dmaengine: sa11x0-dma: kill vchan BH on remove

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

Pre-existing issues:
- [High] The driver's teardown sequence in `sa11x0_dma_remove()` and the probe error path is critically flawed, freeing DMA channels before disabling hardware interrupts and stopping the device tasklet.
--

--- Patch [30]: [PATCH v2 30/64] dmaengine: sa11x0-dma: kill vchan BH on remove ---
commit 34b16ea740696f927777e47c145e53481090efcc
Author: Allen Pais <[email protected]>

dmaengine: sa11x0-dma: kill vchan BH on remove

Use dma_chan_kill_bh() when tearing down virt-dma channels.

> diff --git a/drivers/dma/sa11x0-dma.c b/drivers/dma/sa11x0-dma.c
> index a6fa431530e3d..ea4357da5e39d 100644
> --- a/drivers/dma/sa11x0-dma.c
> +++ b/drivers/dma/sa11x0-dma.c
> @@ -891,7 +891,7 @@ static void sa11x0_dma_free_channels(struct dma_device *dmadev)
>  
>  	list_for_each_entry_safe(c, cn, &dmadev->channels, vc.chan.device_node) {
>  		list_del(&c->vc.chan.device_node);
> -		tasklet_kill(&c->vc.task);
> +		dma_chan_kill_bh(&c->vc.chan);
>  		kfree(c);

[Severity: High]
This isn't a bug introduced by this patch, but is a pre-existing structural
flaw in the driver's teardown sequence.

In both sa11x0_dma_remove() and the error path of sa11x0_dma_probe(),
sa11x0_dma_free_channels() is called before the device interrupts are freed
and the main device tasklet is killed:

sa11x0_dma_remove() {
	...
	sa11x0_dma_free_channels(&d->slave);
	for (pch = 0; pch < NR_PHY_CHAN; pch++)
		sa11x0_dma_free_irq(pdev, pch, &d->phy[pch]);
	tasklet_kill(&d->task);
	...
}

If an unbind happens while a DMA transfer is active, could an interrupt fire
during this window? If so, would sa11x0_dma_irq() dereference a freed channel
(p->vchan) and cause a use-after-free when attempting to acquire c->vc.lock?

Additionally, could the lockless traversal of the d->slave.channels list in
sa11x0_dma_tasklet() encounter list poisoning or a use-after-free if channels
are freed concurrently by the list_del() and kfree() operations here?

>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=30