Re: [PATCH v2 39/64] dmaengine: fsl-edma-common: kill vchan BH on teardown

[email protected]
Newsgroups dev.linux.lists.imx,org.kernel.vger.dmaengine
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Bypassed DMA channel teardown leading to resource leaks and Use-After-Free via dangling sysfs nodes.
--

--- Patch [39]: [PATCH v2 39/64] dmaengine: fsl-edma-common: kill vchan BH on teardown ---
commit 72f422b1de100bcd764808c0c9305bd937347a17
Author: Allen Pais <[email protected]>

dmaengine: fsl-edma-common: kill vchan BH on teardown

This commit updates virt-dma cleanup to use dma_chan_kill_bh() instead of
tasklet_kill() when tearing down channels in fsl_edma_cleanup_vchan().

> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index bb7531c456dfa..70b40e60c39a0 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -915,7 +915,7 @@ void fsl_edma_cleanup_vchan(struct dma_device *dmadev)
>  	list_for_each_entry_safe(chan, _chan,
>  				&dmadev->channels, vchan.chan.device_node) {
>  		list_del(&chan->vchan.chan.device_node);

[Severity: Critical]
This is a pre-existing issue, but does unlinking the channel here bypass
the core DMA engine channel teardown?

During driver removal (for example, in fsl_edma_remove() or
mcf_edma_remove()), calling list_del() before dma_async_device_unregister()
means the core unregister function will see an empty dmadev->channels list.

If the channels are unlinked prematurely, dma_async_device_unregister()
skips __dma_async_device_channel_unregister() for all channels. Can this
leave dangling sysfs nodes active and leak resources like IDAs or percpu
memory, potentially leading to a use-after-free if devres subsequently
frees the underlying channel structures while the sysfs files remain
accessible?

(Note: This same unlinking pattern before unregister also appears to exist
in mtk_cqdma_remove() and sprd_dma_remove()).

> -		tasklet_kill(&chan->vchan.task);
> +		dma_chan_kill_bh(&chan->vchan.chan);
>  	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=39
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.