| 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
| ||
|
Message-ID: <20251119015653.dymmcwebpbrwae5k@synopsys.com>
Date: Wed, 19 Nov 2025 01:56:54 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Selvarasu Ganesan <selvarasu.g@...sung.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Alan Stern <stern@...land.harvard.edu>,
"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>,
"hongpooh.kim@...sung.com" <hongpooh.kim@...sung.com>,
"muhammed.ali@...sung.com" <muhammed.ali@...sung.com>
Subject: Re: [PATCH v2] usb: dwc3: gadget: Prevent EPs resource conflict
during StartTransfer
On Tue, Nov 18, 2025, Selvarasu Ganesan wrote:
>
> On 11/18/2025 7:51 AM, Thinh Nguyen wrote:
> > On Mon, Nov 17, 2025, Selvarasu Ganesan wrote:
> >> The below “No resource for ep” warning appears when a StartTransfer
> >> command is issued for bulk or interrupt endpoints in
> >> `dwc3_gadget_ep_enable` while a previous StartTransfer on the same
> >> endpoint is still in progress. The gadget functions drivers can invoke
> >> `usb_ep_enable` (which triggers a new StartTransfer command) before the
> >> earlier transfer has completed. Because the previous StartTransfer is
> >> still active, `dwc3_gadget_ep_disable` can skip the required
> >> `EndTransfer` due to `DWC3_EP_DELAY_STOP`, leading to the endpoint
> >> resources are busy for previous StartTransfer and warning ("No resource
> >> for ep") from dwc3 driver.
> >>
> >> To resolve this, a check is added to `dwc3_gadget_ep_enable` that
> >> checks the `DWC3_EP_TRANSFER_STARTED` flag before issuing a new
> >> StartTransfer. By preventing a second StartTransfer on an already busy
> >> endpoint, the resource conflict is eliminated, the warning disappears,
> >> and potential kernel panics caused by `panic_on_warn` are avoided.
> >>
> >> ------------[ cut here ]------------
> >> dwc3 13200000.dwc3: No resource for ep1out
> >> WARNING: CPU: 0 PID: 700 at drivers/usb/dwc3/gadget.c:398 dwc3_send_gadget_ep_cmd+0x2f8/0x76c
> >> Call trace:
> >> dwc3_send_gadget_ep_cmd+0x2f8/0x76c
> >> __dwc3_gadget_ep_enable+0x490/0x7c0
> >> dwc3_gadget_ep_enable+0x6c/0xe4
> >> usb_ep_enable+0x5c/0x15c
> >> mp_eth_stop+0xd4/0x11c
> >> __dev_close_many+0x160/0x1c8
> >> __dev_change_flags+0xfc/0x220
> >> dev_change_flags+0x24/0x70
> >> devinet_ioctl+0x434/0x524
> >> inet_ioctl+0xa8/0x224
> >> sock_do_ioctl+0x74/0x128
> >> sock_ioctl+0x3bc/0x468
> >> __arm64_sys_ioctl+0xa8/0xe4
> >> invoke_syscall+0x58/0x10c
> >> el0_svc_common+0xa8/0xdc
> >> do_el0_svc+0x1c/0x28
> >> el0_svc+0x38/0x88
> >> el0t_64_sync_handler+0x70/0xbc
> >> el0t_64_sync+0x1a8/0x1ac
> >>
> >> Fixes: a97ea994605e ("usb: dwc3: gadget: offset Start Transfer latency for bulk EPs")
> >> Cc: stable@...r.kernel.org
> >> Signed-off-by: Selvarasu Ganesan <selvarasu.g@...sung.com>
> >> ---
> >>
> >> Changes in v2:
> >> - Removed change-id.
> >> - Updated commit message.
> >> Link to v1: https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20251117152812.622-1-selvarasu.g@samsung.com/__;!!A4F2R9G_pg!dQNEaMvjzrOiP0c9U6lyj31ysXGkjBAs8c_KjgDCp6ONSZcTrF15DXaJeFTK02v0RLS3w0NQ2K5_pvXak_7c8tIxKL8$
> >> ---
> >> drivers/usb/dwc3/gadget.c | 5 +++--
> >> 1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >> index 1f67fb6aead5..8d3caa71ea12 100644
> >> --- a/drivers/usb/dwc3/gadget.c
> >> +++ b/drivers/usb/dwc3/gadget.c
> >> @@ -963,8 +963,9 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep, unsigned int action)
> >> * Issue StartTransfer here with no-op TRB so we can always rely on No
> >> * Response Update Transfer command.
> >> */
> >> - if (usb_endpoint_xfer_bulk(desc) ||
> >> - usb_endpoint_xfer_int(desc)) {
> >> + if ((usb_endpoint_xfer_bulk(desc) ||
> >> + usb_endpoint_xfer_int(desc)) &&
> >> + !(dep->flags & DWC3_EP_TRANSFER_STARTED)) {
> >> struct dwc3_gadget_ep_cmd_params params;
> >> struct dwc3_trb *trb;
> >> dma_addr_t trb_dma;
> >> --
> >> 2.34.1
> >>
> > 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
>
>
> Hi Thinh,
> Thanks for your comments.
> I agree with you on dwc3 may not want to give back the requests right
> away (ie. DWC3_EP_DELAY_STOP) and might also ignore the End Transfer
> command.
> Consequently, there’s no point in scheduling a new Start Transfer before
> the previous one has completed. The traces below illustrate this: for
> EP1OUT the End Transfer never occurs, so a new Start Transfer is issued,
> which eventually ends with a “No Resource” error.
>
Hi,
In your point, we may as well remove the Start Transfer for all cases.
It actually doesn't impact performance, or required, or needed for No
Response Update Transfer command flow.
The reason we still have that is actually for bulk streams. Your change
actually doesn't fix for the case of bulk streams. (Needed to start and
stop the endpoint to bring it to a certain state as documented in the
same function).
The problem I'm bringing up is related to the dwc3 not really following
the usb_ep_disable() doc in 1 scenario. Depending on how we want to fix
that, then we may not need to change here.
BR,
Thinh
Powered by blists - more mailing lists