Linux kernel and device drivers for NXP i.MX platforms
 help / color / mirror / Atom feed
* [PATCH] usb: chipidea: fix usage_count leak when autosuspend_delay is negative
@ 2026-07-16 10:41 Xu Yang
  2026-07-16 10:49 ` sashiko-bot
  2026-07-16 12:42 ` Frank Li
  0 siblings, 2 replies; 3+ messages in thread
From: Xu Yang @ 2026-07-16 10:41 UTC (permalink / raw)
  To: peter.chen; +Cc: gregkh, linux-usb, linux-kernel, imx, jun.li

From: Xu Yang <xu.yang_2@nxp.com>

The probe() calls pm_runtime_use_autosuspend(), but remove() does not call
pm_runtime_dont_use_autosuspend(). This can lead to a usage_count leak if
autosuspend_delay is set to a negative value.

The pm_runtime_use_autosuspend() also notes that it's important to undo
this with pm_runtime_dont_use_autosuspend() at driver exit time.

Fixes: 1f874edcb731 ("usb: chipidea: add runtime power management support")
Assisted-by: Claude:claude-sonnet-4.6
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
 drivers/usb/chipidea/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 7edc512cc37d..3461dd763a6e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -1250,6 +1250,7 @@ static void ci_hdrc_remove(struct platform_device *pdev)
 		usb_role_switch_unregister(ci->role_switch);
 
 	if (ci->supports_runtime_pm) {
+		pm_runtime_dont_use_autosuspend(&pdev->dev);
 		pm_runtime_get_sync(&pdev->dev);
 		pm_runtime_disable(&pdev->dev);
 		pm_runtime_put_noidle(&pdev->dev);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: chipidea: fix usage_count leak when autosuspend_delay is negative
  2026-07-16 10:41 [PATCH] usb: chipidea: fix usage_count leak when autosuspend_delay is negative Xu Yang
@ 2026-07-16 10:49 ` sashiko-bot
  2026-07-16 12:42 ` Frank Li
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-16 10:49 UTC (permalink / raw)
  To: Xu Yang; +Cc: imx, Frank.Li

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

Pre-existing issues:
- [High] Background tasks and timers are not properly canceled during driver removal, leading to Use-After-Free.
- [High] The hardware is put into low power mode during removal without disabling the devm-managed shared IRQ, leading to a kernel crash if the IRQ fires.
--

--- Patch [1]: [PATCH] usb: chipidea: fix usage_count leak when autosuspend_delay is negative ---
commit fdadfb5f628818e9f20b2b5de671e334ccae62bf
Author: Xu Yang <xu.yang_2@nxp.com>

usb: chipidea: fix usage_count leak when autosuspend_delay is negative

The probe function calls pm_runtime_use_autosuspend(), but the remove
function misses the corresponding pm_runtime_dont_use_autosuspend() call.
This fixes a usage count leak when autosuspend_delay is set to a negative
value.

> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 07563be0013f4..b53672f3ea634 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -1250,6 +1250,7 @@ static void ci_hdrc_remove(struct platform_device *pdev)
>  		usb_role_switch_unregister(ci->role_switch);
>  
>  	if (ci->supports_runtime_pm) {
> +		pm_runtime_dont_use_autosuspend(&pdev->dev);
>  		pm_runtime_get_sync(&pdev->dev);
>  		pm_runtime_disable(&pdev->dev);
>  		pm_runtime_put_noidle(&pdev->dev);

[Severity: High]
This is a pre-existing issue, but does this driver leave background tasks
running after removal?

Looking at the remainder of ci_hdrc_remove(), it finishes with:

	dbg_remove_files(ci);
	ci_role_destroy(ci);
	ci_hdrc_enter_lpm(ci, true);
	ci_usb_phy_exit(ci);
	ci_ulpi_exit(ci);
}

The ci structure is devres-managed and will be freed after ci_hdrc_remove()
completes, but ci->otg_fsm_hrtimer and ci->power_lost_work are not
explicitly canceled during this teardown.

If the hrtimer fires or the work executes after this point, could it cause a
use-after-free? Furthermore, since the timer queues work to ci->wq, which is
destroyed in ci_hdrc_otg_destroy(), would a firing timer attempt to use a
destroyed workqueue?

[Severity: High]
This is also a pre-existing issue, but does entering low power mode during
removal leave a race window with the shared IRQ?

In ci_hdrc_remove(), the hardware is powered down by calling
ci_hdrc_enter_lpm(ci, true).

Because the interrupt is requested as devm-managed and IRQF_SHARED, it
remains active until devres cleanup runs after the remove function returns.
If another device on the same shared IRQ line triggers an interrupt during
this window, ci_irq_handler() will execute.

Since ci->in_lpm was not set to true before powering down, will the handler
bypass the low-power early exit check and crash attempting to read
powered-down registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716104126.2763454-1-xu.yang_2@oss.nxp.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: chipidea: fix usage_count leak when autosuspend_delay is negative
  2026-07-16 10:41 [PATCH] usb: chipidea: fix usage_count leak when autosuspend_delay is negative Xu Yang
  2026-07-16 10:49 ` sashiko-bot
@ 2026-07-16 12:42 ` Frank Li
  1 sibling, 0 replies; 3+ messages in thread
From: Frank Li @ 2026-07-16 12:42 UTC (permalink / raw)
  To: Xu Yang; +Cc: peter.chen, gregkh, linux-usb, linux-kernel, imx, jun.li

On Thu, Jul 16, 2026 at 06:41:26PM +0800, Xu Yang wrote:
> From: Xu Yang <xu.yang_2@nxp.com>
>
> The probe() calls pm_runtime_use_autosuspend(), but remove() does not call
> pm_runtime_dont_use_autosuspend(). This can lead to a usage_count leak if
> autosuspend_delay is set to a negative value.
>
> The pm_runtime_use_autosuspend() also notes that it's important to undo
> this with pm_runtime_dont_use_autosuspend() at driver exit time.
>
> Fixes: 1f874edcb731 ("usb: chipidea: add runtime power management support")
> Assisted-by: Claude:claude-sonnet-4.6
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/usb/chipidea/core.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 7edc512cc37d..3461dd763a6e 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -1250,6 +1250,7 @@ static void ci_hdrc_remove(struct platform_device *pdev)
>  		usb_role_switch_unregister(ci->role_switch);
>
>  	if (ci->supports_runtime_pm) {
> +		pm_runtime_dont_use_autosuspend(&pdev->dev);
>  		pm_runtime_get_sync(&pdev->dev);
>  		pm_runtime_disable(&pdev->dev);
>  		pm_runtime_put_noidle(&pdev->dev);
> --
> 2.34.1
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-16 12:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 10:41 [PATCH] usb: chipidea: fix usage_count leak when autosuspend_delay is negative Xu Yang
2026-07-16 10:49 ` sashiko-bot
2026-07-16 12:42 ` Frank Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox