Re: [PATCH v3 2/4] dmaengine: use dma_chan_name() helper to get per-channel device name

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing tasklet_kill() in driver removal path leads to Use-After-Free.
- [High] Devres pool leak in error path of channel resource allocation.
--

commit 46c4d3dece5bd1126d02c64f04aca18a9510a860
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: use dma_chan_name() helper to get per-channel device name

This patch updates several DMA engine drivers to use the dma_chan_name() helper
instead of open-coding the channel name logic. This improves consistency and
simplifies the code across the subsystem.

[ ... ]

> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> index 386e85cd4882a..6b29e15b0616c 100644
> --- a/drivers/dma/mmp_pdma.c
> +++ b/drivers/dma/mmp_pdma.c
> @@ -565,7 +565,7 @@ static int mmp_pdma_alloc_chan_resources(struct dma_chan *dchan)
>  	if (chan->desc_pool)
>  		return 1;
>  
> -	chan->desc_pool = dma_pool_create(dev_name(&dchan->dev->device),
> +	chan->desc_pool = dma_pool_create(dma_chan_name(dchan),
>  					  chan->dev,
>  					  sizeof(struct mmp_pdma_desc_sw),
>  					  __alignof__(struct mmp_pdma_desc_sw),

[Severity: High]
This is a pre-existing issue, but while reviewing this file's DMA resource
management, I noticed a potential use-after-free during driver unbind.

In mmp_pdma_chan_init(), a tasklet is initialized for each DMA channel using
tasklet_setup(&chan->tasklet, dma_do_tasklet), and the channel structures are
allocated via devm_kzalloc().

However, mmp_pdma_remove() unregisters the DMA engine but does not call
tasklet_kill() to cancel pending tasklets. Because there is no explicit
teardown of the tasklets, will pending or executing tasklets continue running
after devres automatically frees the channel memory, resulting in a
use-after-free if the driver is unbound while a tasklet is scheduled?

[ ... ]

> diff --git a/drivers/dma/stm32/stm32-mdma.c b/drivers/dma/stm32/stm32-mdma.c
> index e3bbdc9ee36ed..c274638e919c2 100644
> --- a/drivers/dma/stm32/stm32-mdma.c
> +++ b/drivers/dma/stm32/stm32-mdma.c
> @@ -1484,7 +1484,7 @@ static int stm32_mdma_alloc_chan_resources(struct dma_chan *c)
>  	struct stm32_mdma_device *dmadev = stm32_mdma_get_dev(chan);
>  	int ret;
>  
> -	chan->desc_pool = dmam_pool_create(dev_name(&c->dev->device),
> +	chan->desc_pool = dmam_pool_create(dma_chan_name(c),
>  					   c->device->dev,
>  					   sizeof(struct stm32_mdma_hwdesc),
>  					  __alignof__(struct stm32_mdma_hwdesc),

[Severity: High]
This isn't a bug introduced by this patch, but looking at the error handling
path immediately following this pool creation, is there a devres leak here?

When dmam_pool_create() is called, it binds the devres release action to the
DMA controller device (c->device->dev), rather than the channel lifecycle.

If the subsequent pm_runtime_resume_and_get() call or stm32_mdma_disable_chan()
fail, stm32_mdma_alloc_chan_resources() returns an error without destroying
the newly allocated pool via dmam_pool_destroy().

Since the DMA engine core assumes the allocation function cleans up after
itself on failure, will repeated allocation failures endlessly leak devres pool
entries and memory on the controller device until the driver is unloaded?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-dma-chan_dev-rename-v3-0-685601b0b788@nxp.com?part=2