* Re: [PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe
2026-08-13 15:31 [PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe Ruoyu Wang
@ 2026-08-13 15:49 ` sashiko-bot
2026-08-14 0:35 ` Baolin Wang
2026-08-19 19:29 ` Frank Li
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-13 15:49 UTC (permalink / raw)
To: Ruoyu Wang; +Cc: dmaengine, vkoul, Frank.Li
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe
2026-08-13 15:31 [PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe Ruoyu Wang
2026-08-13 15:49 ` sashiko-bot
@ 2026-08-14 0:35 ` Baolin Wang
2026-08-19 19:29 ` Frank Li
2 siblings, 0 replies; 4+ messages in thread
From: Baolin Wang @ 2026-08-14 0:35 UTC (permalink / raw)
To: Ruoyu Wang, dmaengine
Cc: vkoul, Frank.Li, orsonzhai, zhang.lyra, linux-kernel
On 8/13/26 11:31 PM, Ruoyu Wang wrote:
> 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>
> ---
LGTM. Thanks.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.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;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe
2026-08-13 15:31 [PATCH] dmaengine: sprd: Fix runtime PM reference leak in probe Ruoyu Wang
2026-08-13 15:49 ` sashiko-bot
2026-08-14 0:35 ` Baolin Wang
@ 2026-08-19 19:29 ` Frank Li
2 siblings, 0 replies; 4+ messages in thread
From: Frank Li @ 2026-08-19 19:29 UTC (permalink / raw)
To: Ruoyu Wang
Cc: dmaengine, vkoul, Frank.Li, orsonzhai, baolin.wang, zhang.lyra,
linux-kernel
On Thu, Aug 13, 2026 at 11:31:49PM +0800, Ruoyu Wang wrote:
> 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>
> ---
Reviewed-by: Frank Li <Frank.Li@nxp.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
>
^ permalink raw reply [flat|nested] 4+ messages in thread