Re: [PATCH v2 44/64] dmaengine: mtk-hsdma: kill vchan BH on teardown

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

Pre-existing issues:
- [High] Killing channel bottom-halves before disabling hardware interrupts allows pending work to be maliciously re-queued and executed after channel memory is freed.
- [High] Manual removal of virtual channels from the device list bypasses the DMA core's cleanup, leaking per-CPU memory and sysfs resources.
--

--- Patch [44]: [PATCH v2 44/64] dmaengine: mtk-hsdma: kill vchan BH on teardown ---
commit 2d206408161f8be2e557e668a6b7dd498f1ec43d
Author: Allen Pais <[email protected]>
Subject: dmaengine: mtk-hsdma: kill vchan BH on teardown

This commit replaces the tasklet cleanup with dma_chan_kill_bh() for virt-dma
cleanup in the mtk-hsdma driver's teardown path.

> diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
> index a43412ff5eddc..1c98fcedf3acd 100644
> --- a/drivers/dma/mediatek/mtk-hsdma.c
> +++ b/drivers/dma/mediatek/mtk-hsdma.c
> @@ -1020,7 +1020,7 @@ static void mtk_hsdma_remove(struct platform_device *pdev)
>  		vc = &hsdma->vc[i];
>  
>  		list_del(&vc->vc.chan.device_node);

[Severity: High]
This is a pre-existing issue, but does removing the channel from the device
list here bypass the DMA core's cleanup?

The core function dma_async_device_unregister() relies on iterating over the
device->channels list to execute __dma_async_device_channel_unregister(),
which frees the per-CPU memory and unregisters the sysfs devices. Because the
list is emptied preemptively before calling dma_async_device_unregister(),
the core cleanup loop might be bypassed, which could lead to leaking per-cpu
variables and orphaned sysfs nodes.

> -		tasklet_kill(&vc->vc.task);
> +		dma_chan_kill_bh(&vc->vc.chan);
>  	}
>  
>  	/* Disable DMA interrupt */

[Severity: High]
This is also a pre-existing issue, but does killing the channel bottom-halves
before disabling hardware interrupts allow pending work to be maliciously
re-queued?

mtk_hsdma_remove() terminates the asynchronous workers here, but the interrupt
is disabled and synchronized later in the function:

	/* Disable DMA interrupt */
	mtk_dma_write(hsdma, MTK_HSDMA_INT_ENABLE, 0);

	/* Waits for any pending IRQ handlers to complete */
	synchronize_irq(hsdma->irq);

If an interrupt triggers or is handled concurrently during this window, the
ISR (mtk_hsdma_irq) could complete descriptors and call
vchan_cookie_complete(), which re-schedules the channel's bottom-half onto
the workqueue. Since the driver continues unbinding and eventually frees the
channel structures, the re-scheduled work might execute later and access
freed memory.

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