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: <20251119014858.5phpkofkveb2q2at@synopsys.com>
Date: Wed, 19 Nov 2025 01:49:12 +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 Mon, Nov 17, 2025, Alan Stern wrote:
> On Tue, Nov 18, 2025 at 02:21:17AM +0000, Thinh Nguyen wrote:
> > Thanks for the catch. The problem is that the "ep_disable" process
> > should be completed after usb_ep_disable is completed. But currently it
> > may be an async call.
> > 
> > This brings up some conflicting wording of the gadget API regarding
> > usb_ep_disable. Here's the doc regarding usb_ep_disable:
> > 
> > 	/**
> > 	 * usb_ep_disable - endpoint is no longer usable
> > 	 * @ep:the endpoint being unconfigured.  may not be the endpoint named "ep0".
> > 	 *
> > 	 * no other task may be using this endpoint when this is called.
> > 	 * any pending and uncompleted requests will complete with status
> > 	 * indicating disconnect (-ESHUTDOWN) before this call returns.
> > 	 * gadget drivers must call usb_ep_enable() again before queueing
> > 	 * requests to the endpoint.
> > 	 *
> > 	 * This routine may be called in an atomic (interrupt) context.
> > 	 *
> > 	 * returns zero, or a negative error code.
> > 	 */
> > 
> > It expects all requests to be completed and given back on completion. It
> > also notes that this can also be called in interrupt context. Currently,
> > there's a scenario where dwc3 may not want to give back the requests
> > right away (ie. DWC3_EP_DELAY_STOP). To fix that in dwc3, it would need
> > to "wait" for the right condition. But waiting does not make sense in
> > interrupt context. (We could busy-poll to satisfy the interrupt context,
> > but that doesn't sound right either)
> > 
> > This was updated from process context only to may be called in interrupt
> > context:
> > 
> > b0d5d2a71641 ("usb: gadget: udc: core: Revise comments for USB ep enable/disable")
> > 
> > 
> > Hi Alan,
> > 
> > Can you help give your opinion on this?
> 
> Well, I think the change to the API was made because drivers _were_ 
> calling these routines in interrupt context.  That's what the commit's 
> description says, anyway.
> 
> One way out of the problem would be to change the kerneldoc for 
> usb_ep_disable().  Instead of saying that pending requests will complete 
> before the all returns, say that the the requests will be marked for 
> cancellation (with -ESHUTDOWN) before the call returns, but the actual 
> completions might happen asynchronously later on.

The burden of synchronization would be shifted to the gadget drivers.
The problem with this is that gadget drivers may modify the requests
after usb_ep_disable() when it should not (e.g. the controller may still
be processing the buffer). Also, gadget drivers shouldn't call
usb_ep_enabled() until the requests are returned.

> 
> The difficulty comes when a gadget driver has to handle a Set-Interface 
> request, or Set-Config for the same configuration.  The endpoints for 
> the old altsetting/config have to be disabled and then the endpoints for 
> the new altsetting/config have to be enabled, all while managing any 

Right.

> pending requests.  I don't know how various function drivers handle 
> this, just that f_mass_storage is very careful about taking care of 
> everything in a separate kernel thread that explicitly dequeues the 
> pending requests and flushes the endpoints.  In fact, this scenario was 
> the whole reason for inventing the DELAYED_STATUS mechanism, because it 
> was impossible to do all the necessary work within the callback routine 
> for a control-request interrupt handler.
> 

If we want to keep usb_ep_disable in interrupt context, should we revise
the wording such that gadget drivers must ensure pending requests are
dequeued before calling usb_ep_disable()? That requests are expected to
be given back before usb_ep_disable().

Or perhaps revert the commit above (commit b0d5d2a71641), fix the dwc3
driver for that, and gadget drivers need to follow the original
requirement.

BR,
Thinh