[PATCH] dmaengine: img-mdc: Fix runtime PM usage counter leak

[PATCH] dmaengine: img-mdc: Fix runtime PM usage counter leak

[Ruoyu Wang]

pm_runtime_get_sync() leaves the IMG MDC device's usage counter
incremented when runtime resume fails. mdc_alloc_chan_resources() returns
that error to the DMA core, so the channel's client count is not
incremented and mdc_free_chan_resources() is not called to drop the
reference. Repeated allocation attempts can therefore accumulate usage
references and prevent runtime suspend.

Use pm_runtime_resume_and_get() so a failed resume does not retain a
usage reference while successful allocations remain paired with
mdc_free_chan_resources(). DMA core only treats negative return values as
allocation failures, so the helper's zero success return preserves
behavior.

This issue was found by a static analysis checker and confirmed by manual
source review.

Fixes: 56d355e6f586 ("dmaengine: img-mdc: Add runtime PM")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/dma/img-mdc-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/img-mdc-dma.c b/drivers/dma/img-mdc-dma.c
index b3765ba1580308..a3192d49b6c860 100644
--- a/drivers/dma/img-mdc-dma.c
+++ b/drivers/dma/img-mdc-dma.c
@@ -738,7 +738,7 @@ static int mdc_alloc_chan_resources(struct dma_chan *chan)
 	struct mdc_chan *mchan = to_mdc_chan(chan);
 	struct device *dev = mdma2dev(mchan->mdma);
 
-	return pm_runtime_get_sync(dev);
+	return pm_runtime_resume_and_get(dev);
 }
 
 static void mdc_free_chan_resources(struct dma_chan *chan)
-- 
2.51.0

Re: [PATCH] dmaengine: img-mdc: Fix runtime PM usage counter leak

[sashiko-bot]

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

Pre-existing issues:
- [High] Hardware register access before clock enablement during device probe.
- [High] NULL pointer dereference when calculating residue for cyclic transfers.
--

--- Patch [1]: [PATCH] dmaengine: img-mdc: Fix runtime PM usage counter leak ---
commit ab74c845981a8cdefe6ab65f16f64bbdc3a65623
Author: Ruoyu Wang <ruoyuw560@gmail.com>

dmaengine: img-mdc: Fix runtime PM usage counter leak

pm_runtime_get_sync() leaves the IMG MDC device's usage counter
incremented when runtime resume fails. This patch replaces it with
pm_runtime_resume_and_get() so a failed resume does not retain a usage
reference, preventing runtime PM usage counter leaks.

> diff --git a/drivers/dma/img-mdc-dma.c b/drivers/dma/img-mdc-dma.c
> index b3765ba158030..a3192d49b6c86 100644
> --- a/drivers/dma/img-mdc-dma.c
> +++ b/drivers/dma/img-mdc-dma.c
> @@ -738,7 +738,7 @@ static int mdc_alloc_chan_resources(struct dma_chan *chan)
>  	struct mdc_chan *mchan = to_mdc_chan(chan);
>  	struct device *dev = mdma2dev(mchan->mdma);
>  
> -	return pm_runtime_get_sync(dev);
> +	return pm_runtime_resume_and_get(dev);
>  }
>  
>  static void mdc_free_chan_resources(struct dma_chan *chan)

[Severity: High]
This is a pre-existing issue, but I noticed a potential hardware register
access before clock enablement during device probe:

drivers/dma/img-mdc-dma.c:mdc_dma_probe() {
    ...
	mdma->clk = devm_clk_get(&pdev->dev, "sys");
	if (IS_ERR(mdma->clk))
		return PTR_ERR(mdma->clk);

	dma_cap_zero(mdma->dma_dev.cap_mask);
	dma_cap_set(DMA_SLAVE, mdma->dma_dev.cap_mask);
	dma_cap_set(DMA_PRIVATE, mdma->dma_dev.cap_mask);
	dma_cap_set(DMA_CYCLIC, mdma->dma_dev.cap_mask);
	dma_cap_set(DMA_MEMCPY, mdma->dma_dev.cap_mask);

	val = mdc_readl(mdma, MDC_GLOBAL_CONFIG_A);
    ...
}

In mdc_dma_probe(), MDC_GLOBAL_CONFIG_A is read to determine hardware
capabilities immediately after fetching the clock, but before the clock is
ever enabled. The clock is explicitly enabled only later via
img_mdc_runtime_resume() or by PM runtime. 

If the bootloader has not left the clock enabled, could accessing the
unclocked peripheral's registers cause a system hang or synchronous external
abort?

[Severity: High]
This is also a pre-existing issue, but is it possible to encounter a NULL
pointer dereference when calculating the residue for cyclic transfers?

drivers/dma/img-mdc-dma.c:mdc_tx_status() {
    ...
		bytes = mdesc->list_xfer_size;
		ldesc = mdesc->list;
		for (i = 0; i < cmds; i++) {
			bytes -= ldesc->xfer_size + 1;
			ldesc = ldesc->next_desc;
		}
    ...
}

In mdc_tx_status(), the cmds variable is computed by adding unhandled hardware
completions to mdesc->list_cmds_done. In cyclic mode, the CPU-side descriptor
list is not circular, so the last descriptor's next_desc is NULL.

If the interrupt handler is delayed due to high system load, cmds might exceed
the list length. Under these conditions, wouldn't the loop traverse past the
end of the list, causing ldesc to become NULL and dereferencing it on the
subsequent loop iteration's ldesc->xfer_size read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813153143.3953303-1-ruoyuw560@gmail.com?part=1

Re: [PATCH] dmaengine: img-mdc: Fix runtime PM usage counter leak

[Frank Li]

On Thu, Aug 13, 2026 at 11:31:43PM +0800, Ruoyu Wang wrote:
> [You don't often get email from ruoyuw560@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> pm_runtime_get_sync() leaves the IMG MDC device's usage counter
> incremented when runtime resume fails. mdc_alloc_chan_resources() returns
> that error to the DMA core, so the channel's client count is not
> incremented and mdc_free_chan_resources() is not called to drop the
> reference. Repeated allocation attempts can therefore accumulate usage
> references and prevent runtime suspend.
>
> Use pm_runtime_resume_and_get() so a failed resume does not retain a
> usage reference while successful allocations remain paired with
> mdc_free_chan_resources(). DMA core only treats negative return values as
> allocation failures, so the helper's zero success return preserves
> behavior.
>
> This issue was found by a static analysis checker and confirmed by manual
> source review.
>
> Fixes: 56d355e6f586 ("dmaengine: img-mdc: Add runtime PM")
> Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/img-mdc-dma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/img-mdc-dma.c b/drivers/dma/img-mdc-dma.c
> index b3765ba1580308..a3192d49b6c860 100644
> --- a/drivers/dma/img-mdc-dma.c
> +++ b/drivers/dma/img-mdc-dma.c
> @@ -738,7 +738,7 @@ static int mdc_alloc_chan_resources(struct dma_chan *chan)
>         struct mdc_chan *mchan = to_mdc_chan(chan);
>         struct device *dev = mdma2dev(mchan->mdma);
>
> -       return pm_runtime_get_sync(dev);
> +       return pm_runtime_resume_and_get(dev);
>  }
>
>  static void mdc_free_chan_resources(struct dma_chan *chan)
> --
> 2.51.0
>