[PATCH] dmaengine: xilinx: zynqmp_dma: fix autosuspend cleanup during teardown

[PATCH] dmaengine: xilinx: zynqmp_dma: fix autosuspend cleanup during teardown

[Guangshuo Li]

zynqmp_dma_probe() calls pm_runtime_use_autosuspend(), but its failure
paths and zynqmp_dma_remove() do not call the matching
pm_runtime_dont_use_autosuspend().

If the autosuspend delay is set to a negative value while autosuspend
is enabled, the runtime PM core increments usage_count to prevent
runtime suspend. Without calling pm_runtime_dont_use_autosuspend()
during teardown, this reference is not dropped and usage_count remains
unbalanced.

The documentation for pm_runtime_use_autosuspend() also notes that it
is important to undo it with pm_runtime_dont_use_autosuspend() at
driver exit time, unless runtime PM was initially enabled with
devm_pm_runtime_enable().

Add the missing pm_runtime_dont_use_autosuspend() calls to the probe
failure and remove paths.

This issue was found by manual code inspection.

Fixes: 64c6f7da8c2c ("dmaengine: zynqmp_dma: Add runtime pm support")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/dma/xilinx/zynqmp_dma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
index f6a812e49ddc..635c3f6cc5c2 100644
--- a/drivers/dma/xilinx/zynqmp_dma.c
+++ b/drivers/dma/xilinx/zynqmp_dma.c
@@ -1116,8 +1116,10 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
 	}
 	if (!pm_runtime_enabled(zdev->dev)) {
 		ret = zynqmp_dma_runtime_resume(zdev->dev);
-		if (ret)
+		if (ret) {
+			pm_runtime_dont_use_autosuspend(zdev->dev);
 			return ret;
+		}
 	}
 
 	ret = zynqmp_dma_chan_probe(zdev, pdev);
@@ -1152,6 +1154,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
 err_disable_pm:
 	if (!pm_runtime_enabled(zdev->dev))
 		zynqmp_dma_runtime_suspend(zdev->dev);
+	pm_runtime_dont_use_autosuspend(zdev->dev);
 	pm_runtime_disable(zdev->dev);
 	return ret;
 }
@@ -1172,6 +1175,7 @@ static void zynqmp_dma_remove(struct platform_device *pdev)
 	zynqmp_dma_chan_remove(zdev->chan);
 	if (pm_runtime_active(zdev->dev))
 		zynqmp_dma_runtime_suspend(zdev->dev);
+	pm_runtime_dont_use_autosuspend(zdev->dev);
 	pm_runtime_disable(zdev->dev);
 }
 
-- 
2.43.0

Re: [PATCH] dmaengine: xilinx: zynqmp_dma: fix autosuspend cleanup during teardown

[sashiko-bot]

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

Pre-existing issues:
- [High] The probe error path mishandles runtime PM state, leaking the usage count, leaving clocks enabled, and causing double-disable of PM.
- [High] Manual invocation of the runtime suspend callback during remove() races with the PM core, potentially leading to double clock disable or leaked clocks.
- [High] If pm_runtime_resume_and_get() fails during probe, the success path unconditionally calls pm_runtime_put_sync_autosuspend(), which underflows the usage count and triggers a kernel warning.
- [High] Fallback manual PM resume in probe is not cleanly undone during device removal, leaking clocks.
- [High] Overriding the return value of platform_get_irq() to -ENXIO breaks deferred probing.
- [High] Dropping a spinlock during list traversal in the cleanup tasklet causes double-free and infinite loops.
- [High] Memory and PM reference leak in zynqmp_dma_alloc_chan_resources() on failure paths.
--

--- Patch [1]: [PATCH] dmaengine: xilinx: zynqmp_dma: fix autosuspend cleanup during teardown ---
commit 84d893f1819f6fc2e37ae0cbd28a5db284008c1e
Author: Guangshuo Li <lgs201920130244@gmail.com>

dmaengine: xilinx: zynqmp_dma: fix autosuspend cleanup during teardown

This commit addresses an unbalanced usage_count by adding missing calls
to pm_runtime_dont_use_autosuspend() in the probe failure and remove paths.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index f6a812e49ddca..635c3f6cc5c2f 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c
> [ ... ]
> @@ -1152,6 +1154,7 @@ static int zynqmp_dma_probe(struct platform_device *pdev)
>  err_disable_pm:
>  	if (!pm_runtime_enabled(zdev->dev))
>  		zynqmp_dma_runtime_suspend(zdev->dev);
> +	pm_runtime_dont_use_autosuspend(zdev->dev);
>  	pm_runtime_disable(zdev->dev);
>  	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but does this error path mishandle runtime
PM state?

If pm_runtime_resume_and_get() succeeds but a subsequent step like
zynqmp_dma_chan_probe() fails, execution jumps to err_disable_pm. Because
pm_runtime_enabled() is true, zynqmp_dma_runtime_suspend() is skipped, leaving
hardware clocks on. It also never calls pm_runtime_put_noidle() to drop the
usage count.

Conversely, if pm_runtime_resume_and_get() initially fails, the driver calls
pm_runtime_disable() and falls back to manual resume. If probe then fails,
err_disable_pm calls pm_runtime_disable() a second time, incrementing
disable_depth to 2 and breaking PM permanently.

[Severity: High]
This is also a pre-existing issue, but does the success path of probe
unconditionally underflow the usage count if the fallback manual resume
path was taken?

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_probe() {
    ...
	ret = of_dma_controller_register(pdev->dev.of_node,
					 of_zynqmp_dma_xlate, zdev);
	if (ret) {
        ...
	}

	pm_runtime_put_sync_autosuspend(zdev->dev);

	return 0;
    ...
}

If pm_runtime_resume_and_get() fails, it drops its internal reference. The
failure path handles the error by calling pm_runtime_disable() and falling
back to manual resume.

If the rest of the probe succeeds, the function reaches
pm_runtime_put_sync_autosuspend() at the end. Even though runtime PM is
disabled, this function passes the RPM_GET_PUT flag to the PM core, which
calls rpm_drop_usage_count(), unconditionally decrementing the count below 0.

> [ ... ]
> @@ -1172,6 +1175,7 @@ static void zynqmp_dma_remove(struct platform_device *pdev)
>  	zynqmp_dma_chan_remove(zdev->chan);
>  	if (pm_runtime_active(zdev->dev))
>  		zynqmp_dma_runtime_suspend(zdev->dev);

[Severity: High]
This isn't a bug introduced by this patch, but does the manual invocation of
the runtime suspend callback race with the PM core?

Because runtime PM is still enabled during this manual check and suspend,
the auto-suspend timer or a PM workqueue can concurrently invoke the suspend
callback, which can cause unbalanced clk_disable_unprepare() calls.

Should pm_runtime_disable() be called before attempting manual suspend
management to serialize state and ensure no async operations are running?

[Severity: High]
This is also a pre-existing issue, but is the pm_runtime_active() check
bypassing the hardware suspend if the fallback PM path was taken during probe?

If pm_runtime_resume_and_get() fails during probe, the driver disables PM and
manually calls zynqmp_dma_runtime_resume() to turn on clocks. In this
fallback state, the PM core's runtime status remains RPM_SUSPENDED.

During zynqmp_dma_remove(), the pm_runtime_active() check returns false,
so zynqmp_dma_runtime_suspend() is incorrectly skipped and clocks are left
permanently enabled.

> +	pm_runtime_dont_use_autosuspend(zdev->dev);
>  	pm_runtime_disable(zdev->dev);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does overriding the return value
of platform_get_irq() break deferred probing?

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_chan_probe() {
    ...
	zynqmp_dma_init(chan);
	chan->irq = platform_get_irq(pdev, 0);
	if (chan->irq < 0)
		return -ENXIO;
    ...
}

platform_get_irq() can return -EPROBE_DEFER if the interrupt controller is
not yet ready. By swallowing the actual error code and unconditionally
returning -ENXIO, the driver prevents the kernel from retrying the probe
later.

[Severity: High]
This is a pre-existing issue, but can dropping the spinlock during list
traversal cause an infinite loop?

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_chan_desc_cleanup() {
    ...
			spin_unlock_irqrestore(&chan->lock, irqflags);
			dmaengine_desc_callback_invoke(&cb, NULL);
			spin_lock_irqsave(&chan->lock, irqflags);
    ...
}

The code iterates over chan->done_list using list_for_each_entry_safe().
To invoke a DMA callback, it drops chan->lock.

While dropped, a concurrent call to zynqmp_dma_device_terminate_all() can
acquire the lock and call zynqmp_dma_free_descriptors(), which moves all
descriptors from chan->done_list to chan->free_list.

When the tasklet reacquires the lock, its next pointer now points to an
element in chan->free_list. The tasklet loop incorrectly continues traversing
free_list until it crashes, never hitting the done_list list head to terminate
the loop.

[Severity: High]
This isn't a bug introduced by this patch, but do these error paths leak
memory and the PM usage counter?

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_alloc_chan_resources() {
    ...
	chan->sw_desc_pool = kzalloc_objs(*desc, ZYNQMP_DMA_NUM_DESCS);
	if (!chan->sw_desc_pool)
		return -ENOMEM;

	chan->desc_pool_v = dma_alloc_coherent(chan->dev,
					       (2 * ZYNQMP_DMA_DESC_SIZE(chan) *
					       ZYNQMP_DMA_NUM_DESCS),
					       &chan->desc_pool_p, GFP_KERNEL);
	if (!chan->desc_pool_v)
		return -ENOMEM;
    ...
}

pm_runtime_resume_and_get() increments the device's PM usage count earlier in
this function. If kzalloc_objs() fails, it returns -ENOMEM without dropping
the PM reference via pm_runtime_put().

If dma_alloc_coherent() fails, it returns -ENOMEM without freeing the
just-allocated chan->sw_desc_pool and without dropping the PM reference.
The dmaengine core does not invoke device_free_chan_resources() when
allocation fails.

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