[PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal

Myeonghun Pak posted 1 patch 1 week ago
drivers/usb/gadget/udc/tegra-xudc.c |    1 +
1 file changed, 1 insertion(+)
[PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal
Posted by Myeonghun Pak 1 week ago
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
Re: [PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal
Posted by Thierry Reding 3 days, 13 hours ago
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
Re: [PATCH] usb: gadget: tegra-xudc: Disable port reset work on removal
Posted by Myeonghun Pak 3 days, 2 hours ago
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