Linux Tegra architecture development
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal
@ 2026-09-17 19:20 Myeonghun Pak
  2026-09-21 11:28 ` Thierry Reding
  0 siblings, 1 reply; 3+ messages in thread
From: Myeonghun Pak @ 2026-09-17 19:20 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Thierry Reding, Jonathan Hunter
  Cc: linux-usb, linux-tegra, linux-kernel, Ijae Kim, stable

The port status interrupt can schedule port_reset_war_work to handle the
Tegra210 port reset workaround. The remove path does not cancel this
work, so it can run after the PHYs and controller resources have been
released and access freed memory.

Disable and drain port_reset_war_work before tearing down the controller.
Use disable_delayed_work_sync() so that the interrupt handler cannot
queue the work again while the managed IRQ is still registered.

This issue was identified during our ongoing static-analysis research
while reviewing kernel code.

Fixes: 49db427232fe ("usb: gadget: Add UDC driver for tegra XUSB
device mode controller")
Cc: stable@vger.kernel.org # 6.10+
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/usb/gadget/udc/tegra-xudc.c |    1 +
 1 file changed, 1 insertion(+)

--- a/drivers/usb/gadget/udc/tegra-xudc.c
+++ b/drivers/usb/gadget/udc/tegra-xudc.c
@@ -3926,6 +3926,7 @@

 	pm_runtime_get_sync(xudc->dev);

+	disable_delayed_work_sync(&xudc->port_reset_war_work);
 	cancel_delayed_work_sync(&xudc->plc_reset_work);
 	cancel_work_sync(&xudc->usb_role_sw_work);

base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
2.47.1

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

* Re: [PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal
  2026-09-17 19:20 [PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal Myeonghun Pak
@ 2026-09-21 11:28 ` Thierry Reding
  2026-09-21 22:16   ` Myeonghun Pak
  0 siblings, 1 reply; 3+ messages in thread
From: Thierry Reding @ 2026-09-21 11:28 UTC (permalink / raw)
  To: Myeonghun Pak
  Cc: Greg Kroah-Hartman, Jonathan Hunter, linux-usb, linux-tegra,
	linux-kernel, Ijae Kim, stable

[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]

On Thu, Sep 17, 2026 at 07:20:58PM +0000, Myeonghun Pak wrote:
> The port status interrupt can schedule port_reset_war_work to handle the
> Tegra210 port reset workaround. The remove path does not cancel this
> work, so it can run after the PHYs and controller resources have been
> released and access freed memory.
> 
> Disable and drain port_reset_war_work before tearing down the controller.
> Use disable_delayed_work_sync() so that the interrupt handler cannot
> queue the work again while the managed IRQ is still registered.
> 
> This issue was identified during our ongoing static-analysis research
> while reviewing kernel code.
> 
> Fixes: 49db427232fe ("usb: gadget: Add UDC driver for tegra XUSB
> device mode controller")

Please don't wrap lines like this.

> Cc: stable@vger.kernel.org # 6.10+
> Assisted-by: LLM
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
>  drivers/usb/gadget/udc/tegra-xudc.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/drivers/usb/gadget/udc/tegra-xudc.c
> +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> @@ -3926,6 +3926,7 @@
> 
>  	pm_runtime_get_sync(xudc->dev);
> 
> +	disable_delayed_work_sync(&xudc->port_reset_war_work);
>  	cancel_delayed_work_sync(&xudc->plc_reset_work);
>  	cancel_work_sync(&xudc->usb_role_sw_work);

This is a preexisting problem, but given that we only delete the gadget
below this cleanup, shouldn't the other two cancel_*() calls be
disable_*() as well? Otherwise there's potentially a race condition
between this and the interrupt handler (which is only unregistered after
.remove() completes).

Either that or the existing cancel_*() calls are enough in which case
your patch probably should be using that variant as well.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal
  2026-09-21 11:28 ` Thierry Reding
@ 2026-09-21 22:16   ` Myeonghun Pak
  0 siblings, 0 replies; 3+ messages in thread
From: Myeonghun Pak @ 2026-09-21 22:16 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Greg Kroah-Hartman, Jonathan Hunter, linux-usb, linux-tegra,
	linux-kernel, Ijae Kim, stable

Thanks for the review. v2 keeps the Fixes tag on one line and switches
plc_reset_work to disable_delayed_work_sync() as well, since the
port-status IRQ can queue it after cancel_delayed_work_sync() returns.
usb_role_sw_work stays on cancel_work_sync(). Its producer is the USB
PHY notifier, which v2 unregisters first. The two bugs are split so
the notifier fix can be backported below v6.10.

2026년 9월 21일 (월) 오전 7:28, Thierry Reding <thierry.reding@kernel.org>님이 작성:
>
> On Thu, Sep 17, 2026 at 07:20:58PM +0000, Myeonghun Pak wrote:
> > The port status interrupt can schedule port_reset_war_work to handle the
> > Tegra210 port reset workaround. The remove path does not cancel this
> > work, so it can run after the PHYs and controller resources have been
> > released and access freed memory.
> >
> > Disable and drain port_reset_war_work before tearing down the controller.
> > Use disable_delayed_work_sync() so that the interrupt handler cannot
> > queue the work again while the managed IRQ is still registered.
> >
> > This issue was identified during our ongoing static-analysis research
> > while reviewing kernel code.
> >
> > Fixes: 49db427232fe ("usb: gadget: Add UDC driver for tegra XUSB
> > device mode controller")
>
> Please don't wrap lines like this.
>
> > Cc: stable@vger.kernel.org # 6.10+
> > Assisted-by: LLM
> > Co-developed-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> > ---
> >  drivers/usb/gadget/udc/tegra-xudc.c |    1 +
> >  1 file changed, 1 insertion(+)
> >
> > --- a/drivers/usb/gadget/udc/tegra-xudc.c
> > +++ b/drivers/usb/gadget/udc/tegra-xudc.c
> > @@ -3926,6 +3926,7 @@
> >
> >       pm_runtime_get_sync(xudc->dev);
> >
> > +     disable_delayed_work_sync(&xudc->port_reset_war_work);
> >       cancel_delayed_work_sync(&xudc->plc_reset_work);
> >       cancel_work_sync(&xudc->usb_role_sw_work);
>
> This is a preexisting problem, but given that we only delete the gadget
> below this cleanup, shouldn't the other two cancel_*() calls be
> disable_*() as well? Otherwise there's potentially a race condition
> between this and the interrupt handler (which is only unregistered after
> .remove() completes).
>
> Either that or the existing cancel_*() calls are enough in which case
> your patch probably should be using that variant as well.
>
> Thierry

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

end of thread, other threads:[~2026-09-21 22:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-17 19:20 [PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal Myeonghun Pak
2026-09-21 11:28 ` Thierry Reding
2026-09-21 22:16   ` Myeonghun Pak

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