[PATCH 0/7] dmaengine: drain completion tasklets during teardown

[PATCH 0/7] dmaengine: drain completion tasklets during teardown

[Jiale Yao]

Several DMAengine drivers defer interrupt completion processing to
tasklets, but release channel resources without draining tasklets that
were already queued. Stopping or releasing an IRQ prevents new hard IRQ
handlers, but does not synchronize the deferred tasklet.

Depending on the driver, a late tasklet can access freed descriptor
storage or invoke a completion callback after its client data has been
released. Ensure each producer is stopped before killing the tasklet and
freeing the resources it can access.

Each patch is independent and keeps the synchronization local to one
driver's channel or device teardown path.

Jiale Yao (7):
  dmaengine: ep93xx: drain tasklet before freeing channel resources
  dmaengine: fsldma: drain channel tasklet during removal
  dmaengine: mmp_pdma: drain channel tasklets during removal
  dmaengine: mmp_tdma: synchronize tasklet when freeing a channel
  dmaengine: mv_xor: drain IRQ tasklet before freeing channel memory
  dmaengine: mxs-dma: synchronize tasklet when freeing a channel
  dmaengine: ste_dma40: synchronize tasklet when releasing a channel

 drivers/dma/ep93xx_dma.c | 4 +++-
 drivers/dma/fsldma.c     | 1 +
 drivers/dma/mmp_pdma.c   | 4 ++++
 drivers/dma/mmp_tdma.c   | 2 ++
 drivers/dma/mv_xor.c     | 6 ++++--
 drivers/dma/mxs-dma.c    | 1 +
 drivers/dma/ste_dma40.c  | 2 ++
 7 files changed, 17 insertions(+), 3 deletions(-)

-- 
2.34.1

[PATCH 1/7] dmaengine: ep93xx: drain tasklet before freeing channel resources

[Jiale Yao]

The DMA interrupt handler can schedule the channel tasklet. Releasing
the IRQ waits for a running interrupt handler, but it does not drain a
tasklet that was already queued.

The channel resource release path frees the software descriptors and
disables the clock while such a tasklet can still access the channel and
invoke a completion callback.

Free the IRQ first to stop new tasklet producers, then kill the tasklet
before releasing the descriptors and clock.

Fixes: 5fa29a17fabf ("dmaengine: add ep93xx DMA support")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/dma/ep93xx_dma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
index 311e55a97ba9..6134e9ca699d 100644
--- a/drivers/dma/ep93xx_dma.c
+++ b/drivers/dma/ep93xx_dma.c
@@ -1015,11 +1015,13 @@ static void ep93xx_dma_free_chan_resources(struct dma_chan *chan)
 	list_splice_init(&edmac->free_list, &list);
 	spin_unlock_irqrestore(&edmac->lock, flags);
 
+	free_irq(edmac->irq, edmac);
+	tasklet_kill(&edmac->tasklet);
+
 	list_for_each_entry_safe(desc, d, &list, node)
 		kfree(desc);
 
 	clk_disable_unprepare(edmac->clk);
-	free_irq(edmac->irq, edmac);
 }
 
 /**
-- 
2.34.1

[PATCH 2/7] dmaengine: fsldma: drain channel tasklet during removal

[Jiale Yao]

The channel IRQ schedules a tasklet that accesses the channel descriptor
lists. Device removal frees the IRQs and then releases each channel
without draining tasklets that were already queued.

Kill each channel tasklet before unmapping and freeing the channel. The
removal path has already freed all controller or channel IRQs, so no
interrupt handler can queue the tasklet again.

Fixes: 173acc7ce853 ("dmaengine: add driver for Freescale MPC85xx DMA controller")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/dma/fsldma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 22d62d958abd..0e2f84862261 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1205,6 +1205,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 
 static void fsl_dma_chan_remove(struct fsldma_chan *chan)
 {
+	tasklet_kill(&chan->tasklet);
 	irq_dispose_mapping(chan->irq);
 	list_del(&chan->common.device_node);
 	iounmap(chan->regs);
-- 
2.34.1

[PATCH 3/7] dmaengine: mmp_pdma: drain channel tasklets during removal

[Jiale Yao]

PDMA interrupt handlers schedule per-channel tasklets that process
descriptor lists, invoke callbacks and return descriptors to their
pools. Freeing the IRQs does not drain tasklets that were already queued.

After freeing all controller or channel IRQs, kill every channel tasklet
before unregistering the DMA device and releasing its managed state.

Fixes: c8acd6aa6bed ("dmaengine: mmp-pdma support")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/dma/mmp_pdma.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index ed520737882b..a371f017fc3e 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -1119,6 +1119,7 @@ static void dma_do_tasklet(struct tasklet_struct *t)
 static void mmp_pdma_remove(struct platform_device *op)
 {
 	struct mmp_pdma_device *pdev = platform_get_drvdata(op);
+	struct mmp_pdma_chan *chan;
 	struct mmp_pdma_phy *phy;
 	int i, irq = 0, irq_num = 0;
 
@@ -1141,6 +1142,9 @@ static void mmp_pdma_remove(struct platform_device *op)
 		}
 	}
 
+	list_for_each_entry(chan, &pdev->device.channels, chan.device_node)
+		tasklet_kill(&chan->tasklet);
+
 	dma_async_device_unregister(&pdev->device);
 }
 
-- 
2.34.1

[PATCH 4/7] dmaengine: mmp_tdma: synchronize tasklet when freeing a channel

[Jiale Yao]

The channel interrupt handler schedules a tasklet to invoke the transfer
completion callback. Channel resource release does not drain a tasklet
that was already queued, so a callback can run after its client state has
been released.

Disable the channel to stop the shared interrupt path from producing
more completions, release a dedicated IRQ when present, and kill the
tasklet before freeing the descriptor storage.

Fixes: c6da0ba8dfc8 ("dmaengine: mmp_tdma: add mmp tdma support")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/dma/mmp_tdma.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c
index ba03321eeff7..e0de0967d7e7 100644
--- a/drivers/dma/mmp_tdma.c
+++ b/drivers/dma/mmp_tdma.c
@@ -399,8 +399,10 @@ static void mmp_tdma_free_chan_resources(struct dma_chan *chan)
 {
 	struct mmp_tdma_chan *tdmac = to_mmp_tdma_chan(chan);
 
+	mmp_tdma_disable_chan(chan);
 	if (tdmac->irq)
 		devm_free_irq(tdmac->dev, tdmac->irq, tdmac);
+	tasklet_kill(&tdmac->tasklet);
 	mmp_tdma_free_descriptor(tdmac);
 	return;
 }
-- 
2.34.1

[PATCH 5/7] dmaengine: mv_xor: drain IRQ tasklet before freeing channel memory

[Jiale Yao]

The XOR interrupt handler schedules irq_tasklet to clean channel
descriptor lists. Both device removal and late channel setup failures
can free the coherent descriptor pool while a previously queued tasklet
still refers to it.

Release the IRQ first so no new tasklet can be queued, kill the tasklet,
and only then free the descriptor pool and DMA mappings.

Fixes: ff7b04796d98 ("dmaengine: DMA engine driver for Marvell XOR engine")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/dma/mv_xor.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 25ed61f1b089..ec746cd16960 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1013,6 +1013,9 @@ static int mv_xor_channel_remove(struct mv_xor_chan *mv_chan)
 
 	dma_async_device_unregister(&mv_chan->dmadev);
 
+	free_irq(mv_chan->irq, mv_chan);
+	tasklet_kill(&mv_chan->irq_tasklet);
+
 	dma_free_wc(dev, MV_XOR_POOL_SIZE,
 			  mv_chan->dma_desc_pool_virt, mv_chan->dma_desc_pool);
 	dma_unmap_single(dev, mv_chan->dummy_src_addr,
@@ -1025,8 +1028,6 @@ static int mv_xor_channel_remove(struct mv_xor_chan *mv_chan)
 		list_del(&chan->device_node);
 	}
 
-	free_irq(mv_chan->irq, mv_chan);
-
 	return 0;
 }
 
@@ -1162,6 +1163,7 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
 
 err_free_irq:
 	free_irq(mv_chan->irq, mv_chan);
+	tasklet_kill(&mv_chan->irq_tasklet);
 err_free_dma:
 	dma_free_wc(&pdev->dev, MV_XOR_POOL_SIZE,
 			  mv_chan->dma_desc_pool_virt, mv_chan->dma_desc_pool);
-- 
2.34.1

[PATCH 6/7] dmaengine: mxs-dma: synchronize tasklet when freeing a channel

[Jiale Yao]

The DMA interrupt handler schedules a per-channel tasklet to invoke the
completion callback. Releasing the IRQ does not drain a tasklet that was
already queued, so the callback can run after its client state has been
released.

Kill the tasklet after freeing the IRQ and before releasing the channel
command storage and clock.

Fixes: a580b8c5429a ("dmaengine: mxs-dma: add dma support for i.MX23/28")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/dma/mxs-dma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/mxs-dma.c b/drivers/dma/mxs-dma.c
index 7acb3d29dad3..9e85ba45f0fa 100644
--- a/drivers/dma/mxs-dma.c
+++ b/drivers/dma/mxs-dma.c
@@ -442,6 +442,7 @@ static void mxs_dma_free_chan_resources(struct dma_chan *chan)
 	mxs_dma_disable_chan(chan);
 
 	free_irq(mxs_chan->chan_irq, mxs_dma);
+	tasklet_kill(&mxs_chan->tasklet);
 
 	dma_free_coherent(mxs_dma->dma_device.dev, CCW_BLOCK_SIZE,
 			mxs_chan->ccw, mxs_chan->ccw_phys);
-- 
2.34.1

[PATCH 7/7] dmaengine: ste_dma40: synchronize tasklet when releasing a channel

[Jiale Yao]

DMA completion handling schedules a per-channel tasklet that processes
completed descriptors and invokes client callbacks. Channel release
terminates and frees the descriptor state without draining a tasklet
that was already queued.

After d40_free_dma() removes the channel from the interrupt lookup
tables, kill the tasklet before returning the channel to the DMAengine
core.

Fixes: 8d318a50b3d7 ("DMAENGINE: Support for ST-Ericssons DMA40 block v3")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 drivers/dma/ste_dma40.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0d9ffa3e2663..647a265590c2 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2484,6 +2484,8 @@ static void d40_free_chan_resources(struct dma_chan *chan)
 	if (err)
 		chan_err(d40c, "Failed to free channel\n");
 	spin_unlock_irqrestore(&d40c->lock, flags);
+
+	tasklet_kill(&d40c->tasklet);
 }
 
 static struct dma_async_tx_descriptor *d40_prep_memcpy(struct dma_chan *chan,
-- 
2.34.1

Re: [PATCH 1/7] dmaengine: ep93xx: drain tasklet before freeing channel resources

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: ep93xx: free_list spliced before tasklet_kill() leaks descriptors
--

commit 2aab3bd053da4cc14229fbf115862f410d9ef2f3
Author: Jiale Yao <yaojiale02@163.com>

dmaengine: ep93xx: drain tasklet before freeing channel resources

This commit frees the channel IRQ and kills the channel tasklet in
ep93xx_dma_free_chan_resources() before freeing descriptors and disabling the
clock.

> diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
> index 311e55a97ba9f..6134e9ca699dc 100644
> --- a/drivers/dma/ep93xx_dma.c
> +++ b/drivers/dma/ep93xx_dma.c
> @@ -1015,11 +1015,13 @@ static void ep93xx_dma_free_chan_resources(struct dma_chan *chan)
>  	list_splice_init(&edmac->free_list, &list);
>  	spin_unlock_irqrestore(&edmac->lock, flags);
>  
> +	free_irq(edmac->irq, edmac);
> +	tasklet_kill(&edmac->tasklet);

[Severity: High]
Should free_irq() and tasklet_kill() be called before checking edmac->active
and splicing edmac->free_list in ep93xx_dma_free_chan_resources()?

In ep93xx_dma_tasklet(), completed descriptors are moved from edmac->active
to a stack-local list under edmac->lock, and only returned to
edmac->free_list after dropping the lock:

drivers/dma/ep93xx_dma.c:ep93xx_dma_tasklet() {
    ...
	if (desc) {
		if (desc->complete) {
			/* mark descriptor complete for non cyclic case only */
			if (!test_bit(EP93XX_DMA_IS_CYCLIC, &edmac->flags))
				dma_cookie_complete(&desc->txd);
			list_splice_init(&edmac->active, &list);
		}
		dmaengine_desc_get_callback(&desc->txd, &cb);
	}
	spin_unlock_irq(&edmac->lock);

	/* Pick up the next descriptor from the queue */
	ep93xx_dma_advance_work(edmac);

	/* Now we can release all the chained descriptors */
	list_for_each_entry_safe(desc, d, &list, node) {
		dma_descriptor_unmap(&desc->txd);\
		ep93xx_dma_desc_put(edmac, desc);
	}
    ...
}

If ep93xx_dma_tasklet() is running concurrently and has already spliced
edmac->active into its local list and dropped edmac->lock,
ep93xx_dma_free_chan_resources() will splice edmac->free_list into its own
local list before waiting in tasklet_kill(&edmac->tasklet). While
tasklet_kill() waits for the tasklet to finish, ep93xx_dma_desc_put() will
place those in-flight struct ep93xx_dma_desc descriptors back onto
edmac->free_list, where they will not be freed by the subsequent kfree()
loop.

Additionally, if a tasklet scheduled by ep93xx_dma_interrupt() on
INTERRUPT_DONE has not yet entered its locked section when
ep93xx_dma_free_chan_resources() starts, edmac->active will still be
non-empty and hit BUG_ON(!list_empty(&edmac->active)) at the top of
ep93xx_dma_free_chan_resources().

Would draining the IRQ and tasklet before the BUG_ON() checks and
list_splice_init(&edmac->free_list, &list) avoid leaking descriptors and
hitting the BUG_ON()?

> +
>  	list_for_each_entry_safe(desc, d, &list, node)
>  		kfree(desc);
>  
>  	clk_disable_unprepare(edmac->clk);
> -	free_irq(edmac->irq, edmac);
>  }
>  
>  /**

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018040724.534641-1-yaojiale02@163.com?part=1

Re: [PATCH 4/7] dmaengine: mmp_tdma: synchronize tasklet when freeing a channel

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dmaengine: mmp_tdma: shared IRQ race in mmp_tdma_free_chan_resources()
- [High] dmaengine: mmp_tdma: unclocked MMIO access in mmp_tdma_free_chan_resources()
--

commit 6a728febb9516f46ef428e03caafe8fc8e1dbec8
Author: Jiale Yao <yaojiale02@163.com>

dmaengine: mmp_tdma: synchronize tasklet when freeing a channel

This commit disables the DMA channel and kills the completion tasklet in
mmp_tdma_free_chan_resources() before freeing the descriptor storage.

> diff --git a/drivers/dma/mmp_tdma.c b/drivers/dma/mmp_tdma.c
> index ba03321eeff79..e0de0967d7e70 100644
> --- a/drivers/dma/mmp_tdma.c
> +++ b/drivers/dma/mmp_tdma.c
> @@ -399,8 +399,10 @@ static void mmp_tdma_free_chan_resources(struct dma_chan *chan)
>  {
>  	struct mmp_tdma_chan *tdmac = to_mmp_tdma_chan(chan);
>  
> +	mmp_tdma_disable_chan(chan);

[Severity: High]
Can calling mmp_tdma_disable_chan() unconditionally here access TDCR while
the MMP2 Audio island clock and power domain are disabled?

In drivers/dma/mmp_tdma.c, mmp_tdma_disable_chan() performs MMIO accesses on
TDCR:

drivers/dma/mmp_tdma.c:mmp_tdma_disable_chan() {
	tdcr = readl(tdmac->reg_base + TDCR);
	tdcr |= TDCR_ABR;
	tdcr &= ~TDCR_CHANEN;
	writel(tdcr, tdmac->reg_base + TDCR);
}

On MMP2 (arch/arm/boot/dts/marvell/mmp2.dtsi), the adma0 and adma1
controllers reside in the Audio subsystem power island alongside sspa0/sspa1.
The mmp_tdma driver does not manage clocks or power domains itself; its
consumer driver in sound/soc/pxa/mmp-sspa.c controls sspa->audio_clk and the
runtime PM domain.

In sound/soc/pxa/mmp-sspa.c, asoc_mmp_sspa_probe() registers dmaengine PCM
(which requests the DMA channels) before enabling runtime PM and
sspa->audio_clk:

sound/soc/pxa/mmp-sspa.c:asoc_mmp_sspa_probe() {
	if (pdev->dev.of_node) {
		ret = devm_snd_dmaengine_pcm_register(&pdev->dev,
						      &mmp_pcm_config, 0);
		if (ret)
			return ret;
	}

	ret = devm_snd_soc_register_component(&pdev->dev, &mmp_sspa_component,
					      &mmp_sspa_dai, 1);
	if (ret)
		return ret;

	pm_runtime_enable(&pdev->dev);
	clk_prepare_enable(sspa->audio_clk);
}

And on driver unbind, asoc_mmp_sspa_remove() disables sspa->audio_clk and
runtime PM before devres releases the DMA channels:

sound/soc/pxa/mmp-sspa.c:asoc_mmp_sspa_remove() {
	clk_disable_unprepare(sspa->audio_clk);
	pm_runtime_disable(&pdev->dev);
}

When devres subsequently runs snd_dmaengine_pcm_unregister() ->
dmaengine_pcm_release_chan() -> dma_release_channel() ->
mmp_tdma_free_chan_resources() (either after asoc_mmp_sspa_remove() or if
devm_snd_soc_register_component() fails during probe), won't
mmp_tdma_disable_chan() read and write TDCR while the Audio island is powered
off and unclocked, causing an external abort or hang?

>  	if (tdmac->irq)
>  		devm_free_irq(tdmac->dev, tdmac->irq, tdmac);
> +	tasklet_kill(&tdmac->tasklet);

[Severity: High]
Does this prevent tdmac->tasklet from being scheduled again when the
controller uses a shared interrupt (tdmac->irq == 0)?

In mmp_tdma_probe(), when irq_num != chan_num, a single shared handler
mmp_tdma_int_handler() is registered on tdev and each channel has
tdmac->irq = 0:

drivers/dma/mmp_tdma.c:mmp_tdma_probe() {
	if (irq_num != chan_num) {
		irq = platform_get_irq(pdev, 0);
		ret = devm_request_irq(&pdev->dev, irq,
			mmp_tdma_int_handler, IRQF_SHARED, "tdma", tdev);
		if (ret)
			return ret;
	}

	/* initialize channel parameters */
	for (i = 0; i < chan_num; i++) {
		irq = (irq_num != chan_num) ? 0 : platform_get_irq(pdev, i);
		ret = mmp_tdma_chan_init(tdev, i, irq, type, pool);
}

When mmp_tdma_free_chan_resources() runs on such a controller:

1. Unlike mmp_tdma_terminate_all(), which calls both mmp_tdma_disable_chan()
   and mmp_tdma_enable_irq(tdmac, false), mmp_tdma_free_chan_resources() only
   calls mmp_tdma_disable_chan(), which updates TDCR without masking TDIMR or
   clearing a latched TDISR_COMP bit in TDISR.

2. Because tdmac->irq is 0, devm_free_irq() is skipped and nothing
   synchronizes with a concurrently running mmp_tdma_int_handler() ->
   mmp_tdma_chan_handler() before tasklet_kill(&tdmac->tasklet) is called.

If mmp_tdma_int_handler() -> mmp_tdma_chan_handler() is already executing on
another CPU, or if TDISR_COMP was latched before TDCR_CHANEN was cleared and
the shared interrupt fires (for instance from the sibling channel in
tdev->tdmac[]), mmp_tdma_chan_handler() will still schedule tdmac->tasklet
after tasklet_kill() and mmp_tdma_free_descriptor() have completed:

drivers/dma/mmp_tdma.c:mmp_tdma_chan_handler() {
	if (mmp_tdma_clear_chan_irq(tdmac) == 0) {
		tasklet_schedule(&tdmac->tasklet);
		return IRQ_HANDLED;
	} else
		return IRQ_NONE;
}

Wouldn't that allow dma_do_tasklet() to invoke the client callback after
mmp_tdma_free_chan_resources() returns and dereference a NULL tdmac->desc_arr
if the callback queries mmp_tdma_tx_status() -> mmp_tdma_get_pos()?

>  	mmp_tdma_free_descriptor(tdmac);
>  	return;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261018040724.534641-1-yaojiale02@163.com?part=4