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: <f4d27a4c-df75-42b8-9a1c-3fe2a14666ed@rowland.harvard.edu>
Date: Mon, 17 Nov 2025 23:20:18 -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 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 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 
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.

Alan Stern