[PATCH] dmaengine: fsl-edma: make channel resource allocation transactional

[PATCH] dmaengine: fsl-edma: make channel resource allocation transactional

[Pengpeng Hou]

Channel allocation ignores a required clock failure and does not reject
a failed TCD DMA-pool allocation before requesting IRQ resources.

Check both prerequisites and unwind only the channel clock that was
enabled by this allocation attempt.

Fixes: 9d831528a656 ("dmaengine: fsl-edma: extract common fsl-edma code (no changes in behavior intended)")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/dma/fsl-edma-common.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
index bb7531c456dfa..02b92d1464cca 100644
--- a/drivers/dma/fsl-edma-common.c
+++ b/drivers/dma/fsl-edma-common.c
@@ -844,13 +844,20 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
 	struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
 	int ret = 0;
 
-	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK)
-		clk_prepare_enable(fsl_chan->clk);
+	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK) {
+		ret = clk_prepare_enable(fsl_chan->clk);
+		if (ret)
+			return ret;
+	}
 
 	fsl_chan->tcd_pool = dma_pool_create("tcd_pool", chan->device->dev,
 				fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_TCD64 ?
 				sizeof(struct fsl_edma_hw_tcd64) : sizeof(struct fsl_edma_hw_tcd),
 				32, 0);
+	if (!fsl_chan->tcd_pool) {
+		ret = -ENOMEM;
+		goto err_pool;
+	}
 
 	if (fsl_chan->txirq)
 		ret = request_irq(fsl_chan->txirq, fsl_chan->irq_handler, IRQF_SHARED,
@@ -873,7 +880,9 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
 		free_irq(fsl_chan->txirq, fsl_chan);
 err_txirq:
 	dma_pool_destroy(fsl_chan->tcd_pool);
-	clk_disable_unprepare(fsl_chan->clk);
+err_pool:
+	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK)
+		clk_disable_unprepare(fsl_chan->clk);
 
 	return ret;
 }

base-commit: 08dbfad3f5040f5bdb6c529da20d6d4e81fefd72
-- 
2.50.1

Re: [PATCH] dmaengine: fsl-edma: make channel resource allocation transactional

[sashiko-bot]

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

Pre-existing issues:
- [High] Pending vchan tasklet is not synchronized before destroying the channel's DMA pool in fsl_edma_free_chan_resources, causing a use-after-free race.
--

commit 4815c2b1d9762a4bc22fe65a8797f16c559c652f
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

    dmaengine: fsl-edma: make channel resource allocation transactional

This commit modifies fsl_edma_alloc_chan_resources to properly handle clock
enable failures and DMA pool allocation failures. It checks these prerequisites
and ensures resources are correctly unwound on failure.

> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index bb7531c456dfa..02b92d1464cca 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
[ ... ]
> @@ -873,7 +880,9 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
>  		free_irq(fsl_chan->txirq, fsl_chan);
>  err_txirq:
>  	dma_pool_destroy(fsl_chan->tcd_pool);
> -	clk_disable_unprepare(fsl_chan->clk);
> +err_pool:
> +	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK)
> +		clk_disable_unprepare(fsl_chan->clk);
>  
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but while we are looking at channel resource
management, is there a use-after-free race when destroying the DMA pool in
fsl_edma_free_chan_resources()?

drivers/dma/fsl-edma-common.c:fsl_edma_free_chan_resources() {
    ...
	vchan_dma_desc_free_list(&fsl_chan->vchan, &head);
	dma_pool_destroy(fsl_chan->tcd_pool);
	fsl_chan->tcd_pool = NULL;
    ...
}

If a client driver releases an active DMA channel without explicitly calling
dmaengine_terminate_sync() beforehand, dma_release_channel() synchronizes
running tasklets and then calls free_chan_resources.

Because free_chan_resources disables the active hardware itself without
subsequently killing the tasklet again, an interrupt can fire right after the
initial synchronization but before the hardware is disabled.

This interrupt will schedule the vchan_complete tasklet, which then executes
concurrently with or after free_chan_resources destroys the tcd_pool.

Can this lead to a use-after-free when the tasklet attempts to free
descriptors into the destroyed pool via vchan_vdesc_fini()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830140925.30268-1-pengpeng@iscas.ac.cn?part=1

Re: [PATCH] dmaengine: fsl-edma: make channel resource allocation transactional

[Frank Li]

On Sun, Aug 30, 2026 at 10:09:25PM +0800, Pengpeng Hou wrote:
> Channel allocation ignores a required clock failure and does not reject
> a failed TCD DMA-pool allocation before requesting IRQ resources.
>
> Check both prerequisites and unwind only the channel clock that was
> enabled by this allocation attempt.
>
> Fixes: 9d831528a656 ("dmaengine: fsl-edma: extract common fsl-edma code (no changes in behavior intended)")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/dma/fsl-edma-common.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> index bb7531c456dfa..02b92d1464cca 100644
> --- a/drivers/dma/fsl-edma-common.c
> +++ b/drivers/dma/fsl-edma-common.c
> @@ -844,13 +844,20 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
>  	struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(chan);
>  	int ret = 0;
>
> -	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK)
> -		clk_prepare_enable(fsl_chan->clk);
> +	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK) {


Actually check is unnecesary because it is no-ops when clock is NULL.

Joy have did some work
at https://patchwork.kernel.org/project/imx/list/?series=1119587

Joy:
	Can you take this patch into your serie to avoid merge conflicts?

Frnak

> +		ret = clk_prepare_enable(fsl_chan->clk);
> +		if (ret)
> +			return ret;
> +	}
>
>  	fsl_chan->tcd_pool = dma_pool_create("tcd_pool", chan->device->dev,
>  				fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_TCD64 ?
>  				sizeof(struct fsl_edma_hw_tcd64) : sizeof(struct fsl_edma_hw_tcd),
>  				32, 0);
> +	if (!fsl_chan->tcd_pool) {
> +		ret = -ENOMEM;
> +		goto err_pool;
> +	}
>
>  	if (fsl_chan->txirq)
>  		ret = request_irq(fsl_chan->txirq, fsl_chan->irq_handler, IRQF_SHARED,
> @@ -873,7 +880,9 @@ int fsl_edma_alloc_chan_resources(struct dma_chan *chan)
>  		free_irq(fsl_chan->txirq, fsl_chan);
>  err_txirq:
>  	dma_pool_destroy(fsl_chan->tcd_pool);
> -	clk_disable_unprepare(fsl_chan->clk);
> +err_pool:
> +	if (fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_HAS_CHCLK)
> +		clk_disable_unprepare(fsl_chan->clk);
>
>  	return ret;
>  }
>
> base-commit: 08dbfad3f5040f5bdb6c529da20d6d4e81fefd72
> --
> 2.50.1
>
>