| 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: <28035b59-3138-40e6-beb3-1a3793e8df84@samsung.com>
Date: Thu, 11 Dec 2025 16:08:59 +0530
From: Selvarasu Ganesan <selvarasu.g@...sung.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: 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>
Subject: Re: [PATCH v2] usb: dwc3: gadget: Prevent EPs resource conflict
during StartTransfer
On 12/5/2025 6:48 AM, Thinh Nguyen wrote:
> On Fri, Dec 05, 2025, Thinh Nguyen wrote:
>> On Thu, Dec 04, 2025, Selvarasu Ganesan wrote:
>>> On 12/4/2025 7:21 AM, Thinh Nguyen wrote:
>>>> At the moment, I can't think of a way to workaround for all cases. Let's
>>>> just leave bulk streams alone for now. Until we have proper fixes to the
>>>> gadget framework, let's just try the below.
>>>>
>>>
>>> Hi Thinh,
>>>
>>> Thanks for the changes. We understand the given fix and have verified
>>> that the original issue is resolved, but a similar below warning appears
>>> again in `dwc3_gadget_ep_queue` when we run a long duration our test.
>>> And we confirmed this is not due to this new given changes.
>>>
>>> This warning is caused by a race between `dwc3_gadget_ep_disable` and
>>> `dwc3_gadget_ep_queue` that manipulates `dep->flags`.
>>>
>>> Please refer the below sequence for the reference.
>>>
>>> The warning originates from a race condition between
>>> dwc3_gadget_ep_disable and dwc3_send_gadget_ep_cmd from
>>> dwc3_gadget_ep_queue that both manipulate dep->flags. Proper
>>> synchronization or a check is needed when masking (dep->flags &= mask)
>>> inside dwc3_gadget_ep_disable.
>>>
>> I was hoping that the dwc3_gadget_ep_queue() won't come early to run
>> into this scenario. What I've provided will only mitigate and will not
>> resolve for all cases. It seems adding more checks in dwc3 will be
>> more messy.
Hi Thinh,
Thank you for the insightful comments. I agree that adding more checks
directly in the dwc3 driver would be messy, and a comprehensive rework
of the dwc3 ep disable would ultimately be the cleaner solution.
In the meantime, Introducing additional checks for
DWC3_EP_TRANSFER_STARTED in dwc3 driver is the most practical way to
unblock the current issue while we work toward that longer‑term fix.
We have applied the patches and performed additional testing, no
regressions or new issues were observed.
Could you please confirm whether below interim fix is acceptable along
with your proposed earlier patch for unblocking the current development
flow?
Patch 2: usb: dwc3: protect dep->flags from concurrent modify in
dwc3_gadget_ep_disable
=======================================================================================
Subject: [PATCH] usb: dwc3: protect dep->flags from concurrent modify in
dwc3_gadget_ep_disable
The below warnings in `dwc3_gadget_ep_queue` observed during the RNDIS
enable/disable test is caused by a race between `dwc3_gadget_ep_disable`
and `dwc3_gadget_ep_queue`. Both functions manipulate `dep->flags`, and
the lock released temporarily by `dwc3_gadget_giveback` (called from
`dwc3_gadget_ep_disable`) can be acquired by `dwc3_gadget_ep_queue`
before `dwc3_gadget_ep_disable` has finished. This leads to an
inconsistent state of the `DWC3_EP_TRANSFER_STARTED` dep->flag.
To fix this issue by add a condition check when masking `dep->flags`
in `dwc3_gadget_ep_disable` to ensure the `DWC3_EP_TRANSFER_STARTED`
flag is not cleared when it is actually set. This prevents the spurious
warning and eliminates the race.
Thread#1:
dwc3_gadget_ep_disable
->__dwc3_gadget_ep_disable
->dwc3_remove_requests
->dwc3_stop_active_transfer
->__dwc3_stop_active_transfer
-> dwc3_send_gadget_ep_cmd (cmd =DWC3_DEPCMD_ENDTRANSFER)
->if(!interrupt)dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
->dwc3_gadget_giveback
->spin_unlock(&dwc->lock)
...
While Thread#1 is still running, Thread#2 starts:
Thread#2:
usb_ep_queue
->dwc3_gadget_ep_queue
->__dwc3_gadget_kick_transfer
-> starting = !(dep->flags & DWC3_EP_TRANSFER_STARTED);
->if(starting)
->dwc3_send_gadget_ep_cmd (cmd = DWC3_DEPCMD_STARTTRANSFER)
->dep->flags |= DWC3_EP_TRANSFER_STARTED;
...
->__dwc3_gadget_ep_disable
->mask = DWC3_EP_TXFIFO_RESIZED |DWC3_EP_RESOURCE_ALLOCATED;
->dep->flags &= mask; --> // Possible of clears
DWC3_EP_TRANSFER_STARTED flag as well without
sending DWC3_DEPCMD_ENDTRANSFER
------------[ cut here ]------------
dwc3 13200000.dwc3: No resource for ep1in
WARNING: CPU: 7 PID: 1748 at drivers/usb/dwc3/gadget.c:398
dwc3_send_gadget_ep_cmd+0x2f8/0x76c
pc : dwc3_send_gadget_ep_cmd+0x2f8/0x76c
lr : dwc3_send_gadget_ep_cmd+0x2f8/0x76c
Call trace:
dwc3_send_gadget_ep_cmd+0x2f8/0x76c
__dwc3_gadget_kick_transfer+0x2ec/0x3f4
dwc3_gadget_ep_queue+0x140/0x1f0
usb_ep_queue+0x60/0xec
mp_tx_task+0x100/0x134
mp_tx_timeout+0xd0/0x1e0
__hrtimer_run_queues+0x130/0x318
hrtimer_interrupt+0xe8/0x340
exynos_mct_comp_isr+0x58/0x80
__handle_irq_event_percpu+0xcc/0x25c
handle_irq_event+0x40/0x9c
handle_fasteoi_irq+0x154/0x2c8
generic_handle_domain_irq+0x58/0x80
gic_handle_irq+0x48/0x104
call_on_irq_stack+0x3c/0x50
do_interrupt_handler+0x4c/0x84
el1_interrupt+0x34/0x58
el1h_64_irq_handler+0x18/0x24
el1h_64_irq+0x68/0x6c
Change-Id: Ib6a77ce5130e25d0162f72d0e52c845dbb1d18f5
Signed-off-by: Selvarasu Ganesan <selvarasu.g@...sung.com>
---
drivers/usb/dwc3/gadget.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index b42d225b67408..1dc5798072120 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1051,6 +1051,22 @@ static int __dwc3_gadget_ep_disable(struct
dwc3_ep *dep)
*/
if (dep->flags & DWC3_EP_DELAY_STOP)
mask |= (DWC3_EP_DELAY_STOP | DWC3_EP_TRANSFER_STARTED);
+
+ /*
+ * When dwc3_gadget_ep_disable() calls dwc3_gadget_giveback(),
+ * the dwc->lock is temporarily released. If dwc3_gadget_ep_queue()
+ * runs in that window it may set the DWC3_EP_TRANSFER_STARTED flag as
+ * part of dwc3_send_gadget_ep_cmd. The original code cleared the flag
+ * unconditionally, which could overwrite the concurrent modification.
+ *
+ * The added check ensures the DWC3_EP_TRANSFER_STARTED flag is only
+ * cleared if it is not set already, preserving the state updated
by the
+ * concurrent ep_queue path and eliminating the EP resource conflict
+ * warning.
+ */
+ if (dep->flags & DWC3_EP_TRANSFER_STARTED)
+ mask |= DWC3_EP_TRANSFER_STARTED;
+
dep->flags &= mask;
/* Clear out the ep descriptors for non-ep0 */
--
2.31.1
>>
>> Probably we should try rework the usb gadget framework instead of
>> workaround the problem in dwc3. Here is a potential solution I'm
>> thinking: introduce usb_ep_disable_with_flush().
>>
> Actually, no. Let's just revert this:
>
> b0d5d2a71641 ("usb: gadget: udc: core: Revise comments for USB ep enable/disable")
>
> Reword the implementation in dwc3 and audit where usb_ep_disable() is used.
>
> Thanks,
> Thinh
Powered by blists - more mailing lists