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] [day] [month] [year] [list]

Message-ID: <2b944e45-c39a-4c34-b159-ba91dd627fe4@rowland.harvard.edu>
Date: Wed, 19 Nov 2025 22:33:42 -0500
From: Alan Stern <stern@...land.harvard.edu>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: 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 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.

> > 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?

> > 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.  

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.

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.

Alan Stern