diff mbox series

bus: sunxi-rsb: fix usage_count leak when autosuspend_delay is negative

Message ID 20260808062029.2597581-1-lgs201920130244@gmail.com (mailing list archive)
State New
Headers
Series bus: sunxi-rsb: fix usage_count leak when autosuspend_delay is negative |

Commit Message

Guangshuo Li Aug. 8, 2026, 6:20 a.m. UTC 
sunxi_rsb_probe() calls pm_runtime_use_autosuspend(), but
sunxi_rsb_remove() does not call the matching
pm_runtime_dont_use_autosuspend() before disabling runtime PM.

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 driver teardown, this reference is not dropped and usage_count
remains unbalanced.

Add the missing pm_runtime_dont_use_autosuspend() call before disabling
runtime PM.

This issue was found by manual code inspection.

Fixes: 4a0dbc12e618 ("bus: sunxi-rsb: Implement runtime power management")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/bus/sunxi-rsb.c | 1 +
 1 file changed, 1 insertion(+)

Comments

sashiko-bot@kernel.org Aug. 8, 2026, 6:38 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] Clock enable count leak during system resume if the device was runtime-suspended.
- [High] Division by zero if clock-frequency property is 0.
- [High] Unclocked hardware access due to IRQ registration before clock enablement.
- [High] Use-After-Free of child device `of_node` due to missing `of_node_get()`.
- [High] Sleeping in atomic context within `.resume_noirq`.
--

--- Patch [1]: [PATCH] bus: sunxi-rsb: fix usage_count leak when autosuspend_delay is negative ---
commit 353b6956f11193cdddcc23fba6f083e34c249931
Author: Guangshuo Li <lgs201920130244@gmail.com>

bus: sunxi-rsb: fix usage_count leak when autosuspend_delay is negative

The patch adds pm_runtime_dont_use_autosuspend() in sunxi_rsb_remove()
to fix a usage count leak when autosuspend delay is negative.

> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index daf0ea563e477..cd4c857756cfa 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
[ ... ]
> static struct sunxi_rsb_device *sunxi_rsb_device_create(struct sunxi_rsb *rsb,
> 		struct device_node *node, u16 hwaddr, u8 rtaddr)
> {
[ ... ]
> 	rdev->dev.bus = &sunxi_rsb_bus;
> 	rdev->dev.parent = rsb->dev;
> 	rdev->dev.of_node = node;
> 	rdev->dev.release = sunxi_rsb_dev_release;
[ ... ]
> static int of_rsb_register_devices(struct sunxi_rsb *rsb)
> {
[ ... ]
> 	for_each_available_child_of_node(np, child) {
> 		struct sunxi_rsb_device *rdev;
[ ... ]
> 		rdev = sunxi_rsb_device_create(rsb, child, hwaddr, rtaddr);

[Severity: High]
This isn't a bug introduced by this patch, but does this cause a
use-after-free of the child device node?

for_each_available_child_of_node() automatically drops the reference to the
previous node on each iteration. Since sunxi_rsb_device_create() assigns it
to rdev->dev.of_node without taking an extra reference via of_node_get(),
the node could be freed when the loop advances.

[ ... ]
> static int sunxi_rsb_hw_init(struct sunxi_rsb *rsb)
> {
[ ... ]
> 	/* reset the controller */
> 	writel(RSB_CTRL_SOFT_RST, rsb->regs + RSB_CTRL);
> 	readl_poll_timeout(rsb->regs + RSB_CTRL, reg,
> 			   !(reg & RSB_CTRL_SOFT_RST), 1000, 100000);
[ ... ]
> static const struct dev_pm_ops sunxi_rsb_dev_pm_ops = {
> 	SET_RUNTIME_PM_OPS(sunxi_rsb_runtime_suspend,
> 			   sunxi_rsb_runtime_resume, NULL)
> 	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(sunxi_rsb_suspend, sunxi_rsb_resume)
> };

[Severity: High]
This is a pre-existing issue, but does this sleep in an atomic context?

sunxi_rsb_resume() is registered to run in the _noirq context via
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS, which executes with local interrupts
disabled. However, it calls sunxi_rsb_hw_init(), which uses
readl_poll_timeout() with a 1000us delay. This causes the macro to invoke
usleep_range() and attempt to sleep in an atomic context, which will
trigger a kernel panic during system resume.

[ ... ]
> static int sunxi_rsb_hw_init(struct sunxi_rsb *rsb)
> {
[ ... ]
> 	p_clk_freq = clk_get_rate(rsb->clk);
> 	clk_div = p_clk_freq / rsb->clk_freq / 2;

[Severity: High]
This is a pre-existing issue, but can this cause a division by zero
exception?

In sunxi_rsb_probe(), the clock-frequency from the device tree is checked
against RSB_MAX_FREQ, but it's not checked for 0.

If it is 0, the division p_clk_freq / rsb->clk_freq / 2 will trigger a
division by zero panic.

[ ... ]
> static int __maybe_unused sunxi_rsb_runtime_resume(struct device *dev)
> {
> 	struct sunxi_rsb *rsb = dev_get_drvdata(dev);
> 
> 	return clk_prepare_enable(rsb->clk);
> }
[ ... ]
> static int __maybe_unused sunxi_rsb_resume(struct device *dev)
> {
> 	struct sunxi_rsb *rsb = dev_get_drvdata(dev);
> 
> 	return sunxi_rsb_hw_init(rsb);
> }

[Severity: High]
This isn't a bug introduced by this patch, but does this leak the clock
enable count during system resume?

If the device is runtime-suspended before a system suspend,
sunxi_rsb_resume() calls sunxi_rsb_hw_init() which calls
clk_prepare_enable().

Since it was runtime-suspended, a subsequent pm_runtime_resume_and_get()
would call sunxi_rsb_runtime_resume() which enables the clock a second
time, leaving the clock permanently enabled.

[ ... ]
> static int sunxi_rsb_probe(struct platform_device *pdev)
> {
[ ... ]
> 	ret = devm_request_irq(dev, irq, sunxi_rsb_irq, 0, RSB_CTRL_NAME, rsb);
> 	if (ret)
> 		return dev_err_probe(dev, ret,
> 				     "can't register interrupt handler irq %d\n", irq);
> 
> 	ret = sunxi_rsb_hw_init(rsb);
> 	if (ret)
> 		return ret;
[ ... ]
> @@ -815,6 +815,7 @@ static void sunxi_rsb_remove(struct platform_device *pdev)
>  	struct sunxi_rsb *rsb = platform_get_drvdata(pdev);
>  
>  	device_for_each_child(rsb->dev, NULL, sunxi_rsb_remove_devices);
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);
>  	pm_runtime_disable(&pdev->dev);
>  	sunxi_rsb_hw_exit(rsb);
>  }

[Severity: High]
This isn't a bug introduced by this patch, but can this lead to an
unclocked hardware access?

In sunxi_rsb_probe(), the IRQ handler is registered before
sunxi_rsb_hw_init() enables the clock. If a pending interrupt triggers
immediately upon request, sunxi_rsb_irq() could read memory-mapped
registers while the clock is disabled.

Similarly, in sunxi_rsb_remove() shown in the diff here, the hardware clock
is disabled by sunxi_rsb_hw_exit(), but the devm-managed IRQ is still
registered and active, creating another window where sunxi_rsb_irq() could
execute after the clock is off.
Johan Hovold Aug. 18, 2026, 9:22 a.m. UTC | #2 
On Sat, Aug 08, 2026 at 02:20:29PM +0800, Guangshuo Li wrote:
> sunxi_rsb_probe() calls pm_runtime_use_autosuspend(), but
> sunxi_rsb_remove() does not call the matching
> pm_runtime_dont_use_autosuspend() before disabling runtime PM.
> 
> 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 driver teardown, this reference is not dropped and usage_count
> remains unbalanced.

As I've explained elsewhere, this is just misleading. There is no usage
count leak here as the count is balanced whenever the user re-enables
autosuspend through sysfs (by writing a non-negative timeout).

Drivers should clean up after themselves and disable autosuspend, but
this is more of a clean up than a fix and should not be backported.

You've sent upwards of 60 of these in the matter of just a few days,
some of which have even been picked up. Please send follow-ups (replies
or v2s) as soon as possible to prevent further of these from getting
merged.

> Add the missing pm_runtime_dont_use_autosuspend() call before disabling
> runtime PM.
> 
> This issue was found by manual code inspection.
> 
> Fixes: 4a0dbc12e618 ("bus: sunxi-rsb: Implement runtime power management")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>

Johan
diff mbox series

Patch

diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index daf0ea563e47..cd4c857756cf 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -815,6 +815,7 @@  static void sunxi_rsb_remove(struct platform_device *pdev)
 	struct sunxi_rsb *rsb = platform_get_drvdata(pdev);
 
 	device_for_each_child(rsb->dev, NULL, sunxi_rsb_remove_devices);
+	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	sunxi_rsb_hw_exit(rsb);
 }