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

[sashiko-bot@kernel.org]

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