linux-kernel - Re: [PATCH v2] usb: dwc3: gadget: Prevent EPs resource conflict during StartTransfer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251121022156.vbnheb6r2ytov7bt@synopsys.com>
Date: Fri, 21 Nov 2025 02:22:02 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Alan Stern <stern@...land.harvard.edu>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Selvarasu Ganesan <selvarasu.g@...sung.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jh0801.jung@...sung.com" <jh0801.jung@...sung.com>,
        "dh10.jung@...sung.com" <dh10.jung@...sung.com>,
        "naushad@...sung.com" <naushad@...sung.com>,
        "akash.m5@...sung.com" <akash.m5@...sung.com>,
        "h10.kim@...sung.com" <h10.kim@...sung.com>,
        "eomji.oh@...sung.com" <eomji.oh@...sung.com>,
        "alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
        "thiagu.r@...sung.com" <thiagu.r@...sung.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [PATCH v2] usb: dwc3: gadget: Prevent EPs resource conflict
 during StartTransfer

On Wed, Nov 19, 2025, Alan Stern wrote:
> On Thu, Nov 20, 2025 at 02:07:33AM +0000, Thinh Nguyen wrote:
> > > Function drivers would have to go to great lengths to guarantee that 
> > > requests had completed before the endpoint is re-enabled.  Right now 
> > > their ->set_alt() callback routines are designed to run in interrupt 
> > > context; they can't afford to wait for requests to complete.
> > 
> > Why is ->set_alt() designed for interrupt context? We can't expect
> > requests to be completed before usb_ep_disable() completes _and_ also
> > expect usb_ep_disable() be able to be called in interrupt context.
> 
> ->set_alt() is called by the composite core when a Set-Interface or 
> Set-Config control request arrives from the host.  It happens within the 
> composite_setup() handler, which is called by the UDC driver when a 
> control request arrives, which means it happens in the context of the 
> UDC driver's interrupt handler.  Therefore ->set_alt() callbacks must 
> not sleep.

This should be changed. I don't think we can expect set_alt() to
be in interrupt context only.

> 
> > > The easiest way out is for usb_ep_disable() to do what the kerneldoc 
> > > says: ensure that pending requests do complete before it returns.  Can 
> > > dwc3 do this?  (And what if at some time in the future we want to start 
> > 
> > The dwc3 can do that, but we need to note that usb_ep_disable() must be
> > executed in process context and might sleep. I suspect we may run into
> > some issues from some function drivers that expected usb_ep_disable() to
> > be executable in interrupt context.
> 
> Well, that's part of what I meant to ask.  Is it possible to wait for 
> all pending requests to be given back while in interrupt context?

The dwc3 controller will need some time (usually less than 2ms) to flush
the endpoints and give back requests. It's probably too long to have
busy poll in interrupt context.

> 
> > > using an asynchronous bottom half for request completions, like usbcore 
> > > does for URBs?)
> > 
> > Which one are you referring to? From what I see, even the host side
> > expected ->endpoint_disable to be executed in process context.
> 
> I was referring to the way usb_hcd_giveback_urb() uses system_bh_wq or 
> system_bh_highpri_wq to do its work.  This makes it impossible for an 
> interrupt handler to wait for a giveback to complete.
> 
> If the gadget core also switches over to using a work queue for request 
> completions, it will then likewise become impossible for an interrupt 
> handler to wait for a request to complete.
> 
> > Perhaps we can introduce endpoint_flush() on gadget side for
> > synchronization if we want to keep usb_ep_disable() to be asynchronous?
> > 
> > > 
> > > Let's face it; the situation is a mess.
> > > 
> > 
> > Glad you're here to help with the mess :)
> 
> To do this right, I can't think of any approach other than to make the 
> composite core use a work queue or other kernel thread for handling 
> Set-Interface and Set-Config calls.  

Sounds like it should've been like this initially.

> 
> It would be nice if there was a way to invoke the ->set_alt() callback 
> that would just disable the interface's endpoints without re-enabling 
> anything.  Then the composite core could disable the existing 
> altsetting, flush the old endpoints, and call ->set_alt() a second time 
> to install the new altsetting and enable the new endpoints.  But 
> implementing this would require us to update every function driver's 
> ->set_alt() callback routine.

Yeah..

> 
> Without that ability, we will have to audit every function driver to 
> make sure the ->set_alt() callbacks do ensure that endpoints are flushed 
> before they are re-enabled.
> 
> There does not seem to be any way to fix the problem just by changing 
> the gadget core.
> 

We can have a workaround in dwc3 that can temporarily "work" with what
we have. However, eventually, we will need to properly rework this and
audit the gadget drivers.

Thanks,
Thinh