[PATCH 1/4] usb: musb: cancel irq_work after masking interrupts in musb_remove()

[PATCH 1/4] usb: musb: cancel irq_work after masking interrupts in musb_remove()

[Chengyu Liu]

musb_stage0_irq() can schedule irq_work while handling stage-0 USB
interrupts, and the work can also re-arm itself from the session
recheck path (musb_pm_runtime_check_session()).  musb_remove()
cancels irq_work while the interrupt is still enabled and the IRQ
line not yet freed: an interrupt landing after the cancel re-arms
the work, and musb_irq_work() then runs after musb_free() has
released the struct.

Move the cancel behind musb_disable_interrupts() and synchronize the
IRQ line first, so no handler can be running or re-arm the work past
the cancel.

Found by static analysis.

Fixes: c5340bd14336 ("usb: musb: cancel work on removal")
Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
Build-tested on x86_64: musb_core.o compiles clean
(USB_MUSB_HDRC).  No runtime reproducer: requires MUSB-based OTG
hardware.  Fix written with AI assistance.

 drivers/usb/musb/musb_core.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 9b63b56edc45..eea43a20a1d0 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2630,7 +2630,6 @@ static void musb_remove(struct platform_device *pdev)
 	 */
 	musb_exit_debugfs(musb);
 
-	cancel_delayed_work_sync(&musb->irq_work);
 	cancel_delayed_work_sync(&musb->finish_resume_work);
 	cancel_delayed_work_sync(&musb->deassert_reset_work);
 	pm_runtime_get_sync(musb->controller);
@@ -2642,6 +2641,15 @@ static void musb_remove(struct platform_device *pdev)
 	musb_disable_interrupts(musb);
 	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
 	spin_unlock_irqrestore(&musb->lock, flags);
+
+	/*
+	 * musb_stage0_irq() arms irq_work, so it can only be cancelled
+	 * for good once interrupts are masked and any in-flight handler
+	 * has completed.
+	 */
+	synchronize_irq(musb->nIrq);
+	cancel_delayed_work_sync(&musb->irq_work);
+
 	musb_platform_exit(musb);
 
 	pm_runtime_dont_use_autosuspend(musb->controller);
-- 
2.25.1

Re: [PATCH 1/4] usb: musb: cancel irq_work after masking interrupts in musb_remove()

[Neill Kapron]

> musb_stage0_irq() can schedule irq_work while handling stage-0 USB
> interrupts, and the work can also re-arm itself from the session
> recheck path (musb_pm_runtime_check_session()).  musb_remove()
> cancels irq_work while the interrupt is still enabled and the IRQ
> line not yet freed: an interrupt landing after the cancel re-arms
> the work, and musb_irq_work() then runs after musb_free() has
> released the struct.
> 
> Move the cancel behind musb_disable_interrupts() and synchronize the
> IRQ line first, so no handler can be running or re-arm the work past
> the cancel.
> 
> Found by static analysis.
> 
> Fixes: c5340bd14336 ("usb: musb: cancel work on removal")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
> ---
> Build-tested on x86_64: musb_core.o compiles clean
> (USB_MUSB_HDRC).  No runtime reproducer: requires MUSB-based OTG
> hardware.  Fix written with AI assistance.
> 
>  drivers/usb/musb/musb_core.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 73ac25f53698..c71d0c7f1bcb 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -2630,7 +2630,6 @@ static void musb_remove(struct platform_device *pdev)
>  	 */
>  	musb_exit_debugfs(musb);
>  
> -	cancel_delayed_work_sync(&musb->irq_work);
>  	cancel_delayed_work_sync(&musb->finish_resume_work);
>  	cancel_delayed_work_sync(&musb->deassert_reset_work);

Shouldn't all three cancel_delayed_work_sync calls be moved to after
synchronize_irq? Seems like these other two work queues are subject to
similar style issues.

-- 
Neill Kapron <nkapron@google.com>

Re: [PATCH 1/4] usb: musb: cancel irq_work after masking interrupts in musb_remove()

[Chengyu Liu]

On Fri, Nov 13, 2026 at 3:18 AM Neill Kapron <nkapron@google.com> wrote:
> Shouldn't all three cancel_delayed_work_sync calls be moved to after
> synchronize_irq? Seems like these other two work queues are subject to
> similar style issues.

Hi Neill,

Good catch. The static analyzer I wrote only reported irq_work, and I
missed checking the other two work items in the same function.

I checked their scheduling paths:
finish_resume_work can also be queued from the IRQ handler and
musb_port_suspend().
deassert_reset_work can be queued from musb_hub_control() before
musb_host_cleanup() removes the HCD.

I'll move all three cancellations after host/gadget cleanup, interrupt
masking, and synchronize_irq() in v2.

Thanks,
Chengyu

[PATCH 2/4] usb: musb: dsps: free the VBUS interrupt before the child teardown

[Chengyu Liu]

dsps_remove() unregisters the musb child, which deletes
musb->dev_timer and frees the structure, while the glue's VBUS
interrupt is devm-managed and only freed after dsps_remove() returns.
dsps_vbus_threaded_irq() re-arms the timer via dsps_mod_timer(), so
a VBUS event in between re-arms dev_timer past its deletion and
otg_timer() runs on freed memory.

Free the VBUS interrupt first, which also waits for any in-flight
handler, so nothing can re-arm the timer once the child teardown
runs.

Found by static analysis.

Fixes: 369469a92393 ("usb: musb: Add support for optional VBUS irq to dsps glue layer")
Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
Build-tested on x86_64: musb_dsps.o compiles clean
(USB_MUSB_DSPS).  No runtime reproducer: requires AM33xx/DSPS
hardware.  Fix written with AI assistance.

 drivers/usb/musb/musb_dsps.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 91d8712efa41..69c8b4b3ffa7 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -942,6 +942,17 @@ static void dsps_remove(struct platform_device *pdev)
 {
 	struct dsps_glue *glue = platform_get_drvdata(pdev);
 
+	/*
+	 * The VBUS interrupt handler re-arms musb->dev_timer via
+	 * dsps_mod_timer(); the IRQ is devm-managed and would only be
+	 * freed after this function returns, while the child teardown
+	 * below deletes the timer and frees the musb structure.  Free
+	 * the interrupt first so no VBUS event can re-arm the timer
+	 * past that point.
+	 */
+	if (glue->vbus_irq)
+		devm_free_irq(glue->dev, glue->vbus_irq, glue);
+
 	platform_device_unregister(glue->musb);
 
 	pm_runtime_disable(&pdev->dev);
-- 
2.25.1

Re: [PATCH 2/4] usb: musb: dsps: free the VBUS interrupt before the child teardown

[Neill Kapron]

On Fri, 13 Nov 2026 00:04:46 +0800, Chengyu Liu <lui2012qwq@gmail.com> wrote:
> dsps_remove() unregisters the musb child, which deletes
> musb->dev_timer and frees the structure, while the glue's VBUS
> interrupt is devm-managed and only freed after dsps_remove() returns.
> dsps_vbus_threaded_irq() re-arms the timer via dsps_mod_timer(), so
> a VBUS event in between re-arms dev_timer past its deletion and
> otg_timer() runs on freed memory.
> 
> [...]

Reviewed-by: Neill Kapron <nkapron@google.com>

-- 
Neill Kapron <nkapron@google.com>

[PATCH 3/4] usb: gadget: r8a66597: free the IRQ before deleting the timer

[Chengyu Liu]

r8a66597_remove() deletes r8a66597->timer while the interrupt is
still live: the IRQ is devm-managed and only freed at detach, and
r8a66597_irq() re-arms the timer when sampling VBUS (VBINT).  An
interrupt landing after timer_delete_sync() re-arms the timer, and
r8a66597_timer() then runs on freed memory.

Store the requested IRQ number at probe time and free it before
deleting the timer; freeing also waits for any running handler, so
nothing can re-arm the timer past the delete.

Found by static analysis.

Fixes: c41442474a26 ("usb: gadget: R8A66597 peripheral controller support.")
Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
Build-tested on x86_64: r8a66597-udc.o compiles clean
(USB_R8A66597 + COMPILE_TEST).  No runtime reproducer: requires
R8A66597 hardware.  Fix written with AI assistance.

 drivers/usb/gadget/udc/r8a66597-udc.c | 8 ++++++++
 drivers/usb/gadget/udc/r8a66597-udc.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/gadget/udc/r8a66597-udc.c b/drivers/usb/gadget/udc/r8a66597-udc.c
index 469075b064ba..bfccc6565e0a 100644
--- a/drivers/usb/gadget/udc/r8a66597-udc.c
+++ b/drivers/usb/gadget/udc/r8a66597-udc.c
@@ -1810,6 +1810,13 @@ static void r8a66597_remove(struct platform_device *pdev)
 	struct r8a66597		*r8a66597 = platform_get_drvdata(pdev);
 
 	usb_del_gadget_udc(&r8a66597->gadget);
+
+	/*
+	 * The interrupt handler re-arms the timer on VBUS changes; the
+	 * IRQ is devm-managed and freed only at detach, so free it
+	 * before deleting the timer.
+	 */
+	devm_free_irq(&pdev->dev, r8a66597->irq, r8a66597);
 	timer_delete_sync(&r8a66597->timer);
 	r8a66597_free_request(&r8a66597->ep[0].ep, r8a66597->ep0_req);
 
@@ -1896,6 +1903,7 @@ static int r8a66597_probe(struct platform_device *pdev)
 			       udc_name, r8a66597);
 	if (ret < 0)
 		goto clean_up2;
+	r8a66597->irq = irq;
 
 	INIT_LIST_HEAD(&r8a66597->gadget.ep_list);
 	r8a66597->gadget.ep0 = &r8a66597->ep[0].ep;
diff --git a/drivers/usb/gadget/udc/r8a66597-udc.h b/drivers/usb/gadget/udc/r8a66597-udc.h
index 9a115caba661..08acabe02421 100644
--- a/drivers/usb/gadget/udc/r8a66597-udc.h
+++ b/drivers/usb/gadget/udc/r8a66597-udc.h
@@ -110,6 +110,7 @@ struct r8a66597 {
 	unsigned char interrupt;
 	unsigned char isochronous;
 	unsigned char num_dma;
+	int irq;
 
 	unsigned irq_sense_low:1;
 };
-- 
2.25.1

Re: [PATCH 3/4] usb: gadget: r8a66597: free the IRQ before deleting the timer

[Neill Kapron]

On Fri, 13 Nov 2026 00:04:47 +0800, Chengyu Liu <lui2012qwq@gmail.com> wrote:
> r8a66597_remove() deletes r8a66597->timer while the interrupt is
> still live: the IRQ is devm-managed and only freed at detach, and
> r8a66597_irq() re-arms the timer when sampling VBUS (VBINT).  An
> interrupt landing after timer_delete_sync() re-arms the timer, and
> r8a66597_timer() then runs on freed memory.
> 
> Store the requested IRQ number at probe time and free it before
> deleting the timer; freeing also waits for any running handler, so
> nothing can re-arm the timer past the delete.
> 
> [...]

Reviewed-by: Neill Kapron <nkapron@google.com>

-- 
Neill Kapron <nkapron@google.com>

[PATCH 4/4] usb: phy: gpio-vbus: free the interrupt before cancelling the work

[Chengyu Liu]

gpio_vbus_remove() cancels gpio_vbus->work while the VBUS sensing GPIO
interrupt is still live: it is devm-managed and freed only at detach,
and gpio_vbus_irq() schedules the work with a 100 ms debounce delay.
A plug event landing after the cancel re-arms the work, and its timer
then fires on the freed gpio_vbus_data.

Free the interrupt first, which also waits for any running handler,
so nothing can re-arm the work past the cancel.

Found by static analysis and confirmed with a KASAN reproducer:
slab-use-after-free write in __run_timer_base() within one debounce
interval of unregistering the device while its VBUS GPIO is toggled
(object allocated by gpio_vbus_probe() via devm_kmalloc(), freed by
devres_release_all() at unbind).

Fixes: c2344f13b59e ("USB: gpio_vbus: add delayed vbus_session calls")
Cc: stable@vger.kernel.org
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
The reproducer instantiates the unmodified driver on a gpio-sim
chip in QEMU and toggles the line while the device is unregistered;
it widens the remove()-to-detach window with a 500 ms sleep, logic
otherwise unmodified.  Repeated runs: 5/5 KASAN splats without the
fix, 0/5 with it.  The KASAN report and the reproducer kit follow
in this thread.  Also build-tested on x86_64 (USB_GPIO_VBUS).
Fix written with AI assistance.

 drivers/usb/phy/phy-gpio-vbus-usb.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/usb/phy/phy-gpio-vbus-usb.c b/drivers/usb/phy/phy-gpio-vbus-usb.c
index 60b7c1219dd2..82d2301c3804 100644
--- a/drivers/usb/phy/phy-gpio-vbus-usb.c
+++ b/drivers/usb/phy/phy-gpio-vbus-usb.c
@@ -325,6 +325,14 @@ static void gpio_vbus_remove(struct platform_device *pdev)
 	struct gpio_vbus_data *gpio_vbus = platform_get_drvdata(pdev);
 
 	device_init_wakeup(&pdev->dev, 0);
+
+	/*
+	 * The VBUS sensing GPIO interrupt is devm-managed and freed only
+	 * at detach, and its handler schedules the work with a 100ms
+	 * delay; free the interrupt before cancelling the work so a
+	 * late plug event cannot re-arm it past the cancel.
+	 */
+	devm_free_irq(&pdev->dev, gpio_vbus->irq, pdev);
 	cancel_delayed_work_sync(&gpio_vbus->work);
 
 	usb_remove_phy(&gpio_vbus->phy);
-- 
2.25.1

Re: [PATCH 4/4] usb: phy: gpio-vbus: free the interrupt before cancelling the work

[Neill Kapron]

On Fri, 13 Nov 2026 00:04:48 +0800, Chengyu Liu <lui2012qwq@gmail.com> wrote:
> gpio_vbus_remove() cancels gpio_vbus->work while the VBUS sensing GPIO
> interrupt is still live: it is devm-managed and freed only at detach,
> and gpio_vbus_irq() schedules the work with a 100 ms debounce delay.
> A plug event landing after the cancel re-arms the work, and its timer
> then fires on the freed gpio_vbus_data.
> 
> Free the interrupt first, which also waits for any running handler,
> so nothing can re-arm the work past the cancel.
> 
> [...]

Reviewed-by: Neill Kapron <nkapron@google.com>

-- 
Neill Kapron <nkapron@google.com>