[PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe

[PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe

[Ruoyu Wang]

pm_runtime_get_sync() increments a device's usage counter even when it
fails. sprd_dma_probe() currently jumps directly to controller clock
cleanup on that error, bypassing both pm_runtime_put_noidle() and
pm_runtime_disable(). This can happen if the preceding unchecked
pm_runtime_set_active() fails and the following runtime-resume attempt
also returns an error.

Enter the existing runtime-PM unwind path instead. This drops the
reference without idling the partially initialized device, disables
runtime PM, and then releases the controller clocks. The success path
and propagated error code are unchanged.

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

Fixes: 9b3b8171f7f4 ("dmaengine: sprd: Add Spreadtrum DMA driver")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/dma/sprd-dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 087fea3af2e411..19b32a23c882de 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -1212,7 +1212,7 @@ static int sprd_dma_probe(struct platform_device *pdev)
 
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (ret < 0)
-		goto err_rpm;
+		goto err_register;
 
 	ret = dma_async_device_register(&sdev->dma_dev);
 	if (ret < 0) {
@@ -1234,7 +1234,6 @@ static int sprd_dma_probe(struct platform_device *pdev)
 err_register:
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
-err_rpm:
 	sprd_dma_disable(sdev);
 	return ret;
 }
-- 
2.51.0

Re: [PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe

[sashiko-bot]

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

Pre-existing issues:
- [High] Premature IRQ registration and incorrect devres teardown order leading to UAF or bus errors.
- [High] The interrupt handler accesses hardware registers without checking if the device is active.
--

--- Patch [1]: [PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe ---
commit 274e07c9f2493c369c243df16cab2b58e4834d15
Author: Ruoyu Wang <ruoyuw560@gmail.com>

dmaengine: sprd: Fix runtime PM reference leak in probe

The commit modifies the error handling path in sprd_dma_probe() to
correctly unwind runtime PM state. If pm_runtime_get_sync() fails, it
now jumps to err_register to drop the reference without idling the
partially initialized device, and disables runtime PM before turning off
clocks.

> diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
> index 087fea3af2e41..19b32a23c882d 100644
> --- a/drivers/dma/sprd-dma.c
> +++ b/drivers/dma/sprd-dma.c
> @@ -1212,7 +1212,7 @@ static int sprd_dma_probe(struct platform_device *pdev)
>  
>  	ret = pm_runtime_get_sync(&pdev->dev);
>  	if (ret < 0)
> -		goto err_rpm;
> +		goto err_register;

[Severity: High]
This is a pre-existing issue, but does the devm cleanup order risk a NULL
pointer dereference or accessing unmapped memory on probe failure?

Looking at sprd_dma_probe(), the IRQ is requested before the device IO
memory is mapped:

drivers/dma/sprd-dma.c:sprd_dma_probe() {
    ...
	if (sdev->irq > 0) {
		ret = devm_request_irq(&pdev->dev, sdev->irq, dma_irq_handle,
				       0, "sprd_dma", (void *)sdev);
    ...
	sdev->glb_base = devm_platform_ioremap_resource(pdev, 0);
    ...
}

If an interrupt is already pending on the line, it could fire immediately
upon unmasking. This would cause dma_irq_handle() to read from
sdev->glb_base before it is initialized, resulting in a NULL pointer
dereference.

Additionally, because devm_request_irq() is called before
devm_platform_ioremap_resource(), devres will unmap the memory before
freeing the IRQ during probe failure teardown. If an interrupt fires during
this window, could the handler access unmapped memory?


[Severity: High]
This is also a pre-existing issue, but does the interrupt handler safely
access hardware registers when the device might be runtime suspended?

drivers/dma/sprd-dma.c:dma_irq_handle() {
    ...
	u32 irq_status = readl(sdev->glb_base + SPRD_DMA_GLB_INT_MSK_STS);
    ...
}

If a spurious interrupt occurs while the device is suspended and clocks are
disabled, dma_irq_handle() unconditionally reads from the hardware
register.

Should the handler verify the device's runtime PM state via
pm_runtime_get_if_active() before accessing registers to prevent a
synchronous external abort?

[ ... ]

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