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?
On Fri, Sep 04, 2026 at 05:49:44PM -0400, Frank.Li@oss.nxp.com wrote:
> Use the existing dma_chan_name() helper to obtain the per-channel device
> name instead of open-coding: dev_name(chan->dev->device) /
> dev_name(chan2dev())
>
> Simplify the code and improve consistency.
>
> No functional change intended.
...
> @@ -507,7 +507,7 @@ static int idma64_alloc_chan_resources(struct dma_chan *chan)
> struct idma64_chan *idma64c = to_idma64_chan(chan);
>
> /* Create a pool of consistent memory blocks for hardware descriptors */
> - idma64c->pool = dma_pool_create(dev_name(chan2dev(chan)),
> + idma64c->pool = dma_pool_create(dma_chan_name(chan),
> chan->device->dev,
> sizeof(struct idma64_lli), 8, 0);
For example here it may now be condensed to less LoC:
idma64c->pool = dma_pool_create(dma_chan_name(chan), chan->device->dev,
sizeof(struct idma64_lli), 8, 0);
Consider that in case you will need a new version (no need
to address it separately).
@@ -760,7 +760,7 @@ static int jz4780_dma_alloc_chan_resources(struct dma_chan *chan)
{
struct jz4780_dma_chan *jzchan = to_jz4780_dma_chan(chan);
- jzchan->desc_pool = dma_pool_create(dev_name(&chan->dev->device),
+ jzchan->desc_pool = dma_pool_create(dma_chan_name(chan),
chan->device->dev,
JZ_DMA_DESC_BLOCK_SIZE,
PAGE_SIZE, 0);
@@ -531,7 +531,7 @@ static int dma_chan_alloc_chan_resources(struct dma_chan *dchan)
}
/* LLI address must be aligned to a 64-byte boundary */
- chan->desc_pool = dma_pool_create(dev_name(chan2dev(chan)),
+ chan->desc_pool = dma_pool_create(dma_chan_name(dchan),
chan->chip->dev,
sizeof(struct axi_dma_lli),
64, 0);
@@ -507,7 +507,7 @@ static int idma64_alloc_chan_resources(struct dma_chan *chan)
struct idma64_chan *idma64c = to_idma64_chan(chan);
/* Create a pool of consistent memory blocks for hardware descriptors */
- idma64c->pool = dma_pool_create(dev_name(chan2dev(chan)),
+ idma64c->pool = dma_pool_create(dma_chan_name(chan),
chan->device->dev,
sizeof(struct idma64_lli), 8, 0);
if (!idma64c->pool) {
@@ -282,7 +282,7 @@ static int ls2x_dma_alloc_chan_resources(struct dma_chan *chan)
struct ls2x_dma_chan *lchan = to_ldma_chan(chan);
/* Create a pool of consistent memory blocks for hardware descriptors */
- lchan->pool = dma_pool_create(dev_name(chan2dev(chan)),
+ lchan->pool = dma_pool_create(dma_chan_name(chan),
chan->device->dev, PAGE_SIZE,
__alignof__(struct ls2x_dma_hw_desc), 0);
if (!lchan->pool) {
@@ -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),
@@ -268,7 +268,7 @@ static int st_fdma_alloc_chan_res(struct dma_chan *chan)
struct st_fdma_chan *fchan = to_st_fdma_chan(chan);
/* Create the dma pool for descriptor allocation */
- fchan->node_pool = dma_pool_create(dev_name(&chan->dev->device),
+ fchan->node_pool = dma_pool_create(dma_chan_name(chan),
fchan->fdev->dev,
sizeof(struct st_fdma_hw_node),
__alignof__(struct st_fdma_hw_node),
@@ -1126,7 +1126,7 @@ static int stm32_dma3_alloc_chan_resources(struct dma_chan *c)
goto err_put_sync;
}
- chan->lli_pool = dmam_pool_create(dev_name(&c->dev->device), c->device->dev,
+ chan->lli_pool = dmam_pool_create(dma_chan_name(c), c->device->dev,
sizeof(struct stm32_dma3_hwdesc),
__alignof__(struct stm32_dma3_hwdesc), SZ_64K);
if (!chan->lli_pool) {
@@ -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),
@@ -11,7 +11,7 @@ TRACE_EVENT(tegra_dma_tx_status,
TP_PROTO(struct dma_chan *dc, dma_cookie_t cookie, struct dma_tx_state *state),
TP_ARGS(dc, cookie, state),
TP_STRUCT__entry(
- __string(chan, dev_name(&dc->dev->device))
+ __string(chan, dma_chan_name(dc))
__field(dma_cookie_t, cookie)
__field(__u32, residue)
),
@@ -28,7 +28,7 @@ TRACE_EVENT(tegra_dma_complete_cb,
TP_PROTO(struct dma_chan *dc, int count, void *ptr),
TP_ARGS(dc, count, ptr),
TP_STRUCT__entry(
- __string(chan, dev_name(&dc->dev->device))
+ __string(chan, dma_chan_name(dc))
__field(int, count)
__field(void *, ptr)
),
@@ -45,7 +45,7 @@ TRACE_EVENT(tegra_dma_isr,
TP_PROTO(struct dma_chan *dc, int irq),
TP_ARGS(dc, irq),
TP_STRUCT__entry(
- __string(chan, dev_name(&dc->dev->device))
+ __string(chan, dma_chan_name(dc))
__field(int, irq)
),
TP_fast_assign(