* [PATCH AUTOSEL 6.18-5.10] usb_string_copy: Use kzalloc() to avoid leaking old data
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 14:02 ` sashiko-bot
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] usb: typec: mux: tusb1046: add missing MODULE_DEVICE_TABLE() Sasha Levin
` (12 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: David Laight, Greg Kroah-Hartman, Sasha Levin, linux-usb,
linux-kernel
From: David Laight <david.laight.linux@gmail.com>
[ Upstream commit 057df423bf4747091cf942af57307627e941e1fb ]
If the string is read while being updated (which is why the copy is done
in place) and the new string is longer than the old one, then the reader
can read memory that isnt part of either string.
Use memcpy() to copy the known length string instead of strcpy.
Signed-off-by: David Laight <david.laight.linux@gmail.com>
Link: https://patch.msgid.link/20260608095523.2606-37-david.laight.linux@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
# Stable backport analysis: `usb_string_copy: Use kzalloc() to avoid
leaking old data`
Local tree: **v7.3-rc4** (`7.3.0-rc4`, HEAD `f0100363d8c37`). The commit
under review is `057df423bf474`. The buggy `kmalloc` + in-place `strcpy`
path has been in this lineage since **v5.12** (`98f153a10da403`).
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
- Subsystem: USB gadget configfs (`usb_string_copy` in
`drivers/usb/gadget/configfs.c`)
- Action verb: **Use** (kzalloc) / implied **avoid leaking**
- Claimed intent: stop readers from observing memory that is not part of
either the old or new string when an in-place update lengthens the
buffer
Record: `[usb/gadget/configfs] [use/avoid] [zero the string buffer and
copy a known length to prevent leaking heap bytes during in-place
updates]`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
- `Signed-off-by: David Laight <david.laight.linux@gmail.com>` (author)
- `Link: https://patch.msgid.link/20260608095523.2606-37-
david.laight.linux@gmail.com`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (USB
maintainer applied it)
- No `Fixes:`, no `Reported-by:`, no `Tested-by:`, no `Reviewed-by:`, no
`Acked-by:`, no `Cc: stable` (absence is expected)
- Message-Id `2606-37` = patch 37 of a large series
Record: Author SOB + Greg SOB + lore Link. No reporters. Hardening/USB
lists were the original audience (confirmed in Phase 4).
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
The body describes a concurrent-read hole in the in-place update path:
if a reader walks the buffer while a longer string is being copied, it
can pass the old NUL and consume bytes that belong to neither string. It
also switches `strcpy` to `memcpy` of the already-computed length.
Record:
- Bug: kernel heap contents can be observed by a concurrent reader of a
gadget string during an in-place lengthening update
- Symptom: leaked uninitialized `kmalloc` bytes (via configfs show or
USB string descriptors)
- Version info: none in the message
- Root cause: first allocation used `kmalloc` (unzeroed tail); in-place
`strcpy` of a longer string temporarily removes the old NUL before the
new one is written
**Step 1.4: DETECT HIDDEN BUG FIXES**
This is not disguised cleanup. The subject states the leak. It *is* one
patch in a treewide `strcpy` sweep, and `memcpy` is that sweep;
`kzalloc` is the actual leak fix.
Record: `[Not hidden. Real infoleak fix, bundled with strcpy-
elimination.]`
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
- File: `drivers/usb/gadget/configfs.c` (+2 / −2)
- Function: `usb_string_copy()`
- Scope: single-file, two-line surgical change
Record: `[configfs.c +2/-2] [usb_string_copy] [single-file surgical]`
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Before:
- First call: `kmalloc(USB_MAX_STRING_WITH_NULL_LEN)` then `strcpy`
- Later calls: reuse the same buffer (`str = copy`) and `strcpy` over it
(the 2021 UAF workaround)
After:
- First call: `kzalloc` so the unused tail is zeros
- Copy: `memcpy(str, s, ret + 1)` (`ret` is `strlen(s)`, so this
includes the NUL)
Affected path: configfs store of
manufacturer/product/serial/configuration strings (`GS_STRINGS_W`).
Record: `[kmalloc+strcpy -> kzalloc+memcpy of known length; first-alloc
and in-place update paths]`
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Category: **uninitialized data / kernel infoleak** (with a concurrent-
read window created by in-place update).
Mechanism, verified in this tree:
1. `98f153a10da403` (v5.12) stopped `kfree`+`kstrdup` and copies in
place so USB host pointers stay valid.
2. First fill still used `kmalloc`, so bytes after the first NUL are
heap residue.
3. Bind aliases those buffers into USB string tables:
- `stringtab[...].s = language->manufacturer/product/serialnumber`
(`configfs_attach_gadget_strings`)
- `cn->strings.s = cn->configuration` (`configfs_composite_bind`)
4. `usb_gadget_get_string()` does `strlen(s->s)` then converts to
UTF-16.
5. Configfs show is `sprintf(page, "%s\n", gs->__name ?: "")`.
6. `GS_STRINGS_W` calls `usb_string_copy()` with **no** `gi->lock`. Host
`GET_DESCRIPTOR` is not serialized against that store.
7. If a reader is in `strlen`/`sprintf` while `strcpy` lengthens the
string, the old NUL is overwritten first; the walk continues into the
unzeroed tail.
`kzalloc` makes that tail zeros, so a torn read stops instead of leaking
heap. `memcpy` is equivalent to `strcpy` here (`ret` already came from
`strlen`).
Record: `[uninitialized kmalloc tail leaked across a designed
concurrent-read window] [kzalloc zeros the tail; memcpy is strcpy
replacement]`
**Step 2.4: ASSESS THE FIX QUALITY**
Obviously correct. `USB_MAX_STRING_LEN` is 126, so max `ret+1` is 127,
matching `USB_MAX_STRING_WITH_NULL_LEN`. `kzalloc` is strictly safer
than `kmalloc`. `memcpy` of `strlen+1` matches `strcpy` for a C string.
Regression risk: very low. Remaining torn-string reads of *previous
string contents* are the pre-existing in-place design, not introduced
here.
Record: `[minimal, correct] [regression risk very low]`
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
- Function added: `88af8bbe4ef781` (2012, “the start of the configfs
interface”) — originally `kstrdup` + `kfree(old)`
- In-place `kmalloc` + `strcpy`: `98f153a10da403` (2021-03-17, in v5.12)
— **this is the buggy code**
- Empty-string guard: `6d3c721e686ea` (2024, `Cc: stable`)
- This fix: `057df423bf474` (2026-06-08)
Record: `[98f153a10da403, v5.12] [bug present for years in this
lineage]`
**Step 3.2: FOLLOW THE FIXES: TAG**
No `Fixes:` tag. The semantic Fixes target is `98f153a10da403`, which
**is** in this tree (`git merge-base --is-ancestor` succeeded). That
commit was itself `Cc: stable` and exists because USB host string reads
race with configfs stores.
Record: `[No Fixes tag] [real introducer 98f153a is in this tree since
v5.12]`
**Step 3.3: CHECK FILE HISTORY**
Recent related commits on this file: OOB empty-string fixes
(`6d3c721e686ea`, `3014168731b79`), later `f63edb54d8f73` (qw_sign,
unrelated). No prerequisite refactor is required for these two lines.
`kmalloc_obj` treewide conversion did **not** change this `kmalloc`
call; the pre-image is still `kmalloc(...)`.
Record: `[standalone vs file history] [no blocking prerequisites in this
tree]`
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
David Laight is not the USB gadget maintainer. Around this commit he
posted ~100 compile-tested `strcpy` replacements to linux-hardening /
subsystem lists. Greg applied this one. Sibling leak-style changes exist
(e.g. TIPC pad bytes).
Record: `[regular contributor, hardening series] [Greg applied; not a
maintainer-authored USB patch]`
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Needs the in-place buffer (`98f153a`) to be meaningful; that is in this
tree. Does not need `6d3c721e` (empty-string check) for correctness;
that only appears as nearby context. Applies standalone.
Record: `[standalone on any tree that has the 2021 in-place copy] [can
apply standalone here]`
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
`b4 dig -c 057df423bf474` matched:
https://patch.msgid.link/20260608095523.2606-37-
david.laight.linux@gmail.com
`b4 dig -a`: only **v1**, dated 2026-06-08, subject `[PATCH next]`.
Saved mbox: patch only, **no replies**. Trailer notes:
- one of a group removing unbounded `strcpy()`
- **“all the changes are only compile tested”**
- ~100 patches sent individually
- “Apologies for the terse commit messages”
WebFetch to lore.kernel.org returned **403**. Discussion content was
recovered via `b4 dig -m`.
Record: `[lore msgid 20260608095523.2606-37] [v1 only, no review
comments in thread] [no reviewer Cc: stable] [compile-tested series]`
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
`b4 dig -w` original recipients: Kees Cook, linux-hardening, linux-
kernel, linux-usb, Arnd Bergmann, Greg Kroah-Hartman. Greg’s merge SOB
is the only review signal in git. No `Reviewed-by`/`Acked-by` on the
commit.
Record: `[USB + hardening lists and Greg CC’d; Greg applied; no written
review in the thread]`
**Step 4.3: SEARCH FOR THE BUG REPORT**
No `Reported-by`, no syzbot, no bugzilla. The race is inferred from code
and from `98f153a`’s real KASAN UAF on the same overlapping access.
Record: `[no external bug report] [concurrent access is documented by
the 2021 UAF fix, not by a new user report]`
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Patch 37 of a ~100-patch `strcpy` series aimed at linux-next. This
instance is one of the few that also changes allocation to `kzalloc` and
describes a leak. No other series member is required for this function.
Record: `[strcpy series; this patch is self-contained]`
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
lore.kernel.org/stable search via WebFetch and curl returned **403**.
Cannot confirm a stable-list nomination or rejection.
Record: `[UNVERIFIED: stable@ discussion]`
---
## PHASE 5: CODE SEMANTIC ANALYSIS
semcode MCP is not available. Used grep + file reads.
**Step 5.1: KEY FUNCTIONS**
Record: `[usb_string_copy]`
**Step 5.2: CALLERS**
Direct calls only through `GS_STRINGS_W` in
`include/linux/usb/gadget_configfs.h`, instantiated in `configfs.c` as:
- `gadget_language_{manufacturer,product,serialnumber}_store`
- `gadget_config_name_configuration_store`
Those are configfs attribute stores (privileged gadget setup). Host-side
readers are `usb_gadget_get_string()` → `strlen` on the aliased buffers
after bind.
Record: `[configfs stores write; USB GET_STRING and configfs show read
the same buffers]`
**Step 5.3: CALLEES**
`strlen`, `kzalloc`/`kmalloc`, `memcpy`/`strcpy`. No locks, no I/O.
Record: `[string copy + slab alloc only]`
**Step 5.4: CALL CHAIN / REACHABILITY**
Userspace with configfs gadget access: `write(2)` →
`configfs_write_file` → `GS_STRINGS_W` → `usb_string_copy`.
USB host: `GET_DESCRIPTOR` / string request → `composite_setup` →
`usb_gadget_get_string` → `strlen(s->s)`.
Reachable. Typical writers are root (Android/embedded gadgetd). The
**host** is a different trust domain.
Record: `[reachable from configfs writes and from USB host string reads]
[host path is the security-relevant reader]`
**Step 5.5: SIMILAR PATTERNS**
Same file already has other string-safety fixes (`6d3c721e`,
`3014168731b79`). Author’s series has other “don’t leak leftover bytes”
patches. No second copy of this exact helper.
Record: `[isolated helper; same file has prior stable-bound string
bugs]`
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Yes. Pre-image at `057df423bf474^` still has `kmalloc` + `strcpy`.
Introduced v5.12, present through this 7.3-rc4 lineage until this
commit.
Record: `[yes, usb_string_copy in drivers/usb/gadget/configfs.c] [since
v5.12 / 98f153a]`
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Hunk is two lines inside a long-stable helper. Would apply cleanly here
(and would apply to older trees that still have `kmalloc`+`strcpy`;
empty-string check is only nearby context).
Record: `[clean apply]`
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
`6d3c721e` is a different bug (OOB on zero-length write). No other
kzalloc leak fix for this helper.
Record: `[no substitute fix already present before 057df423]`
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: SUBSYSTEM AND CRITICALITY**
USB gadget configfs (`CONFIG_USB_CONFIGFS`). Used to compose gadgets
from userspace (Android gadgets, gadgetfs-style gadgets, UDC testing).
Not core mm/VFS, but a common embedded/Android path.
Record: `[USB gadget configfs] [IMPORTANT for gadget/Android; PERIPHERAL
for desktops without UDC]`
**Step 7.2: SUBSYSTEM ACTIVITY**
File still receives security-relevant string fixes in 2024–2026. Mature
interface, not staging.
Record: `[mature, still getting bounded-string fixes]`
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Gadget systems that set manufacturer/product/serial/configuration via
configfs and have the gadget bound so a host can read string
descriptors. Config-specific: `CONFIG_USB_CONFIGFS`.
Record: `[driver/config-specific: USB gadget configfs users]`
**Step 8.2: TRIGGER CONDITIONS**
Concurrent in-place store of a **longer** string while
`strlen`/`sprintf` is walking the buffer. Window is a 127-byte copy —
short, but the overlap is intentional (`98f153a` exists because host
reads and configfs writes do overlap in practice). Unprivileged local
users generally cannot write these attributes; a USB host can read the
leaked bytes.
Record: `[short race during string lengthening while gadget is live]
[USB host can observe the leak]`
**Step 8.3: FAILURE MODE SEVERITY**
Kernel heap infoleak (uninitialized `kmalloc` bytes), not a crash. Can
cross to a USB host via string descriptors. That is a security issue,
not a panic.
Record: `[kernel heap infoleak to USB host / configfs reader] [HIGH for
security, not CRITICAL crash]`
**Step 8.4: RISK-BENEFIT RATIO**
- Benefit: closes a real infoleak on a path already known to race; 2
lines; `kzalloc` is the usual stable-grade fix
- Risk: negligible (`kzalloc`/`memcpy` behavior-preserving for this
helper)
Record: `[benefit high for gadget infoleak] [risk very low] [favorable]`
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real uninitialized-kmalloc leak, not style-only
- Concurrent reader is why in-place copy exists (`98f153a`, KASAN UAF,
`Cc: stable`)
- Host `strlen` on aliased pointers is a verified path
- Two-line, obviously correct change
- No new API/feature
- Bug present since v5.12 in this tree’s history
- Applied by Greg KH
AGAINST:
- Tiny race window; no syzbot/CVE/user report
- Series was compile-tested only, labeled `[PATCH next]`
- No mailing-list review comments
- Not a universal core-kernel path
- `memcpy` half is strcpy-hardening, not the leak fix
UNRESOLVED:
- Whether stable@ ever discussed this (lore 403)
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — correctness is local and
obvious; series is compile-tested only, but the change cannot
reasonably be wrong
2. Fixes a real bug that affects users? **PASS** — heap bytes can be
read on a live gadget string update
3. Important issue? **PASS** — kernel infoleak (security), not a
crash/deadlock
4. Small and contained? **PASS** — 2 lines, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — pre-image matches; no missing
helpers
**Step 9.3: EXCEPTION CATEGORIES**
Not a device-ID/quirk/DT/build/docs exception. It is a small
security/correctness fix.
**Step 9.4: DECISION**
This is a contained infoleak fix on a helper whose in-place update
exists specifically so USB hosts can read strings while configfs writes
them. `kzalloc` is the right, low-risk close of that hole. It meets
stable rules for this tree.
---
## What the commit solves for stable users
After the 2021 UAF fix, gadget strings live in a reused `kmalloc`
buffer. A host (or configfs reader) can walk that buffer while a longer
string is copied in place, skip the old NUL, and return leftover heap
data in a USB string descriptor. Zeroing on first allocation stops that
leak. `memcpy` is a safe `strcpy` substitute given the existing
`strlen`.
Risk vs benefit: two lines, no API change, no lock surgery, benefit is
closing a kernel-memory leak on Android/embedded gadget configs.
---
## Verification
- [Phase 1] Parsed tags: author SOB, Greg SOB, lore Link; no
Fixes/Reported-by/Cc: stable
- [Phase 2] Diff: `kmalloc`→`kzalloc`, `strcpy`→`memcpy(str,s,ret+1)` in
`usb_string_copy`
- [Phase 2] Confirmed `USB_MAX_STRING_LEN` 126 and buffer size 127;
`ret+1` cannot overflow
- [Phase 3] `git blame`: in-place `kmalloc` from `98f153a10da403`
(v5.12); function from `88af8bbe4ef781` (2012)
- [Phase 3] `git merge-base --is-ancestor 98f153a10da403 HEAD` —
introducer is in this tree
- [Phase 3] `git merge-base --is-ancestor 057df423bf474 HEAD` — this
commit is already in HEAD (ignored as “already selected”)
- [Phase 3] `git describe HEAD` / Makefile: local tree is **v7.3-rc4 /
7.3.0-rc4**
- [Phase 3] Pre-commit file still has `kmalloc`+`strcpy`
- [Phase 4] `b4 dig -c 057df423bf474`: msgid `20260608095523.2606-37`,
`[PATCH next]`, v1 only
- [Phase 4] `b4 dig -w`: linux-usb, linux-hardening, Kees, Greg, Arnd
- [Phase 4] `b4 dig -m`: no replies; author said compile-tested only;
~100 strcpy patches
- [Phase 4] WebFetch lore.kernel.org: 403; used b4 instead
- [Phase 5] Callers: `GS_STRINGS_W` →
manufacturer/product/serial/configuration stores
- [Phase 5] Host path: `stringtab[].s` and `cn->strings.s` alias the
same buffers; `usb_gadget_get_string` uses `strlen`
- [Phase 5] `GS_STRINGS_W` does not take `gi->lock`
- [Phase 6] No other leak fix for this helper; `6d3c721e` is a different
OOB bug
- [Phase 6] Backport difficulty: clean 2-line apply
- [Phase 8] Failure mode: uninitialized heap infoleak, severity HIGH
(security), not a crash
- UNVERIFIED: stable@ mailing-list discussion (lore 403)
- UNVERIFIED: no runtime reproduction performed here; race is from code
+ the 2021 UAF report, not a new reproducer
**YES**
drivers/usb/gadget/configfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index aa0a25efb6aa0..b6237a2e02003 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -125,11 +125,11 @@ static int usb_string_copy(const char *s, char **s_copy)
if (copy) {
str = copy;
} else {
- str = kmalloc(USB_MAX_STRING_WITH_NULL_LEN, GFP_KERNEL);
+ str = kzalloc(USB_MAX_STRING_WITH_NULL_LEN, GFP_KERNEL);
if (!str)
return -ENOMEM;
}
- strcpy(str, s);
+ memcpy(str, s, ret + 1);
if (str[ret - 1] == '\n')
str[ret - 1] = '\0';
*s_copy = str;
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.18] usb: typec: mux: tusb1046: add missing MODULE_DEVICE_TABLE()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-5.10] usb_string_copy: Use kzalloc() to avoid leaking old data Sasha Levin
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 14:03 ` sashiko-bot
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.1] thunderbolt: xdomain: Notify peers after enumeration Sasha Levin
` (11 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Romain Gantois, Greg Kroah-Hartman, Sasha Levin,
heikki.krogerus, linux-usb, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit 5e4eb5c96324b226220d29d025a334df28054ed3 ]
The driver has an OF match table wired to .of_match_table, but does
not export the table with MODULE_DEVICE_TABLE().
Add the missing MODULE_DEVICE_TABLE(of, ...) entry so module alias
information is generated for OF based module autoloading.
This is a source-level fix. It does not claim dynamic hardware
reproduction; the evidence is the driver-owned match table, its use by
the platform driver, and the missing module alias publication.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Romain Gantois <romain.gantois@bootlin.com>
Link: https://patch.msgid.link/20260704124714.18404-1-pengpeng@iscas.ac.cn
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **v7.3-rc4**. The tusb1046 driver is present, and this
is a one-line OF module-alias publication so udev can autoload the
driver. That is a real modular-build bug, not a new feature.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[usb: typec: mux: tusb1046]** **[add]** missing
`MODULE_DEVICE_TABLE()` so OF module aliases are generated.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for this review)
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** Romain Gantois \<romain.gantois@bootlin.com\> (driver
author / listed maintainer)
- **Acked-by:** none in the commit; mailing-list thread also has
**Reviewed-by: Heikki Krogerus** (USB Type-C maintainer) that did not
land in the commit text
- **Link:**
https://patch.msgid.link/20260704124714.18404-1-pengpeng@iscas.ac.cn
- **Cc: stable@vger.kernel.org:** absent (expected)
- **Signed-off-by:** Pengpeng Hou (author); Greg Kroah-Hartman (USB
maintainer, merged)
- Ignore any pipeline “already selected” / Sasha Levin markers (none in
this commit object)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `tusb1046_match_table` is wired to `.of_match_table` but not
exported with `MODULE_DEVICE_TABLE(of, ...)`.
- **Symptom:** when `CONFIG_TYPEC_MUX_TUSB1046=m`, `depmod` does not
emit `of:N*T*Cti,tusb1046` aliases, so udev/kmod will not autoload the
module when DT instantiates the I2C device.
- **Failure mode:** Type-C mux / orientation switch stays unbound until
a manual `modprobe`. Built-in (`=y`) is unaffected.
- **Version info:** none in the message.
- **Root cause:** missing module alias publication; match table itself
is correct.
- The “source-level fix / no dynamic hardware reproduction” sentence is
in both the sent patch and the committed message.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. It is an explicit autoload/alias bug. Same class as
device-ID publication: no new behavior, just making existing OF matching
visible to userspace module loading.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/usb/typec/mux/tusb1046.c` — **+1 / −0**
- **Function:** none; one line after `tusb1046_match_table[]`
- **Scope:** single-file, one-line surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** OF table exists and is used for in-kernel matching if the
module is already loaded; file2alias never sees it.
- **After:** `MODULE_DEVICE_TABLE(of, tusb1046_match_table)` creates the
`__mod_of__tusb1046_match_table` alias consumed by
`scripts/mod/file2alias.c`.
- **Path:** build-time module metadata only; probe/remove/mux paths
unchanged.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** hardware matching / module autoload (device-ID
publication), not UAF/race/leak.
- **Mechanism:** OF compatible `ti,tusb1046` was never exported in the
module’s device table, so autoload could not work.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: this is the standard macro used by sibling Type-C
mux drivers (`fsa4480`, `ptn36502`, `it5205`, `nb7vpq904m`, etc.).
- Minimal; no API, locking, or runtime change.
- Regression risk: negligible. Wrong table/type would be a build
concern; here type `of` matches `struct of_device_id`.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Match table introduced in **7c561b8c3e836** (“usb: typec: mux: Add
support for the TUSB1046 crosspoint switch”), **Romain Gantois**,
2024-10-24, first contained in **v6.13-rc1**.
- Original driver had `.of_match_table = tusb1046_match_table` and
**no** `MODULE_DEVICE_TABLE`. Bug present from introduction.
- Candidate: **5e4eb5c96324b226220d29d025a334df28054ed3**, first
contained in **v7.3-rc1**.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Buggy commit is the driver intro
**7c561b8c3e836**, which **is** an ancestor of this HEAD.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record (on this file): driver add → `dev_err_probe` cleanup → include-
header reshuffle → this alias fix. Standalone; not patch x/y. No
prerequisite refactor required for the one-line add.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Pengpeng Hou is a contributor, not the Type-C mux maintainer.
Nearby USB work is unrelated. Weight comes from **Romain Gantois**
(driver maintainer) and **Heikki Krogerus** (Type-C) reviews, plus Greg
KH merge.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. `MODULE_DEVICE_TABLE` exists in `include/linux/module.h`
in this tree. Applies standalone on any tree that has
`tusb1046_match_table`.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig -c 5e4eb5c96324b:**
https://patch.msgid.link/20260704124714.18404-1-pengpeng@iscas.ac.cn
- **b4 dig -a:** single **v1**, 2026-07-04, no later revision.
- Thread (mbox): Romain Gantois “LGTM, thanks!” + R-b (2026-07-07);
Heikki Krogerus R-b (2026-07-13, after Greg’s 2026-07-08 commit). **No
NAKs, no stable discussion, no risk flags.**
- WebFetch of lore.kernel.org returned **403**; thread recovered via `b4
dig -m`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: **b4 dig -w** original To/Cc: Pengpeng Hou, Romain Gantois,
Heikki Krogerus, Greg Kroah-Hartman, linux-usb, linux-kernel. Right
maintainers were on the patch.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla/Reported-by. Justification is the missing
macro vs an existing OF table. No in-tree DTS consumer of `ti,tusb1046`
(only `Documentation/devicetree/bindings/usb/ti,tusb1046.yaml`). User
impact is boards that describe this chip in DT, including out-of-tree
DTS.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone v1. Same pattern already used in this tree:
**15a34e5d4b23c** (“usb: typec: fix module autoloading”),
**f27891abb7930** (“usb: typec: add missing MODULE_DEVICE_TABLE() to
stusb160x”).
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** — lore.kernel.org/stable WebFetch 403; WebSearch
failed. No stable nomination in the recovered linux-usb thread.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: No function body changed. Data: `tusb1046_match_table`; driver:
`tusb1046_driver` / `module_i2c_driver()`.
**Step 5.2: TRACE CALLERS**
Record: I2C core binds via `module_i2c_driver`. OF match is used once
the module is loaded. Autoload is userspace (udev) using aliases from
`modules.alias`. Reachable on DT platforms with `compatible =
"ti,tusb1046"`.
**Step 5.3: TRACE CALLEES**
Record: N/A for the added line. Macro expands to a static alias
attribute for modpost.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: DT I2C instantiation → udev/kmod looks up OF alias → `tusb1046`
module → `tusb1046_i2c_probe`. Userspace cannot trigger a crash via this
bug; the failure is **driver never loads**.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Other Type-C mux drivers already have `MODULE_DEVICE_TABLE(of,
...)`. This driver was the one that omitted it. Semcode MCP not
available; used `git grep`.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- **Tree:** `git describe HEAD` = **v7.3-rc4-27-gf0100363d8c37**; `make
kernelversion` = **7.3.0-rc4**.
- Driver file exists; Kconfig `TYPEC_MUX_TUSB1046` is **tristate**.
- Bug introduced **v6.13**; this 7.3 lineage includes that driver.
- **HEAD already contains 5e4eb5c96324b** (`git merge-base --is-
ancestor` succeeded). Current file already has line 181
`MODULE_DEVICE_TABLE(of, tusb1046_match_table)`. That is mainline
history, not a reason to reject on technical merit.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: On **current HEAD**, `git apply --check` of the commit **fails**
because the line is already present. Against the pre-fix file (parent of
5e4eb5c96324b) the hunk is a clean one-line insert after the match
table. Any stable tree that still has the original
`tusb1046_match_table` without the macro would take it cleanly.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This exact fix is already in HEAD. No alternate autoload
workaround in the driver. Sibling Type-C autoload fixes are also present
and do not replace this line.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **USB Type-C mux driver** (`drivers/usb/typec/mux/`).
**PERIPHERAL** — one I2C crosspoint switch, not mm/VFS/net core. Still
user-visible USB-C / DP altmode on boards that have the chip.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Type-C mux is actively developed (ps883x and related churn).
`tusb1046.c` itself is low-churn since v6.13.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific + config-specific:**
`CONFIG_TYPEC_MUX_TUSB1046=m` on OF/I2C systems with `ti,tusb1046`. Not
universal. No in-tree DTS users found; out-of-tree / vendor DT is the
realistic consumer.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Every boot/enumeration of that DT node when the driver is a
module. Not a race. Unprivileged users do not exploit it; they just do
not get the mux driver.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Failure mode:** module does not autoload; USB-C orientation/DP
muxing does not come up until manual load. **Severity: MEDIUM**
(functional hardware enablement, not oops/corruption/deadlock). Built-in
builds: no bug.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for TUSB1046 modular users (device actually probes);
moderate overall (narrow hardware, no in-tree board).
- **Risk:** very low (1 line, metadata only).
- **Ratio:** favorable — same justification as adding a device ID to an
existing driver.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR backporting:
- Real autoload bug for a tristate driver with an OF table.
- One line, obviously correct, no runtime path change.
- Reviewed by the driver author and Type-C maintainer; merged by Greg
KH.
- Same pattern already used in this Type-C subsystem.
- Fits the stable exception for publishing match IDs so existing
hardware actually binds.
- Driver and match table exist in this 7.3 tree (introduced v6.13).
AGAINST:
- Not a crash, UAF, deadlock, or security issue.
- No user `Reported-by` and no in-tree DTS enabling the compatible.
- “Tested” is source-level, not hardware reproduction.
- HEAD already contains the commit (expected on 7.3-rc; not a pipeline-
selection marker).
UNRESOLVED:
- Could not query lore.kernel.org/stable (HTTP 403 / search failure).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — standard macro; maintainer
R-b. Hardware test not claimed.
2. Fixes a real bug that affects users? **PASS** — modular OF autoload
is broken.
3. Important issue? **PASS** as hardware enablement (autoload),
analogous to new device IDs; not CRITICAL.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features or APIs? **PASS** — exports an existing table.
6. Can apply to the local tree? **PASS** for this codebase (driver
present). Re-applying on current HEAD conflicts only because the fix
is already merged here; a tree that still lacks the line applies
cleanly.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Device ID / module alias publication** for an existing driver
(OF `MODULE_DEVICE_TABLE`). Also build-time metadata, not a new driver.
**Step 9.4: MAKE YOUR DECISION**
This tree has the TUSB1046 driver. Missing `MODULE_DEVICE_TABLE(of,
...)` prevents automatic loading of a modular Type-C mux, so USB-C/DP
switching does not come up on DT systems until someone loads the module
by hand. The fix is one obviously correct line with essentially no
regression risk. That meets stable rules and the device-ID exception, so
it should be backported to this tree’s stable series.
===============================================================
What the commit solves for stable users
===============================================================
With `TYPEC_MUX_TUSB1046=m`, kmod never learns the `ti,tusb1046` OF
alias, so the I2C Type-C mux never autoprobes. Distro/embedded kernels
that ship this as a module then lose orientation switch and DisplayPort
muxing on that hardware. The patch only emits the alias; it does not
change probe logic.
Risk is negligible. Benefit is real for modular users of this chip. No
extra commits are required beyond the driver that this 7.3 tree already
has.
===============================================================
Verification
===============================================================
- [Phase 1] Parsed commit **5e4eb5c96324b**: R-b Romain Gantois; Link
patch.msgid.link; no Fixes/Reported-by/Cc:stable.
- [Phase 2] Diff: **+1** `MODULE_DEVICE_TABLE(of, tusb1046_match_table)`
in `drivers/usb/typec/mux/tusb1046.c`.
- [Phase 2] Confirmed sibling mux drivers already use
`MODULE_DEVICE_TABLE(of, ...)`.
- [Phase 3] `git blame`: table from **7c561b8c3e836** (v6.13-rc1);
original add had no `MODULE_DEVICE_TABLE`.
- [Phase 3] `git describe --contains 7c561b8c3e836` → v6.13-rc1; `git
describe --contains 5e4eb5c96324b` → v7.3-rc1.
- [Phase 3] `git merge-base --is-ancestor 5e4eb5c96324b HEAD` → yes;
`7c561b8c3e836` → yes.
- [Phase 3] `git show 7c561b8c3e836` confirms OF table without the
macro.
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 only; R-b Gantois + Krogerus;
recipients include Type-C and USB maintainers; no NAK.
- [Phase 4] lore WebFetch 403; used mbox instead.
- [Phase 5] Kconfig `TYPEC_MUX_TUSB1046` is tristate; Makefile builds
`tusb1046.o`; DT binding `ti,tusb1046.yaml` exists.
- [Phase 5] `git grep ti,tusb1046 arch/` → no in-tree DTS.
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-g…; kernelversion
7.3.0-rc4.
- [Phase 6] HEAD already has the line; `git apply --check` of the patch
on HEAD fails; parent of the commit lacks the macro.
- [Phase 7] MAINTAINERS: USB TYPEC TUSB1046 MUX DRIVER → Romain Gantois.
- [Phase 8] Failure = missing autoload, severity MEDIUM; risk very low.
- UNVERIFIED: stable@ discussion (lore/stable unreachable).
**YES**
drivers/usb/typec/mux/tusb1046.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/typec/mux/tusb1046.c b/drivers/usb/typec/mux/tusb1046.c
index 3c1a4551c2fbf..89b47aa56c7a9 100644
--- a/drivers/usb/typec/mux/tusb1046.c
+++ b/drivers/usb/typec/mux/tusb1046.c
@@ -179,6 +179,7 @@ static const struct of_device_id tusb1046_match_table[] = {
{.compatible = "ti,tusb1046"},
{},
};
+MODULE_DEVICE_TABLE(of, tusb1046_match_table);
static struct i2c_driver tusb1046_driver = {
.driver = {
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.18-6.1] thunderbolt: xdomain: Notify peers after enumeration
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-5.10] usb_string_copy: Use kzalloc() to avoid leaking old data Sasha Levin
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] usb: typec: mux: tusb1046: add missing MODULE_DEVICE_TABLE() Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 14:17 ` sashiko-bot
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] usb: fsl_qe_udc: check qe_alloc_request() failure in ch9getstatus() Sasha Levin
` (10 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Milo Chen, Mika Westerberg, Sasha Levin, andreas.noever, westeri,
YehezkelShB, linux-usb, linux-kernel
From: Milo Chen <cmh79479@gmail.com>
[ Upstream commit e027dba038f0008df9bc9575f5c3e803e90636c6 ]
Service drivers may register local XDomain properties while discovery is
still in progress. This can cause the properties changed notification to be
sent before the peer is ready to act on it.
If the peer has already read the local property block before the service
was registered, it may keep using the old property generation and miss the
newly registered service. With ThunderboltIP this can leave the network
service half-discovered after a warm reboot and the login request
eventually times out.
Queue another properties changed notification after the XDomain reaches
ENUMERATED so the peer can re-read the final local properties.
Signed-off-by: Milo Chen <cmh79479@gmail.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
candidate is a one-line Thunderbolt XDomain handshake fix: after
discovery reaches `ENUMERATED`, send another properties-changed
notification so a peer can pick up services registered during discovery.
That matches a real ThunderboltIP failure after warm reboot, and the
surrounding helpers already exist here.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
- Record: `[thunderbolt: xdomain:] [Notify] [send properties-changed to
peers once XDomain discovery reaches ENUMERATED]`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
- Record:
- `Signed-off-by: Milo Chen <cmh79479@gmail.com>` (author)
- `Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>`
(Thunderbolt maintainer; applied the patch)
- No `Fixes:`
- No `Reported-by:` / `Tested-by:` / `Reviewed-by:` / `Acked-by:` /
`Link:` / `Cc: stable`
- Absence of `Fixes:` / `Cc: stable` is expected for this review and
is not a negative signal
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
- Record:
- **Bug:** Service drivers may register local XDomain properties while
discovery is still running. The properties-changed notification can
go out before the peer can act on it.
- **Symptom:** Peer keeps an old property generation and misses the
new service. With ThunderboltIP, the network service stays half-
discovered after a **warm reboot** and the **login request times
out**.
- **Root cause:** Notification is sent too early (during discovery);
nothing is sent once the local host is fully enumerated.
- **Fix:** Queue another properties-changed notification when the
XDomain reaches `ENUMERATED`.
- **Version info:** none in the message.
**Step 1.4: DETECT HIDDEN BUG FIXES**
- Record: Not a disguised cleanup. The message describes a concrete
functional failure (ThunderboltIP login timeout) and a protocol-timing
fix.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
- Record:
- File: `drivers/thunderbolt/xdomain.c` (`+1 / -0`)
- Function: `tb_xdomain_state_work()`
- Scope: single-file, one-line surgical change
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
- Record:
- **Before:** `XDOMAIN_STATE_PROPERTIES` success only sets `xd->state
= XDOMAIN_STATE_ENUMERATED`.
- **After:** same, then `tb_xdomain_queue_properties_changed(xd)`.
- Path: successful remote property fetch at the end of XDomain
discovery (normal path, not an error path).
**Step 2.3: IDENTIFY THE BUG MECHANISM**
- Record:
- **Category:** logic / protocol handshake correctness (missed peer
notification).
- **Mechanism:** `tb_xdomain_queue_properties_changed()` already runs
in `XDOMAIN_STATE_INIT` and after UUID. `tb_register_property_dir()`
(used by `thunderbolt-net`) also notifies via
`update_all_xdomains()`. If that happens while the peer is not
ready, or after the peer already read the old block, the peer can
stick on the old generation. There was no notify after local
discovery finished. The new call reuses the existing notify helper
at ENUMERATED so the peer re-reads the final block.
**Step 2.4: ASSESS THE FIX QUALITY**
- Record:
- **Quality:** Obviously correct relative to the described race. It
reuses `tb_xdomain_queue_properties_changed()`, which already sends
`PROPERTIES_CHANGED_REQUEST` with retries.
- **Minimal:** one call, no API/data-structure change.
- **Regression risk:** Low for the intended case. Extra notifies are
already part of the protocol; the receiver path in
`tb_xdp_handle_request()` treats `PROPERTIES_CHANGED_REQUEST` as
“re-run state work / re-read properties.”
- **Noted risk (code-derived, not field-confirmed):** because
`XDOMAIN_STATE_ENUMERATED` already re-enters `PROPERTIES` when a
peer notify arrives, a successful re-read will now send another
notify. Two Linux hosts both running this change could keep
exchanging property updates. No follow-up revert/fix appears in this
tree after the commit. Treat as theoretical unless proven in use.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
- Record:
- Surrounding state machine: `8e1de7042596ab` (“thunderbolt: Add
support for XDomain lane bonding”, 2022-05-05), first in
**v5.19-rc1**.
- Notify helper and INIT/UUID notify sites: same commit.
- The added line is this candidate (`e027dba038f000`, Milo Chen,
2026-06-24).
- XDomain discovery protocol itself: `d1ff70241a275` (2017),
**v4.15-rc1**.
**Step 3.2: FOLLOW THE FIXES: TAG**
- Record: No `Fixes:` tag. Not applicable. The missing notify at
ENUMERATED dates to the 5.19 state machine.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
- Record:
- Related: `abc27e5bfed2f` (“Allow service drivers to specify their
own properties”) — extra per-service properties, not required for
this one-line call.
- `308092d080852` (“Restart XDomain discovery handshake after
failure”) — nearby error-path change; not a prerequisite.
- Standalone. Not `patch X/Y`.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
- Record: Milo Chen has only this thunderbolt commit here. Applied by
**Mika Westerberg**, Thunderbolt maintainer.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
- Record: Depends only on `tb_xdomain_queue_properties_changed()` and
`XDOMAIN_STATE_ENUMERATED`, both present since 5.19. Applies
standalone.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
- Record:
- `b4 dig -c e027dba038f000…` →
https://patch.msgid.link/20260624060909.2610434-1-cmh79479@gmail.com
- **v1:** `[PATCH] thunderbolt: xdomain: notify peers after
enumeration` (From: Rqirus) —
https://patch.msgid.link/20260624011131.2532004-1-cmh79479@gmail.com
- **v2:** same diff; changelog: “Use full name in From and Signed-off-
by.”
- Mika Westerberg on v2: **“Applied to thunderbolt.git/next,
thanks!”**
- No NAK. No request for `Cc: stable` in the saved thread.
- lore.kernel.org HTTP fetch returned 403; discussion was recovered
via `b4 dig -m`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
- Record (`b4 dig -w`): To/Cc Mika Westerberg, Andreas Noever, Michael
Jamet, Yehezkel Bernat, linux-usb, linux-kernel. Subsystem maintainers
were on the original mail. Mika applied it.
**Step 4.3: SEARCH FOR THE BUG REPORT**
- Record: No bugzilla/syzbot/Link. The commit text is the bug report:
ThunderboltIP half-discovered after warm reboot, login timeout.
Severity from the author: networking service fails to complete
discovery.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
- Record: Single-patch series (v1→v2, identical diff). Not part of a
multi-patch set.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
- Record: Web search of lore.kernel.org/stable for this subject returned
no relevant hits. **UNVERIFIED** that nobody asked for it on stable@;
no evidence it was rejected either.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
- Record: `tb_xdomain_state_work()` modified; calls
`tb_xdomain_queue_properties_changed()`; that queues
`tb_xdomain_properties_changed()` →
`tb_xdp_properties_changed_request()`.
**Step 5.2: TRACE CALLERS**
- Record:
- `tb_xdomain_state_work` is delayed work started from
`start_handshake()` / queue helpers; not a syscall.
- `tb_xdomain_queue_properties_changed` call sites: INIT (no UUID
needed), UUID success, and (after this patch) PROPERTIES success.
- `tb_register_property_dir()` is called from
`drivers/net/thunderbolt/main.c` (`tbnet_init`),
`drivers/thunderbolt/stream.c`, `drivers/thunderbolt/dma_test.c`.
**Step 5.3: TRACE CALLEES**
- Record: Notify path allocates/sends an XDomain control packet with
retries (`XDOMAIN_RETRIES`). Failure is logged; it does not unwind
enumeration.
**Step 5.4: FOLLOW THE CALL CHAIN**
- Record: Reachable when two hosts do XDomain discovery over
Thunderbolt/USB4 (cable/peer already present, including after warm
reboot). Userspace does not need a special syscall; loading
`thunderbolt-net` during boot vs in-progress discovery is the trigger.
Config: `CONFIG_USB4` plus Thunderbolt networking.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
- Record: Same notify is already used at INIT/UUID and when property
dirs are registered. This fills the gap at end-of-discovery. No
sibling drivers needed the same hunk.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
- Record: Yes. `drivers/thunderbolt/xdomain.c` has the PROPERTIES →
ENUMERATED transition. Parent of the candidate only sets `xd->state =
XDOMAIN_STATE_ENUMERATED` with no notify. ThunderboltIP registers
`"network"` via `tb_register_property_dir()` in this tree. Bug class
has been present since the 5.19 state machine; this tree is 7.3-rc4,
so the code is in-scope.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
- Record: Expected **clean apply**. Parent context around the hunk
matches this tree’s pre-patch source. One added line, no helpers
missing.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
- Record: No other commit on this file addresses “notify after
enumeration” or the ThunderboltIP warm-reboot miss. `abc27e5bfed2f` is
a separate feature (per-service properties), not a substitute.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
- Record: `drivers/thunderbolt/` (USB4/Thunderbolt). **IMPORTANT** for
USB4/TBT hosts, **PERIPHERAL** relative to mm/VFS. Affects Thunderbolt
XDomain and ThunderboltIP (`drivers/net/thunderbolt/`), not all
machines.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
- Record: Actively maintained (Mika Westerberg). Recent xdomain work
includes UAF and bounds fixes. Mature protocol with ongoing fixes;
this is not staging.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
- Record: Thunderbolt/USB4 hosts using XDomain, especially ThunderboltIP
between two machines after warm reboot. Driver-specific / config-
specific (`CONFIG_USB4`, thunderbolt-net).
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
- Record: Warm reboot (or service driver registering properties during
discovery). Timing-dependent. Unprivileged users do not trigger it via
a random syscall; they hit it by using Thunderbolt networking. Common
for that hardware/use case, rare globally.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
- Record: ThunderboltIP discovery incomplete; login times out; network
over Thunderbolt does not come up. Not a panic, UAF, or corruption.
**Severity: HIGH for ThunderboltIP users, MEDIUM overall** (feature
broken, host stays up).
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
- Record:
- **Benefit:** Restores Thunderbolt networking after a realistic warm-
reboot race. High for those users.
- **Risk:** Very low line count; extra notify is an existing protocol
message. Theoretical Linux↔Linux notify loop is unverified in the
field; no revert in this tree.
- **Ratio:** Benefit outweighs risk.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR backporting:**
- Real user-visible bug: ThunderboltIP half-discovered, login timeout
after warm reboot.
- One-line, reuses an existing helper.
- Maintainer (Mika Westerberg) applied it with no technical objections.
- All needed symbols/state exist in this 7.3-rc4 tree (state machine
since 5.19).
- Applies cleanly; not a new API/feature.
- Standalone; v2 only fixed the author’s From name.
**AGAINST backporting:**
- Not a crash, UAF, or security bug.
- Affects a subset (USB4/TBT XDomain / ThunderboltIP).
- No `Reported-by` besides the author’s scenario.
- Theoretical extra notify traffic between two patched Linux peers
(unverified in production).
**UNRESOLVED:**
- Could not fetch lore over HTTP (403); used `b4` mbox instead.
- Mika’s v1 mail body was not in the saved v2 mbox; v2 changelog says
the only change was the From/SOB name.
- No stable@ thread found.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — one existing helper call;
maintainer applied to thunderbolt.git/next.
2. Fixes a real bug that affects users? **PASS** — ThunderboltIP login
timeout after warm reboot.
3. Important issue? **PASS** — service remains unusable (connectivity
failure), not a panic; still a serious functional break for that
hardware.
4. Small and contained? **PASS** — +1 line, one file.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — helpers and state machine are
present; hunk context matches.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
- Record: None (not a device ID, quirk table, DT, build, or docs-only
change). Qualifies as a normal small bugfix.
**Step 9.4: DECISION**
This tree has the buggy handshake. The patch is a minimal, maintainer-
acked fix for a real ThunderboltIP discovery failure. That is
appropriate to backport here.
---
### What the commit solves for this tree
On 7.3-rc4, XDomain discovery still notifies peers early and service
drivers (notably `thunderbolt-net`) can still register the `"network"`
property directory while handshake is in progress. Without a notify at
`ENUMERATED`, a peer that already read the old block can miss the
service; ThunderboltIP then sits half-discovered and login times out
after warm reboot. One extra `tb_xdomain_queue_properties_changed()` at
the end of discovery closes that window.
### Stable rules
The change is obviously correct, tiny, and does not add APIs. It fixes a
real connectivity bug, not a refactor.
### Risk vs benefit
Benefit is restoring Thunderbolt networking after reboot. Risk is one
extra protocol notify on a path that already sends that message.
Residual ping-pong risk between two Linux peers is unverified and did
not produce a follow-up fix in this tree.
### Concerns
No `Fixes:` tag (expected). Not a crash-class bug. Depends on
Thunderbolt XDomain, which this tree has. No missing prerequisites.
---
Verification:
- [Phase 1] Parsed commit `e027dba038f0008df9bc9575f5c3e803e90636c6`:
subject/body/SOBs only; no Fixes/Reported-by/Cc stable
- [Phase 2] Diff: +1 line in `tb_xdomain_state_work()` after `xd->state
= XDOMAIN_STATE_ENUMERATED`
- [Phase 2] Confirmed existing notifies at INIT (line 1739) and UUID
success (line 1751); `tb_register_property_dir()` calls
`update_all_xdomains()`
- [Phase 2] Confirmed `thunderbolt-net` `tbnet_init()` calls
`tb_register_property_dir("network", tbnet_dir)`
- [Phase 2] Receiver of `PROPERTIES_CHANGED_REQUEST` queues
`state_work`; `ENUMERATED` re-enters `PROPERTIES` (verified in
`xdomain.c`)
- [Phase 3] `git blame`: state machine from `8e1de7042596ab`
(v5.19-rc1); XDomain protocol from `d1ff70241a275` (v4.15-rc1)
- [Phase 3] `git log --author="Milo Chen" -- drivers/thunderbolt/`: only
this commit
- [Phase 3] No later xdomain.c commit addresses this notify
- [Phase 4] `b4 dig -c e027dba038f000…`: lore match
https://patch.msgid.link/20260624060909.2610434-1-cmh79479@gmail.com
- [Phase 4] `b4 dig -a`: v1 and v2; v2 identical diff, From-name fix
only
- [Phase 4] `b4 dig -w`: thunderbolt maintainers + linux-usb
- [Phase 4] mbox: Mika Westerberg “Applied to thunderbolt.git/next,
thanks!”
- [Phase 4] WebFetch lore: 403; used b4 instead
- [Phase 4] Stable-list search: no relevant hits
- [Phase 5] Callers of `tb_register_property_dir`: thunderbolt-net,
stream, dma_test (grep)
- [Phase 5] semcode MCP: not available; used git grep/read
- [Phase 6] `make kernelversion` / Makefile: 7.3.0-rc4; `git describe
HEAD`: v7.3-rc4-27-gf0100363d8c37
- [Phase 6] Parent hunk lacks the notify; this tree has the helpers
- [Phase 6] `git merge-base --is-ancestor e027dba038f000 HEAD`: ancestor
(original commit in this development tree; not used as “already
selected for stable”)
- [Phase 7] MAINTAINERS: THUNDERBOLT DRIVER, Mika Westerberg
- [Phase 8] Failure mode: ThunderboltIP login timeout / half-discovered
service, not oops
- UNVERIFIED: Mika v1 review body (not in v2 mbox); whether Linux↔Linux
notify looping occurs in the field; whether stable@ ever discussed
this patch
**YES**
drivers/thunderbolt/xdomain.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index d9c2686b5fe41..b35e2f7c318dc 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -1692,6 +1692,7 @@ static void tb_xdomain_state_work(struct work_struct *work)
tb_xdomain_failed(xd);
} else {
xd->state = XDOMAIN_STATE_ENUMERATED;
+ tb_xdomain_queue_properties_changed(xd);
}
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH AUTOSEL 6.18-6.1] thunderbolt: xdomain: Notify peers after enumeration
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.1] thunderbolt: xdomain: Notify peers after enumeration Sasha Levin
@ 2026-10-19 14:17 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-19 14:17 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-usb
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] thunderbolt: xdomain: infinite ping-pong loop in tb_xdomain_state_work()
--
commit e5f041d5682183591ea61bde27f0a9936f3beac3
Author: Milo Chen <cmh79479@gmail.com>
thunderbolt: xdomain: Notify peers after enumeration
This commit queues a properties-changed notification when the XDomain state
machine reaches XDOMAIN_STATE_ENUMERATED so that the peer re-reads any local
properties registered while discovery was in progress.
> diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
> index 9d220ba544ecc..fca78540c9de6 100644
> --- a/drivers/thunderbolt/xdomain.c
> +++ b/drivers/thunderbolt/xdomain.c
> @@ -1661,6 +1661,7 @@ static void tb_xdomain_state_work(struct work_struct *work)
> tb_xdomain_failed(xd);
> } else {
> xd->state = XDOMAIN_STATE_ENUMERATED;
> + tb_xdomain_queue_properties_changed(xd);
> }
> break;
>
[Severity: Medium]
Won't unconditionally queuing a properties-changed notification on every
transition from XDOMAIN_STATE_PROPERTIES to XDOMAIN_STATE_ENUMERATED cause an
infinite protocol ping-pong loop between two connected peers running this
code?
When the peer is already enumerated (so device_is_registered(&xd->dev) is
true) and receives PROPERTIES_CHANGED_REQUEST, tb_xdp_handle_request()
schedules xd->state_work:
drivers/thunderbolt/xdomain.c:tb_xdp_handle_request() {
...
case PROPERTIES_CHANGED_REQUEST:
tb_dbg(tb, "%llx: received XDomain properties changed request\n",
route);
ret = tb_xdp_properties_changed_response(ctl, route, sequence);
/*
* Since the properties have been changed, let's update
* the xdomain related to this connection as well in
* case there is a change in services it offers.
*/
if (xd && device_is_registered(&xd->dev))
queue_delayed_work(tb->wq, &xd->state_work,
msecs_to_jiffies(XDOMAIN_SHORT_TIMEOUT));
break;
...
}
When tb_xdomain_state_work() runs in XDOMAIN_STATE_ENUMERATED, it calls
tb_xdomain_queue_properties(xd), transitioning xd->state back to
XDOMAIN_STATE_PROPERTIES and re-queuing xd->state_work:
drivers/thunderbolt/xdomain.c:tb_xdomain_state_work() {
...
case XDOMAIN_STATE_PROPERTIES:
ret = tb_xdomain_get_properties(xd);
if (ret) {
if (ret == -EAGAIN)
goto retry_state;
tb_xdomain_failed(xd);
} else {
xd->state = XDOMAIN_STATE_ENUMERATED;
tb_xdomain_queue_properties_changed(xd);
}
break;
case XDOMAIN_STATE_ENUMERATED:
tb_xdomain_queue_properties(xd);
break;
...
}
Then in XDOMAIN_STATE_PROPERTIES, tb_xdomain_get_properties() returns 0 even
when the remote property block generation has not changed:
drivers/thunderbolt/xdomain.c:tb_xdomain_get_properties() {
...
/* Only accept newer generation properties */
if (xd->remote_properties && gen <= xd->remote_property_block_gen) {
ret = 0;
goto err_free_block;
}
...
}
Because ret is 0, XDOMAIN_STATE_PROPERTIES transitions back to
XDOMAIN_STATE_ENUMERATED and calls tb_xdomain_queue_properties_changed(xd)
again, sending a new PROPERTIES_CHANGED_REQUEST back to the first peer and
repeating the cycle indefinitely.
Could this notification be sent only on the initial enumeration (or when local
properties actually changed during discovery), rather than on every
re-enumeration pass triggered by PROPERTIES_CHANGED_REQUEST?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-81-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] usb: fsl_qe_udc: check qe_alloc_request() failure in ch9getstatus()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.1] thunderbolt: xdomain: Notify peers after enumeration Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 14:20 ` sashiko-bot
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] net: usb: qmi_wwan: add Rolling Wireless RN947R Sasha Levin
` (9 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Haoxiang Li, Greg Kroah-Hartman, Sasha Levin, linux-usb,
linuxppc-dev, linux-kernel
From: Haoxiang Li <haoxiang_li2024@163.com>
[ Upstream commit 612551bf7cf7113b8bbd5e8c1eea86d9427c2f95 ]
qe_alloc_request() may return NULL on allocation failure. ch9getstatus()
passes the return value directly to container_of() and then immediately
dereferences the resulting qe_req pointer. Check the allocation result
before using it and stall the control request on failure.
Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>
Link: https://patch.msgid.link/20260620120631.2894977-1-haoxiang_li2024@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
Record: `[usb / fsl_qe_udc] [check] check qe_alloc_request() failure in
ch9getstatus()` — USB gadget UDC driver; “check” here is a missing-NULL-
check fix, not a feature.
**Step 1.2: Tags**
Record:
- `Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com>` (author)
- `Link: https://patch.msgid.link/20260620120631.2894977-1-
haoxiang_li2024@163.com`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (USB
maintainer applied it)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence expected)
**Step 1.3: Body**
Record:
- **Bug:** `qe_alloc_request()` can return NULL; that pointer is passed
to `container_of()` and immediately dereferenced.
- **Symptom:** kernel oops / NULL deref on `GET_STATUS` when allocation
fails.
- **Version info:** none in the message.
- **Root cause:** unchecked allocation result in `ch9getstatus()`.
- **Fix:** test the pointer; stall EP0 on failure.
**Step 1.4: Hidden bug fix?**
Record: Not hidden. This is an explicit NULL-deref fix. The “stall on
failure” path is the existing error path, not a new feature.
## Phase 2: Diff analysis
**Step 2.1: Inventory**
Record:
- File: `drivers/usb/gadget/udc/fsl_qe_udc.c` (+6 / −2)
- Function: `ch9getstatus()`
- Scope: single-file surgical fix
**Step 2.2: Code flow**
Record: Before, `container_of(qe_alloc_request(...), struct qe_req,
req)` ran unconditionally, then `req->req.length = 2` and later
`__qe_ep_queue()`. After, the result is stored in `usb_req`, NULL goes
to the existing `stall:` label (`dev_err` + `qe_ep0_stall()`), and only
a live request is converted with `container_of`.
**Step 2.3: Bug mechanism**
Record: **NULL pointer dereference** (memory safety).
`qe_alloc_request()` does `kzalloc_obj()` and returns NULL on failure.
`struct qe_req` has `struct usb_request req` as the first member, so
`container_of(NULL, ...)` is still NULL, then `req->req.length = 2`
oopses.
**Step 2.4: Fix quality**
Record: Obviously correct; uses the existing `stall` path; no API/data-
structure change. Regression risk is very low. Pre-existing issue not
introduced here: if alloc succeeds and `__qe_ep_queue()` fails, the
request is still not freed (leak on that older path). This patch does
not make that worse.
## Phase 3: Git history
**Step 3.1: Blame**
Record: Unchecked `container_of(qe_alloc_request(...))` comes from
`3948f0e0c999a6` (“usb: add Freescale QE/CPM USB peripheral controller
driver”, 2008-09-02), first in `v2.6.28-rc1`. The bug is ancient, not
new 7.3 code.
**Step 3.2: Fixes: tag**
Record: No `Fixes:` tag. The introducing commit is `3948f0e0c999a6`
(above). That commit is an ancestor of this tree.
**Step 3.3: File history**
Record: Related work on the same function: `ce9daa2efc087` (2023) added
`pipe >= USB_MAX_ENDPOINTS` before indexing `udc->eps[]` — that patch
was AUTOSEL’d into stable 5.15. Recent file churn is
kmalloc→`kzalloc_obj` conversion, not a refactor of `ch9getstatus()`.
Standalone, not `patch X/Y`.
**Step 3.4: Author**
Record: Haoxiang Li has several USB NULL/leak-style fixes (`mtu3`,
`cdnsp`, `typec`, `renesas_usbhs`). Not the UDC maintainer. Greg Kroah-
Hartman applied this one. Similar sibling: `4ab2b48c98f2e`
(`fsl_udc_core` allocation check).
**Step 3.5: Dependencies**
Record: None. Uses existing `qe_alloc_request()`, `stall:` /
`qe_ep0_stall()`, and `container_of`. Applies standalone.
`qe_alloc_request()` still returns NULL on failure after the
`kzalloc_obj` conversion.
## Phase 4: Mailing list / external
**Step 4.1: Original discussion**
Record: `b4 dig -c 612551bf7cf71` → https://patch.msgid.link/20260620120
631.2894977-1-haoxiang_li2024@163.com
`b4 dig -a`: **v1 only**, 2026-06-20, same subject. Mbox is the patch
with **no replies**. LKML/Openwall mirrors match; no NAK, no v2, no `Cc:
stable` in review (there was no public review thread).
**Step 4.2: Recipients (`b4 dig -w`)**
Record: To: `kees@kernel.org`, `gregkh@linuxfoundation.org`; Cc: `linux-
usb`, `linuxppc-dev`, `linux-kernel`. USB maintainer was in To and
committed it.
**Step 4.3: Bug report**
Record: No `Reported-by` / syzbot / bugzilla. Looks like static analysis
(NULL after alloc), not a user crash report.
**Step 4.4: Series**
Record: Standalone v1. Not part of a multi-patch series.
**Step 4.5: Stable list**
Record: Direct lore.kernel.org fetches returned 403. No stable-list
discussion found via `b4` or web search. Precedent: the earlier
`ch9getstatus` bounds check in this file **was** AUTOSEL’d to 5.15.
## Phase 5: Semantic / call-graph
**Step 5.1: Functions**
Record: `ch9getstatus()` only (uses `qe_alloc_request`, `container_of`,
`__qe_ep_queue`, `qe_ep0_stall`).
**Step 5.2: Callers**
Record: Only `setup_received_handle()` on `USB_REQ_GET_STATUS`.
**Step 5.3: Callees**
Record: `qe_alloc_request` → `kzalloc_obj`; `__qe_ep_queue`;
`qe_ep0_stall`; `ownercomplete` kfree’s the request on completion.
**Step 5.4: Call chain / reachability**
Record:
`qe_udc_irq` (hardirq, `spin_lock_irqsave(&udc->lock)`)
→ `rx_irq` → `qe_ep0_rx` → `ep0_setup_handle` → `setup_received_handle`
→ `ch9getstatus`
Reachable from USB host `GET_STATUS` during gadget enumeration.
Unprivileged **host** can send it; not a typical local-syscall path.
GFP_KERNEL under a spinlock in IRQ is a pre-existing atomic-sleep smell
and makes NULL more plausible under memory pressure.
**Step 5.5: Similar patterns**
Record: `qe_alloc_request()` is only used here plus the
`usb_ep_ops.alloc_request` hook. Sibling FSL UDC had the same class of
alloc-check fix.
## Phase 6: This checkout
**Step 6.1: Does the buggy code exist?**
Record: Local tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`).
Driver and `ch9getstatus()` are present. The unchecked alloc was
introduced in v2.6.28 and lived until `612551bf7cf71` (`v7.2-rc1-40`,
2026-07-10). HEAD already has the fixed form (`usb_req =
qe_alloc_request...; if (!usb_req) goto stall`). That is the mainline
source of the candidate, not a reason to treat the bug as absent from
this driver.
**Step 6.2: Backport difficulty**
Record: Surrounding `ch9getstatus()` matches the patch; `git diff
612551bf7cf71 HEAD -- drivers/usb/gadget/udc/fsl_qe_udc.c` is empty.
Clean apply on this tree’s file; older stables still have the same
`container_of(qe_alloc_request(...))` shape from 2008.
**Step 6.3: Related fixes already here**
Record: Endpoint-index check `ce9daa2efc087` is already in this file. No
other fix covers the alloc NULL. This patch is the alloc-failure check,
not a duplicate of that bounds check.
## Phase 7: Subsystem context
**Step 7.1:** Record: USB gadget UDC (`drivers/usb/gadget/udc/`).
**PERIPHERAL** — Freescale QE/CPM USB device controller.
`CONFIG_USB_FSL_QE` depends on `FSL_SOC && (QUICC_ENGINE || CPM)` and
`!64BIT || BROKEN` (32-bit PowerPC QE/CPM, e.g. MPC8360/MPC8272).
**Step 7.2:** Record: Mature, low-churn driver (2008). Recent commits
are conversions/typos plus this NULL check. Long-lived bug, not a new-
subsystem issue.
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
Record: Driver-specific / config-specific: USB gadget on 32-bit
Freescale QE/CPM SoCs.
**Step 8.2: Trigger**
Record: Host sends standard `GET_STATUS`; `qe_alloc_request(GFP_KERNEL)`
fails. Uncommon (memory pressure / atomic GFP_KERNEL), but the control
request is a normal enumeration path. Host-triggered.
**Step 8.3: Failure mode**
Record: NULL deref → oops in hardirq UDC path. **Severity: HIGH** (crash
of the gadget/UDC path; can take the machine down depending on
panic_on_oops). Not silent corruption.
**Step 8.4: Risk vs benefit**
Record: **Benefit:** medium for those boards (prevents oops), low
globally (niche HW). **Risk:** very low (6 net lines, existing stall
path). Ratio favors taking it: same class of fix already AUTOSEL’d in
this function.
## Phase 9: Synthesis
**Step 9.1: Evidence**
For:
- Real NULL deref on a live USB control path
- `qe_alloc_request()` documented to return NULL
- Tiny, obviously correct check + existing stall
- Bug present since 2008 in this driver
- USB maintainer merged it
- Same function already had a stable AUTOSEL bounds fix
- Call path is IRQ + spinlock, so GFP_KERNEL failure is not purely
theoretical
Against:
- Niche 32-bit PowerPC QE/CPM gadget (`BROKEN` on 64-bit)
- No user/syzbot report; static-analysis style v1 with no mailing-list
review
- No `Tested-by`
- Alloc-failure is uncommon
- Pre-existing queue-failure leak left unfixed (not a regression)
Unresolved: lore.kernel.org HTTP 403; no public review comments beyond
the patch itself (confirmed via `b4` mbox).
**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** on correctness (code is
unambiguous). Testing is only “merged by Greg”; no hardware Tested-
by, but the change does not need runtime proof beyond reading
`qe_alloc_request()`.
2. Real bug affecting users? **PASS** — oops if this UDC handles
`GET_STATUS` and alloc fails.
3. Important (crash/security/corruption/deadlock)? **PASS** — kernel
oops. Not a new API.
4. Small and contained? **PASS** — 8-line hunk, one function.
5. No new features/APIs? **PASS**
6. Can apply to this tree? **PASS** — file and function exist; context
matches.
**Step 9.3: Exception category**
Record: None (not an ID/quirk/DT/build/doc exception). It is a normal
crash fix.
**Step 9.4: Decision**
This is a textbook missing-NULL-check on a kmalloc-backed helper, in a
path a USB host hits with `GET_STATUS`. It is small, applies to the
driver in this tree, and prevents an oops. Niche hardware lowers
population, not severity for the people who have it. It meets stable-
kernel-rules.
### What it solves for stable users
Without the check, `ch9getstatus()` oopses when `qe_alloc_request()`
returns NULL. With it, EP0 stalls and the gadget stays up. That matters
on QE/CPM gadget devices under memory pressure, including in hardirq.
### Risk vs benefit
Very low risk, contained error-path change. Benefit is avoiding a real
oops on existing hardware. Worth taking.
---
Verification:
- [Phase 1] Parsed tags: author SOB, Greg KH SOB, msgid link; no
Fixes/Reported-by/Cc stable
- [Phase 2] Diff: +6/−2 in `ch9getstatus()`; NULL check then existing
`stall:`
- [Phase 2] `struct qe_req`: `req` is first member
(`fsl_qe_udc.h:252-259`); `container_of(NULL)` is NULL
- [Phase 2] `qe_alloc_request()` returns NULL after `kzalloc_obj`
failure (lines 1669–1681)
- [Phase 3] `git blame` / `git log -1 3948f0e0c999a6`: buggy pattern
from 2008, `v2.6.28-rc1`
- [Phase 3] No Fixes tag; introducing commit is in this tree
- [Phase 3] Related: `ce9daa2efc087` bounds check in same function;
AUTOSEL 5.15 (lkml archive)
- [Phase 3] Author USB fixes listed via `git log --author="Haoxiang Li"`
- [Phase 4] `b4 dig -c 612551bf7cf71`: lore msgid
20260620120631.2894977-1
- [Phase 4] `b4 dig -a`: v1 only; `b4 dig -w`: gregkh + kees + linux-usb
+ linuxppc-dev
- [Phase 4] mbox: patch only, no replies
- [Phase 4] lore WebFetch 403; LKML/Openwall mirrors used instead
- [Phase 5] Callers: only `setup_received_handle` for
`USB_REQ_GET_STATUS`
- [Phase 5] Call chain: `qe_udc_irq` → `rx_irq` → `qe_ep0_rx` →
`ep0_setup_handle` → `setup_received_handle` → `ch9getstatus`
(spinlock held)
- [Phase 6] `git describe HEAD`: `v7.3-rc4-27-gf0100363d8c37`; `make
kernelversion`: 7.3.0-rc4
- [Phase 6] `git merge-base --is-ancestor 612551bf7cf71 HEAD`: yes
(mainline already contains the candidate)
- [Phase 6] File at HEAD matches the fixed version; reverse-apply of the
commit succeeds
- [Phase 7] Kconfig `USB_FSL_QE`: FSL_SOC QE/CPM, `!64BIT || BROKEN`
- [Phase 8] Failure mode: NULL deref oops in UDC IRQ, severity HIGH
- UNVERIFIED: full lore thread beyond the mbox (HTTP 403); whether any
reviewer privately asked for Cc: stable
**YES**
drivers/usb/gadget/udc/fsl_qe_udc.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
index aacfde06387c0..7911dca2135cf 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -1945,6 +1945,7 @@ static void ch9getstatus(struct qe_udc *udc, u8 request_type, u16 value,
u16 index, u16 length)
{
u16 usb_status = 0;
+ struct usb_request *usb_req;
struct qe_req *req;
struct qe_ep *ep;
int status = 0;
@@ -1983,8 +1984,11 @@ static void ch9getstatus(struct qe_udc *udc, u8 request_type, u16 value,
}
}
- req = container_of(qe_alloc_request(&ep->ep, GFP_KERNEL),
- struct qe_req, req);
+ usb_req = qe_alloc_request(&ep->ep, GFP_KERNEL);
+ if (!usb_req)
+ goto stall;
+
+ req = container_of(usb_req, struct qe_req, req);
req->req.length = 2;
req->req.buf = udc->statusbuf;
*(u16 *)req->req.buf = cpu_to_le16(usb_status);
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.10] usb: fsl_qe_udc: check qe_alloc_request() failure in ch9getstatus()
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] usb: fsl_qe_udc: check qe_alloc_request() failure in ch9getstatus() Sasha Levin
@ 2026-10-19 14:20 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-19 14:20 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-usb
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: fsl_qe_udc: sleeping allocation in atomic context in ch9getstatus()
- [Medium] usb: fsl_qe_udc: memory leak in ch9getstatus() when __qe_ep_queue() fails
--
commit 130ef9b6cbb58e7a139f83239ccf232ff79a3793
Author: Haoxiang Li <haoxiang_li2024@163.com>
usb: fsl_qe_udc: check qe_alloc_request() failure in ch9getstatus()
This commit adds a NULL check for the return value of qe_alloc_request()
in ch9getstatus() before passing it to container_of() and dereferencing
it, stalling the endpoint on allocation failure.
Link: https://patch.msgid.link/20260620120631.2894977-1-haoxiang_li2024@163.com
> diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
> index aacfde06387c0..7911dca2135cf 100644
> --- a/drivers/usb/gadget/udc/fsl_qe_udc.c
> +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
> @@ -1945,6 +1945,7 @@ static void ch9getstatus(struct qe_udc *udc, u8 request_type, u16 value,
> u16 index, u16 length)
> {
> u16 usb_status = 0;
> + struct usb_request *usb_req;
> struct qe_req *req;
> struct qe_ep *ep;
> int status = 0;
> @@ -1983,8 +1984,11 @@ static void ch9getstatus(struct qe_udc *udc, u8 request_type, u16 value,
> }
> }
>
> - req = container_of(qe_alloc_request(&ep->ep, GFP_KERNEL),
> - struct qe_req, req);
> + usb_req = qe_alloc_request(&ep->ep, GFP_KERNEL);
[Severity: High]
Should this allocation in ch9getstatus() use GFP_ATOMIC instead of
GFP_KERNEL?
When a USB_REQ_GET_STATUS control request is received, ch9getstatus() is
called from the hardware interrupt handler while holding udc->lock with
interrupts disabled:
qe_udc_irq()
spin_lock_irqsave(&udc->lock, flags)
rx_irq()
qe_ep0_rx()
ep0_setup_handle()
setup_received_handle()
ch9getstatus()
qe_alloc_request(&ep->ep, GFP_KERNEL)
kzalloc(sizeof(*req), gfp_flags)
Passing GFP_KERNEL here allows kzalloc() to sleep in atomic context.
> + if (!usb_req)
> + goto stall;
> +
> + req = container_of(usb_req, struct qe_req, req);
> req->req.length = 2;
> req->req.buf = udc->statusbuf;
> *(u16 *)req->req.buf = cpu_to_le16(usb_status);
[Severity: Medium]
Can this path in ch9getstatus() also leak req if __qe_ep_queue() fails?
Just below this hunk, ch9getstatus() queues the allocated request:
drivers/usb/gadget/udc/fsl_qe_udc.c:ch9getstatus() {
...
status = __qe_ep_queue(&ep->ep, &req->req);
if (status == 0)
return;
stall:
dev_err(udc->dev, "Can't respond to getstatus request \n");
qe_ep0_stall(udc);
}
In __qe_ep_queue(), if !udc->driver or udc->gadget.speed == USB_SPEED_UNKNOWN,
it returns -ESHUTDOWN before adding req->queue to ep->queue, so
ownercomplete() will never be called to free req:
drivers/usb/gadget/udc/fsl_qe_udc.c:__qe_ep_queue() {
...
if (!udc->driver || udc->gadget.speed == USB_SPEED_UNKNOWN)
return -ESHUTDOWN;
...
}
When __qe_ep_queue() returns a non-zero status, ch9getstatus() falls
through to stall: without calling qe_free_request(&ep->ep, usb_req) or
kfree(req).
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-95-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] net: usb: qmi_wwan: add Rolling Wireless RN947R
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] usb: fsl_qe_udc: check qe_alloc_request() failure in ch9getstatus() Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] usb: mtu3: Fix double dereference in TP_printk Sasha Levin
` (8 subsequent siblings)
13 siblings, 0 replies; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Reinhard Speyerer, Jakub Kicinski, Sasha Levin, bjorn,
andrew+netdev, davem, edumazet, pabeni, netdev, linux-usb,
linux-kernel
From: Reinhard Speyerer <rspmn@arcor.de>
[ Upstream commit 7375d38364a9aa66fb31716bcefef38aecad75d8 ]
Add Rolling Wireless RN947R 0x9300 composition:
DIAG + ADB + NMEA + MODEM + RMNET + QDSS + ADPL
T: Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#= 6 Spd=480 MxCh= 0
D: Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs= 1
P: Vendor=33f8 ProdID=9300 Rev= 0.00
S: Manufacturer=Rolling Wireless
S: Product=RN947R
S: SerialNumber=xxxxxxxxxxx
C:* #Ifs= 7 Cfg#= 1 Atr=a0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=82(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
E: Ad=84(I) Atr=03(Int.) MxPS= 10 Ivl=32ms
E: Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
E: Ad=86(I) Atr=03(Int.) MxPS= 10 Ivl=32ms
E: Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#= 8 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
E: Ad=88(I) Atr=03(Int.) MxPS= 8 Ivl=32ms
E: Ad=87(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#=21 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=70 Driver=(none)
E: Ad=89(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
I:* If#=22 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=80 Driver=(none)
E: Ad=8a(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
Signed-off-by: Reinhard Speyerer <rspmn@arcor.de>
Link: https://patch.msgid.link/arZzuOOvLUZtdOlv@arcor.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This is a one-line USB ID addition to the existing `qmi_wwan` driver for
Rolling Wireless RN947R (`33f8:9300`). That is a standard stable
exception, and the ID table plus DTR quirk already exist in this 6.18.y
tree.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
Step 1.1: PARSE THE SUBJECT LINE
Record: [net: usb: qmi_wwan] [add] [USB device ID for Rolling Wireless
RN947R]
Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
Record:
- Link: https://patch.msgid.link/arZzuOOvLUZtdOlv@arcor.de
- Signed-off-by: Reinhard Speyerer <rspmn@arcor.de> (author)
- Signed-off-by: Jakub Kicinski <kuba@kernel.org> (netdev maintainer,
applied)
- No Fixes: tag (expected)
- No Reported-by / Tested-by / Reviewed-by / Acked-by
- No Cc: stable (expected)
- Ignored pipeline SOB / Upstream commit marker
Step 1.3: ANALYZE THE COMMIT BODY TEXT
Record:
- Bug description: RN947R USB WWAN modem (`Vendor=33f8 ProdID=9300`) is
not in `qmi_wwan`’s ID table, so the RMNET/QMI network function does
not bind.
- Symptom: no `qmi_wwan` netdev for this modem; cellular data does not
come up out of the box.
- Version information: none stated.
- Root cause: missing USB ID / interface mapping. `lsusb -t`-style dump
shows If#=8 Cls=ff Sub=ff Prot=50 as the QMI/RMNET function.
Composition is DIAG + ADB + NMEA + MODEM + RMNET + QDSS + ADPL.
Step 1.4: DETECT HIDDEN BUG FIXES
Record: Not a hidden crash/leak fix. It is an explicit device-ID
enablement, which stable rules treat as a first-class exception.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
Step 2.1: INVENTORY THE CHANGES
Record:
- File: `drivers/net/usb/qmi_wwan.c` (+1 / -0)
- Function/table: `products[]` (`static const struct usb_device_id`)
- Scope: single-file, one-line surgical ID addition
Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
Record:
- Before: `33f8:9300` is unmatched; USB core does not probe `qmi_wwan`
for interface 8.
- After: `QMI_QUIRK_SET_DTR(0x33f8, 0x9300, 8)` matches
vendor/product/interface 8 and sets `qmi_wwan_info_quirk_dtr`.
- Path: USB enumeration / driver probe only. No error-path or hot-path
logic change.
Step 2.3: IDENTIFY THE BUG MECHANISM
Record:
- Category: hardware enablement / USB device ID (stable exception 1).
- Mechanism: `QMI_QUIRK_SET_DTR` expands to
`USB_DEVICE_INTERFACE_NUMBER` plus DTR quirk `driver_info`. Interface
8 matches the dump (`Prot=50`, QMI/RMNET). DTR quirk is the same
pattern as Rolling RW101 (`33f8:0104`) and other Qualcomm USB WWAN
entries.
Step 2.4: ASSESS THE FIX QUALITY
Record:
- Obviously correct: VID/PID/interface taken from a real device dump;
match is interface-number-specific so DIAG/ADB/NMEA/MODEM/QDSS/ADPL
are not claimed.
- Minimal: one table entry, no API or structure change.
- Regression risk: very low; only `33f8:9300` is affected. Wrong
interface number would be the main risk, and the dump confirms If#=8.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
Step 3.1: BLAME THE CHANGED LINES
Record:
- Insertion sits next to Rolling RW101: `d362046021ea12`
(“net:usb:qmi_wwan: support Rolling modules”, Vanillan Wang,
2024-04-16).
- That RW101 ID is in this tree’s v6.18.57 base (`git merge-base --is-
ancestor d362046021ea1 v6.18.57` succeeded).
- This is extending an existing vendor block, not introducing a new
driver.
Step 3.2: FOLLOW THE FIXES: TAG
Record: No Fixes: tag. Not applicable. This does not revert or repair a
prior logic bug.
Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
Record:
- Recent `qmi_wwan` history is dominated by the same class of ID
additions (Quectel, Telit, MeiG, Fibocom, SIMCom).
- No prerequisite refactor is required; `products[]` and
`QMI_QUIRK_SET_DTR` are long-standing.
- Standalone: subject is not “patch X/Y”. `b4 dig -a` shows a single v1
patch.
Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Record:
- Reinhard Speyerer has prior `qmi_wwan` ID/work in this history (e.g.
Fibocom FG132 `64761c980cbf7`).
- Not the listed maintainer; maintainer is Bjørn Mork (`MAINTAINERS`:
USB QMI WWAN NETWORK DRIVER). The patch was mailed to `bjorn@mork.no`.
Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Record:
- Depends only on `QMI_QUIRK_SET_DTR` / `USB_DEVICE_INTERFACE_NUMBER`,
both present.
- No companion `option.c` change is in this series. Serial AT/NMEA/DIAG
auto-bind is separate; this patch alone still enables the WWAN netdev.
Can apply standalone.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Record:
- `b4 dig -c 7375d38364a9aa66fb31716bcefef38aecad75d8` matched
https://patch.msgid.link/arZzuOOvLUZtdOlv@arcor.de
- `b4 dig -a`: only v1, 2026-09-25, `[PATCH net] net: usb: qmi_wwan: add
Rolling Wireless RN947R`
- Thread (saved via `b4 dig -m`): submission, then patchwork-bot
“applied to netdev/net.git (main) by Jakub Kicinski”. No NAK, no
review objections, no later revision.
- No reviewer Cc: stable in the thread.
Step 4.2: CHECK WHO REVIEWED THE PATCH
Record (`b4 dig -w`): To `bjorn@mork.no`, Cc `netdev@vger.kernel.org`.
Subsystem maintainer was on To. Applied by netdev maintainer. No
explicit Reviewed-by in the committed message.
Step 4.3: SEARCH FOR THE BUG REPORT
Record: No syzbot/bugzilla. Evidence of a real device is the author’s
usb-devices dump (Manufacturer=Rolling Wireless, Product=RN947R,
33f8:9300). User population is owners of that modem.
Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Record: Single-patch series. No `option.c` RN947R commit in this tree
(`git log --grep=RN947R -- drivers/` only finds this qmi_wwan change).
Web search did not find a matching option serial patch.
Step 4.5: CHECK STABLE MAILING LIST HISTORY
Record: UNVERIFIED. `WebFetch` of lore.kernel.org/stable/?q=RN947R
returned 403. No local evidence this ID was rejected for stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF
Record: No function body edited. Data change in `products[]`, consumed
as `qmi_wwan_driver.id_table`.
Step 5.2: TRACE CALLERS
Record:
- `products` -> `qmi_wwan_driver.id_table` -> USB core match ->
`qmi_wwan_probe()` -> `usbnet` bind path (`qmi_wwan_bind`).
- Reachable on device plug-in / boot with the modem attached. Not
interrupt or syscall hot path for other devices.
Step 5.3: TRACE CALLEES
Record: Unchanged. Bind still uses existing `qmi_wwan_bind` / DTR
handling (`qmi_wwan_change_dtr` when `QMI_WWAN_QUIRK_DTR` or `bcdUSB >=
0x0201`). Dump has `Ver= 2.10`, so DTR would be set even with
`QMI_FIXED_INTF`; `QMI_QUIRK_SET_DTR` matches sibling Rolling entries.
Step 5.4: FOLLOW THE CALL CHAIN
Record: Userspace-visible: plugging the modem should create a QMI WWAN
netdev and cdc-wdm control node. Trigger is ordinary USB enumeration,
not an obscure debug path.
Step 5.5: SEARCH FOR SIMILAR PATTERNS
Record: Same `QMI_QUIRK_SET_DTR(vend, prod, intf)` pattern is used
throughout `products[]`, including `33f8:0104` RW101. This is the
established way to add Qualcomm RMNET compositions.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Record:
- Local tree: `git describe HEAD` / `make kernelversion` => **6.18.57**
(`VERSION=6 PATCHLEVEL=18 SUBLEVEL=57`).
- `drivers/net/usb/qmi_wwan.c` exists; `CONFIG_USB_NET_QMI_WWAN` exists;
`QMI_QUIRK_SET_DTR` exists.
- `git show v6.18.57:drivers/net/usb/qmi_wwan.c` has Rolling RW101
(`33f8:0104`) and **does not** have `33f8:9300` / RN947R. The missing-
ID problem exists on this 6.18.y base.
Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Record: Clean apply. v6.18.57 context is identical around the insertion
point:
```
{QMI_QUIRK_SET_DTR(0x33f8, 0x0104, 4)}, /* Rolling RW101 RMNET */
{QMI_FIXED_INTF(0x2dee, 0x4d22, 5)}, /* MeiG Smart SRM825L */
```
No refactor between mainline hunk and this tree.
Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
Record: No other RN947R ID in this tree’s `option.c` or `qmi_wwan.c` on
the v6.18.57 base. No alternate fix for 33f8:9300.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
Record: [drivers/net/usb/qmi_wwan] [IMPORTANT for WWAN users, peripheral
for everyone else]. Mature, non-staging driver used by laptops and
embedded LTE/5G modems.
Step 7.2: ASSESS SUBSYSTEM ACTIVITY
Record: Actively maintained via ID-table additions. Maintainer Bjørn
Mork; netdev applies these routinely. Mature ID-table design.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
Step 8.1: DETERMINE WHO IS AFFECTED
Record: Driver-specific / hardware-specific. Only Rolling Wireless
RN947R (`33f8:9300`). Other USB devices unchanged.
Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Record: Plug in the modem or boot with it attached. Common for those
users. Unprivileged users do not get a new attack surface; this is bind-
table matching.
Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Record: Without the ID, the RMNET function stays unbound; WWAN
networking does not work. Not a panic/UAF. Severity MEDIUM as a
functional hardware-support miss, and it still qualifies under the
device-ID exception.
Step 8.4: CALCULATE RISK-BENEFIT RATIO
Record:
- Benefit: HIGH for RN947R users (modem network interface works); zero
effect on others.
- Risk: VERY LOW (1 line, existing macro, existing driver,
VID:PID:interface match).
- Ratio: strongly favors backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
Step 9.1: COMPILE THE EVIDENCE
FOR:
- Explicit stable exception: USB ID added to an existing driver.
- One line, obviously correct, matches a real device dump (If#=8,
Prot=50).
- Driver, macros, and Rolling vendor support already in 6.18.57.
- Applies cleanly next to RW101.
- Mailed to qmi_wwan maintainer, applied by netdev maintainer, v1 with
no NAK.
- Same pattern as many other qmi_wwan ID additions.
AGAINST:
- Does not fix a crash, UAF, or corruption.
- No Tested-by/Reviewed-by tags.
- `option.c` still lacks `33f8:9300`, so serial ports may need a
separate ID or `new_id`; that does not block the netdev ID.
UNRESOLVED:
- lore.kernel.org WebFetch 403 for stable@ search.
- Whether an option serial companion exists upstream later than this
tree.
Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? PASS — dump-backed ID, applied to
netdev/net.
2. Fixes a real bug that affects users? PASS — modem otherwise has no
QMI netdev.
3. Important issue? PASS under device-ID exception (hardware enablement
for existing driver).
4. Small and contained? PASS — +1 line in one table.
5. No new features or APIs? PASS — no new driver, syscall, or userspace
API; ID only.
6. Can apply to the local tree? PASS — v6.18.57 has identical context.
Step 9.3: CHECK FOR EXCEPTION CATEGORIES
Record: Device ID addition to existing driver (USB ID in `qmi_wwan`).
Step 9.4: MAKE YOUR DECISION
This should be backported to this 6.18.y tree. It is the standard, low-
risk USB ID pattern stable already takes for `qmi_wwan`.
What problem it solves: RN947R owners on 6.18.y get a working QMI/RMNET
interface instead of an unbound USB function.
Risk vs benefit: one matching table entry vs. a non-working WWAN modem.
Benefit dominates.
Concerns: serial `option` binding is incomplete without a separate ID;
that is independent and not a reason to drop this netdev ID.
Verification:
- [Phase 1] Parsed subject/tags/body: device-ID add; Link to
patch.msgid.link; SOB Speyerer + Kicinski; no Fixes/syzbot.
- [Phase 2] Diff: +1 line in `products[]`; `QMI_QUIRK_SET_DTR(0x33f8,
0x9300, 8)` matches dump If#=8.
- [Phase 2] Read `QMI_QUIRK_SET_DTR` / `qmi_wwan_info_quirk_dtr` / DTR
logic at bind (lines 804–825, 938–958).
- [Phase 3] `git blame` around insertion: RW101 from `d362046021ea12`
(2024); ancestor of v6.18.57.
- [Phase 3] `git log --oneline -20 -- drivers/net/usb/qmi_wwan.c`: many
similar ID adds; this is standalone.
- [Phase 3] `git log --author='Reinhard Speyerer' -- drivers/net/usb/`:
prior qmi_wwan ID work.
- [Phase 4] `b4 dig -c 7375d38364a9…`: lore URL found; `-a` v1 only;
`-w` To bjorn@mork.no Cc netdev; `-m` thread is submit + applied-bot,
no NAK.
- [Phase 4] lore WebFetch 403; used b4 mbox instead.
- [Phase 4] WebSearch: no option.c RN947R companion found.
- [Phase 5] `products` -> `qmi_wwan_driver.id_table` ->
`qmi_wwan_probe`; `MODULE_DEVICE_TABLE(usb, products)`.
- [Phase 5] `option.c` has other 33f8 IDs, not 0x9300.
- [Phase 6] `git describe HEAD` / Makefile: 6.18.57. `git show
v6.18.57:drivers/net/usb/qmi_wwan.c` has RW101, lacks RN947R. Apply
context matches.
- [Phase 6] `git merge-base --is-ancestor 7375d38364a9 HEAD` is false
(upstream SHA not this history); evaluate against local 6.18.y code,
not that SHA.
- [Phase 7] MAINTAINERS: USB QMI WWAN NETWORK DRIVER, M: Bjørn Mork.
- [Phase 8] Failure mode: unbound RMNET on this VID:PID only; risk
confined to that device.
- UNVERIFIED: stable@ lore discussion (HTTP 403).
**YES**
drivers/net/usb/qmi_wwan.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index aa66a30e56cdc..85d2daae805fd 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1453,6 +1453,7 @@ static const struct usb_device_id products[] = {
{QMI_QUIRK_SET_DTR(0x1546, 0x1312, 4)}, /* u-blox LARA-R6 01B */
{QMI_QUIRK_SET_DTR(0x1546, 0x1342, 4)}, /* u-blox LARA-L6 */
{QMI_QUIRK_SET_DTR(0x33f8, 0x0104, 4)}, /* Rolling RW101 RMNET */
+ {QMI_QUIRK_SET_DTR(0x33f8, 0x9300, 8)}, /* Rolling RN947R RMNET */
{QMI_FIXED_INTF(0x2dee, 0x4d22, 5)}, /* MeiG Smart SRM825L */
{QMI_QUIRK_SET_DTR(0x2dee, 0x4d63, 0)}, /* MeiG SRM813Q w/ Modem(PPP) */
{QMI_QUIRK_SET_DTR(0x2dee, 0x4d64, 0)}, /* MeiG SRM813Q w/ NMEA */
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.18-5.10] usb: mtu3: Fix double dereference in TP_printk
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] net: usb: qmi_wwan: add Rolling Wireless RN947R Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 14:18 ` sashiko-bot
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] net: usb: rtl8150: handle link status read failures Sasha Levin
` (7 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Vladimir Murzin, Paul E. McKenney, Mark Rutland, Steven Rostedt,
Sasha Levin, chunfeng.yun, gregkh, linux-usb, linux-arm-kernel,
linux-mediatek, linux-kernel
From: Vladimir Murzin <vladimir.murzin@arm.com>
[ Upstream commit cfb3293c4573289f11f913611f530ea6b1926d67 ]
Paul reported kernel splat:
[ 0.000000] TRACE EVENT ERROR: Event mtu3_gadget_ep_set_halt has double dereference in TP_printk: &REC->gpd_ring->dma
[ 0.000000] ------------[ cut here ]------------
[ 0.000000] Event mtu3_gadget_ep_set_halt has double dereference in TP_printk: &REC->gpd_ring->dma
[ 0.000000] WARNING: kernel/trace/trace_events.c:420 at test_double_dereference+0x144/0x14c, CPU#0: swapper/0/0
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 7.3.0-rc1 #15247 PREEMPT
[ 0.000000] Hardware name: linux,dummy-virt (DT)
[ 0.000000] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 0.000000] pc : test_double_dereference+0x144/0x14c
[ 0.000000] lr : test_double_dereference+0x144/0x14c
[ 0.000000] sp : ffffc80aa7633bf0
[ 0.000000] x29: ffffc80aa7633bf0 x28: ffffc80aa7c0f047 x27: 000508b58019388f
[ 0.000000] x26: 0000000000000003 x25: 0000000000000007 x24: ffffc80aa5fceff8
[ 0.000000] x23: ffffc80aa7c0f05a x22: ffffc80aa64edfe8 x21: ffffc80aa7c0fce8
[ 0.000000] x20: ffffc80aa7c0f047 x19: 0000000000000013 x18: 0000000000000001
[ 0.000000] x17: 6572656420656c62 x16: 756f642073616820 x15: 746c61685f746573
[ 0.000000] x14: 0000000000000000 x13: ffff000139d90000 x12: 0000000000000045
[ 0.000000] x11: 00000000000000cf x10: ffff00013f546428 x9 : ffff000139d90000
[ 0.000000] x8 : 3fffffffffffc000 x7 : 0000000000000001 x6 : 0000000000000001
[ 0.000000] x5 : ffff00013f4e6440 x4 : 0000000000000000 x3 : 0000000000000000
[ 0.000000] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffc80aa764a700
[ 0.000000] Call trace:
[ 0.000000] test_double_dereference+0x144/0x14c (P)
[ 0.000000] trace_event_raw_init+0x37c/0x5d8
[ 0.000000] event_init+0x34/0xc0
[ 0.000000] trace_event_init+0xec/0x588
[ 0.000000] trace_init+0x24/0x6e0
[ 0.000000] start_kernel+0x4a0/0x8ec
[ 0.000000] __primary_switched+0x88/0x90
[ 0.000000] irq event stamp: 0
[ 0.000000] hardirqs last enabled at (0): [<0000000000000000>] 0x0
[ 0.000000] hardirqs last disabled at (0): [<0000000000000000>] 0x0
[ 0.000000] softirqs last enabled at (0): [<0000000000000000>] 0x0
[ 0.000000] softirqs last disabled at (0): [<0000000000000000>] 0x0
[ 0.000000] ---[ end trace 0000000000000000 ]---
[ 0.000000] TRACE EVENT ERROR: Event mtu3_gadget_ep_disable has double dereference in TP_printk: &REC->gpd_ring->dma
[ 0.000000] TRACE EVENT ERROR: Event mtu3_gadget_ep_enable has double dereference in TP_printk: &REC->gpd_ring->dma
Which also observed by Mark and myself.
The splat it result of new check introduced by b5cc230af5e5 ("tracing:
Warn when an event dereferences a pointer in TP_printk()") which
correctly catches issue with %pad dereferencing the address saved in
the ring buffer. TP_fast_assign() logic gets executed when the
tracepoint is triggered, however the TP_printk() is executed when the
user reads the trace buffer which could be seconds, minutes, hours,
days, even months later and nothing guarantee that __entry->gpd_ring
pointer will still be pointing to what it was when it was recorded.
Fix the issue by capturing immediate value of gpd_ring.dma when trace
point is triggered.
Reported-by: Paul E. McKenney <paulmck@kernel.org>
Tested-by: Mark Rutland <mark.rutland@arm.com>
Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
Record: **[usb/mtu3]** **[Fix]** Double dereference in `TP_printk` for
the `mtu3_log_ep` trace class
(`mtu3_gadget_ep_enable/disable/set_halt`).
**Step 1.2: Tags**
Record:
- **Reported-by:** Paul E. McKenney
- **Tested-by:** Mark Rutland
- **Reviewed-by:** Steven Rostedt (tracing maintainer)
- **Signed-off-by:** Vladimir Murzin (author); Paul E. McKenney (merged
via his tree)
- **No Fixes:** tag (expected for this review)
- **No Cc: stable** (expected)
- No syzbot, no CVE, no bugzilla Link
- Ignore pipeline “Upstream commit” / Sasha Levin SOB
Notable: three independent observers (Paul, Mark, the author). Rostedt
reviewed it.
**Step 1.3: Body**
Record:
- **Bug:** `TP_printk` uses `%pad` with `&REC->gpd_ring->dma`, i.e. it
follows a pointer stored in the ring buffer when the trace is printed,
not when it is recorded.
- **Symptom that motivated the patch:** boot `WARNING` in
`test_double_dereference()` on 7.3.0-rc1 (`linux,dummy-virt`), events
`mtu3_gadget_ep_set_halt/disable/enable`.
- **Root cause (author):** check added by `b5cc230af5e5` (“tracing: Warn
when an event dereferences a pointer in TP_printk()”).
`TP_fast_assign()` runs at the tracepoint; `TP_printk()` can run much
later, when `__entry->gpd_ring` may be stale.
- **Fix:** snapshot `mep->gpd_ring.dma` into a `dma_addr_t` field at
assign time and print `&__entry->gpd_ring_dma`.
- **Versions in the message:** 7.3.0-rc1 only. Mailing list later says
the splat was still present in 7.3-rc5/rc6.
Linus’s merge of `8fc6e594884a` describes the same bug as pre-existing:
it can dump garbage traces because `TP_printk()` may run after the
pointed-to memory is gone. Extra tracing checks only **exposed** it.
**Step 1.4: Hidden bug fix?**
Record: Not hidden. It is an explicit tracing UAF/stale-pointer fix. The
boot WARN is how it was found; the real defect is delayed dereference of
a saved kernel pointer.
---
## Phase 2: Diff analysis
**Step 2.1: Inventory**
Record:
- File: `drivers/usb/mtu3/mtu3_trace.h` only (+3 / −1; 4-line class)
- Function/class: `DECLARE_EVENT_CLASS(mtu3_log_ep)` (shared by
`mtu3_gadget_ep_enable`, `mtu3_gadget_ep_disable`,
`mtu3_gadget_ep_set_halt`)
- Scope: single-file surgical tracing fix
**Step 2.2: Code flow**
Record: **Before:** `TP_fast_assign` stores `__entry->gpd_ring =
&mep->gpd_ring`. `TP_printk` prints `%p/%pad` with `__entry->gpd_ring,
&__entry->gpd_ring->dma`. **After:** also stores `__entry->gpd_ring_dma
= mep->gpd_ring.dma` and prints `&__entry->gpd_ring_dma`. Affected path
is trace **print**, not gadget I/O.
Verified in this tree’s `lib/vsprintf.c` `address_val()`: `%pad` does
`*(const dma_addr_t *)addr`. The old argument therefore both follows
`gpd_ring` and then dereferences `.dma`.
**Step 2.3: Bug mechanism**
Record: **Memory safety / delayed pointer dereference in tracing (UAF or
garbage).** `%p` of the saved pointer is safe; `%pad` is not. `gpd_ring`
is embedded in `struct mtu3_ep` (`mtu3.h`). `mtu3_gpd_ring_free()`
`memset`s the ring on disable; the `mtu3_ep` itself is freed on unbind.
Reading traces after unbind is a use-after-free. Reading after disable
but before unbind can print a later (zeroed) DMA address instead of the
recorded one.
**Step 2.4: Fix quality**
Record: Obviously correct and minimal. `%pad` still gets a `dma_addr_t
*`, now into the ring-buffer entry. The missing semicolon on the new
assign is harmless: `TP_fast_assign` is expanded with a trailing `;`. No
API/locking/hot-path change. Regression risk is very low. The leftover
`__field(struct mtu3_gpd_ring *, gpd_ring)` is still only printed with
`%p`.
---
## Phase 3: Git history
**Step 3.1: Blame**
Record: Buggy `mtu3_log_ep` pattern comes from `83374e035b628` (“usb:
mtu3: add tracepoints to help debug”, Chunfeng Yun, 2019-03-21). Present
in this tree (`git merge-base --is-ancestor 83374e035b628 HEAD`
succeeded). Long-lived, not recent code.
**Step 3.2: Fixes: tag**
Record: None on the merged commit. An earlier unused submission
(Shixiong Ou, 2026-08-28) tagged `Fixes: 83374e035b62`. That introducing
commit **is** in this tree.
**Step 3.3: File history**
Record (path-limited log):
- `83374e035b628` add tracepoints
- later style/`__vstring`/`__assign_str` churn
- this fix
Standalone; not patch X/Y. No prerequisite refactor in this file beyond
what 6.18 already has (`__assign_str(name)` without a second argument is
already here).
**Step 3.4: Author**
Record: Vladimir Murzin is not the mtu3 maintainer (that is Chunfeng
Yun, CC’d). He is an ARM engineer fixing a splat he, Paul, and Mark hit.
Rostedt reviewed. Paul merged it when USB maintainers were slow (splat
still in 7.3-rc6).
**Step 3.5: Dependencies**
Record: None. Uses existing `dma_addr_t`, `TP_*`, and
`mep->gpd_ring.dma`. No new tracing helpers. `b5cc230af5e5` is the
detector, **not** a prerequisite for the fix.
---
## Phase 4: Mailing list / external
**Step 4.1: Original discussion**
Record:
- `b4 dig -c cfb3293c4573289f11f913611f530ea6b1926d67` → https://patch.m
sgid.link/20260922103756.104846-1-vladimir.murzin@arm.com
- `b4 dig -a`: single revision (v1 only)
- Thread: ping for 7.3-rc5/rc6 splat; Paul offered to merge because the
WARN broke his tests; later merged and Linus pulled it
- No NAK. No reviewer asked for `Cc: stable`
- lore.kernel.org fetch returned 403; thread recovered via `b4 dig -m`
**Step 4.2: Recipients (`b4 dig -w`)**
Record: linux-usb, linux-arm-kernel, linux-mediatek, linux-kernel,
Chunfeng Yun, Greg KH, Steven Rostedt, Paul McKenney, Mark Rutland.
Right lists and maintainers.
**Step 4.3: Bug report**
Record: Reproduced by Paul, Mark, and Vladimir on 7.3-rc1+ (dummy-virt).
Independent earlier patch from Shixiong Ou describes the same WARN and
the same stale-pointer/UAF. Severity for **those** kernels is a boot
WARN whenever `CONFIG_USB_MTU3` is built. That WARN is **not** in
6.18.y.
**Step 4.4: Series**
Record: Standalone. Duplicate independently submitted by Shixiong Ou
(same approach, field named `gpd_dma`). Murzin’s version is what merged.
**Step 4.5: Stable list**
Record: No useful hit on lore stable for this subject. Not a documented
“do not backport.”
---
## Phase 5: Semantic / call graph
**Step 5.1: Functions**
Record: `mtu3_log_ep` class; events `mtu3_gadget_ep_enable`,
`mtu3_gadget_ep_disable`, `mtu3_gadget_ep_set_halt`.
**Step 5.2: Callers**
Record (this tree):
- `mtu3_gadget_ep_enable()` — `drivers/usb/mtu3/mtu3_gadget.c:202`
- `mtu3_gadget_ep_disable()` — line 214 (before `mtu3_ep_disable()` /
`mtu3_gpd_ring_free()`)
- `mtu3_gadget_ep_set_halt()` — line 404
These are USB gadget ep ops: enable/disable/halt from gadget drivers /
configfs. Common on MediaTek gadget/OTG, not a syscall of its own.
**Step 5.3: Callees**
Record: TRACE_EVENT assign/print only. No alloc/lock/I/O in the changed
code. `%pad` → `address_val()` → dereference `dma_addr_t *`.
**Step 5.4: Reachability**
Record: Recording: gadget ep enable/disable/halt with these events on.
Printing: reading the trace buffer. UAF: print after `mtu3_ep` is freed
(driver unbind). Boot WARN: only if `test_double_dereference()` exists
(it does not here). Unprivileged users normally cannot enable tracefs.
**Step 5.5: Similar patterns**
Record: Other mtu3 events print saved pointers with `%p` only (safe).
Same class already in this 6.18.y tree: `0b0d2f9db2406` (“eth: ice:
don't dereference pointers from TP_printk()”), also written against
`b5cc230af5e5`, and it **is** an ancestor of `v6.18.57`.
---
## Phase 6: This checkout only
**Step 6.1: Does the buggy code exist?**
Record: **Yes, in v6.18.57.** `git describe HEAD` →
`v6.18.57-155-gcdf239ca885b2`; `make kernelversion` → **6.18.57**.
v6.18.57 `mtu3_trace.h` still has `__entry->gpd_ring,
&__entry->gpd_ring->dma`. Introduced 2019. `git merge-base --is-ancestor
dff6c93cf7131 v6.18.57` failed (fix not in 6.18.57).
`test_double_dereference` is **absent** from `kernel/trace/`
(`b5cc230af5e5` is not an ancestor of HEAD).
**Step 6.2: Backport difficulty**
Record: **Clean apply.** `git diff v6.18.57 dff6c93cf7131 --
drivers/usb/mtu3/mtu3_trace.h` is exactly this 4-line change. Context
matches 6.18 (`__assign_str(name)` already present).
**Step 6.3: Related fix already here?**
Record: No other mtu3 `TP_printk` fix in `v6.18..v6.18.57`. The ice
analogue **is** already in 6.18.57. That does not replace this mtu3 bug.
---
## Phase 7: Subsystem context
**Step 7.1**
Record: **drivers/usb/mtu3** — MediaTek USB3 dual-role. Criticality:
**PERIPHERAL/IMPORTANT for MTK** (`CONFIG_USB_MTU3`, `depends on
ARCH_MEDIATEK || COMPILE_TEST`). Tracepoints compiled with the driver
when `CONFIG_TRACEPOINTS` is on.
**Step 7.2**
Record: Mature driver (tracepoints since 2019). Recent 6.18.y mtu3 fixes
include DMA unmap on queue failure and suspend-with-gadget. Not staging.
---
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
Record: MediaTek USB3 gadget users who enable these ftrace events and
later print the buffer. Distro kernels that build mtu3 for
`COMPILE_TEST` do **not** get the 7.3 boot WARN here, because that check
is missing.
**Step 8.2: Trigger**
Record: Not every boot on 6.18.y. Needs `CONFIG_USB_MTU3` + tracepoints
+ enable these events + later print. UAF additionally needs the
endpoint/device object to have been freed. Not unprivileged. The 7.3
boot WARN fires even on dummy-virt; **that path does not exist in this
tree**.
**Step 8.3: Failure mode**
Record:
- On 7.3+: boot `WARNING` — **does not apply to 6.18.57**
- On this tree: delayed deref → **garbage DMA address in traces**, or
**UAF/oops** if `mtu3_ep` was freed — severity **HIGH** when hit,
**MEDIUM** overall because the trigger is a debug path
**Step 8.4: Risk vs benefit**
Record: **Benefit:** closes a real stale-pointer/UAF in mtu3 gadget
traces; same class already taken for ice on this series. **Risk:** very
low (3 insertions, print-time only). Ratio favors backport.
---
## Phase 9: Synthesis
**Step 9.1: Evidence**
For:
- Real delayed dereference of a kernel pointer from the trace buffer
(`%pad` confirmed to dereference)
- Buggy code present in this 6.18.y tree since 2019
- Small, obviously correct, applies cleanly
- Reviewed by Rostedt; reported/tested by McKenney and Rutland
- Same class (`TP_printk` must not follow saved pointers) already in
v6.18.57 for ice
- No new API/feature; no series dependency
Against:
- The boot splat that drove the patch **cannot happen here**
(`test_double_dereference` / `b5cc230af5e5` not in 6.18.y)
- Debug/tracing path, MediaTek-specific, needs events enabled
- Failure is often wrong trace text rather than a production USB failure
- No `Cc: stable` in review (not a negative per the rules)
Unresolved: lore.kernel.org HTTP 403 (thread read from `b4` mbox
instead).
**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** (Rostedt Reviewed-by, Mark
Tested-by, logic is local)
2. Real bug that affects users? **PASS** (stale pointer / UAF when
traces are printed)
3. Important (crash/security/corruption/deadlock)? **PASS** (UAF/oops
possible on print after free; garbage traces otherwise). The 7.3 boot
WARN is not the important part **for this tree**.
4. Small and contained? **PASS** (one header, 4 lines)
5. No new features/APIs? **PASS**
6. Can apply to this tree? **PASS** (clean vs v6.18.57)
**Step 9.3: Exception category**
Record: None (not a device ID/quirk/DT/build/docs exception). It is a
normal small bugfix.
**Step 9.4: Decision**
The 7.3 boot WARN is not a reason to take this on 6.18.y. The reason to
take it is the pre-existing `TP_printk` dereference, which **is** in
this tree, is a real UAF/garbage-print bug, is a 4-line no-brainer, and
matches a fix this series already accepted for ice.
---
## What this means for 6.18.y users
`mtu3_log_ep` stores `&mep->gpd_ring` and later does `%pad` on
`&gpd_ring->dma`. Print can happen long after disable/unbind. That is a
real bug in this tree. The patch only snapshots `gpd_ring.dma` at event
time. It does not change gadget behavior.
## Verification
- **[Phase 1]** Parsed tags: Reported-by Paul McKenney, Tested-by Mark
Rutland, Reviewed-by Steven Rostedt; no Fixes/Cc-stable
- **[Phase 2]** Diff: +`gpd_ring_dma` field, assign `mep->gpd_ring.dma`,
print `&__entry->gpd_ring_dma`
- **[Phase 2]** `%pad` in `lib/vsprintf.c` `address_val()`: `num =
*(const dma_addr_t *)addr`
- **[Phase 2]** `struct mtu3_ep` has embedded `struct mtu3_gpd_ring
gpd_ring`; `mtu3_gpd_ring_free()` memsets it
- **[Phase 3]** Blame: `83374e035b628` (2019) introduced the pattern;
ancestor of HEAD
- **[Phase 3]** `git merge-base --is-ancestor b5cc230af5e5 HEAD` failed;
no `test_double_dereference` in this tree
- **[Phase 3]** Author log on `drivers/usb/`: this commit plus unrelated
xhci work
- **[Phase 4]** `b4 dig -c cfb3293c4573`: msgid
`20260922103756.104846-1-vladimir.murzin@arm.com`; v1 only
- **[Phase 4]** `b4 dig -w`: USB/Mediatek lists, Greg KH, Chunfeng Yun,
Rostedt
- **[Phase 4]** mbox: Paul merged after 7.3-rc5/rc6 pings; no NAK; no
stable nomination
- **[Phase 4]** UNVERIFIED: live lore.kernel.org page (403)
- **[Phase 5]** Callers: `mtu3_gadget_ep_enable/disable/set_halt` in
`mtu3_gadget.c`
- **[Phase 6]** `git describe HEAD` = v6.18.57-155-gcdf239ca885b2;
kernelversion 6.18.57
- **[Phase 6]** v6.18.57 still has `&__entry->gpd_ring->dma`; fix not an
ancestor of v6.18.57
- **[Phase 6]** File diff vs v6.18.57 is exactly this patch (clean)
- **[Phase 6]** ice TP_printk analogue `0b0d2f9db2406` **is** in
v6.18.57
- **[Phase 8]** Boot WARN not reachable here; UAF/garbage on delayed
print is
The fix is small, correct, already matches this tree’s context, and
closes a real tracing UAF, not a 7.3-only WARN.
**YES**
drivers/usb/mtu3/mtu3_trace.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/mtu3/mtu3_trace.h b/drivers/usb/mtu3/mtu3_trace.h
index 89870175d6356..9aaa167d69c13 100644
--- a/drivers/usb/mtu3/mtu3_trace.h
+++ b/drivers/usb/mtu3/mtu3_trace.h
@@ -224,6 +224,7 @@ DECLARE_EVENT_CLASS(mtu3_log_ep,
__field(unsigned int, flags)
__field(unsigned int, direction)
__field(struct mtu3_gpd_ring *, gpd_ring)
+ __field(dma_addr_t, gpd_ring_dma)
),
TP_fast_assign(
__assign_str(name);
@@ -235,12 +236,13 @@ DECLARE_EVENT_CLASS(mtu3_log_ep,
__entry->flags = mep->flags;
__entry->direction = mep->is_in;
__entry->gpd_ring = &mep->gpd_ring;
+ __entry->gpd_ring_dma = mep->gpd_ring.dma
),
TP_printk("%s: type %s maxp %d slot %d mult %d burst %d ring %p/%pad flags %c:%c%c%c:%c",
__get_str(name), usb_ep_type_string(__entry->type),
__entry->maxp, __entry->slot,
__entry->mult, __entry->maxburst,
- __entry->gpd_ring, &__entry->gpd_ring->dma,
+ __entry->gpd_ring, &__entry->gpd_ring_dma,
__entry->flags & MTU3_EP_ENABLED ? 'E' : 'e',
__entry->flags & MTU3_EP_STALL ? 'S' : 's',
__entry->flags & MTU3_EP_WEDGE ? 'W' : 'w',
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.10] usb: mtu3: Fix double dereference in TP_printk
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] usb: mtu3: Fix double dereference in TP_printk Sasha Levin
@ 2026-10-19 14:18 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-19 14:18 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-usb
> From: Vladimir Murzin <vladimir.murzin@arm.com>
>
> [ Upstream commit cfb3293c4573289f11f913611f530ea6b1926d67 ]
>
> Paul reported kernel splat:
>
> [ 0.000000] TRACE EVENT ERROR: Event mtu3_gadget_ep_set_halt has double dereference in TP_printk: &REC->gpd_ring->dma
> [ 0.000000] ------------[ cut here ]------------
> [ 0.000000] Event mtu3_gadget_ep_set_halt has double dereference in TP_printk: &REC->gpd_ring->dma
> [ 0.000000] WARNING: kernel/trace/trace_events.c:420 at test_double_dereference+0x144/0x14c, CPU#0: swapper/0/0
> [ 0.000000] Modules linked in:
> [ 0.000000] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 7.3.0-rc1 #15247 PREEMPT
> [ 0.000000] Hardware name: linux,dummy-virt (DT)
> [ 0.000000] pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 0.000000] pc : test_double_dereference+0x144/0x14c
> [ ... ]
> Tested-by: Mark Rutland <mark.rutland@arm.com>
> Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-102-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] net: usb: rtl8150: handle link status read failures
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (5 preceding siblings ...)
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] usb: mtu3: Fix double dereference in TP_printk Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 14:20 ` sashiko-bot
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] usb: musb: host: clear stale RX interrupt on three-strikes error Sasha Levin
` (6 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Yousef Alhouseen, syzbot+9db6c624635564ad813c, Petko Manolov,
Andrew Lunn, Paolo Abeni, Sasha Levin, andrew+netdev, davem,
edumazet, kuba, linux-usb, netdev, linux-kernel
From: Yousef Alhouseen <alhouseenyousef@gmail.com>
[ Upstream commit 8c9c5b9a689612dcb92a04a4218c975cd19f19d8 ]
set_carrier() ignores the result of the USB control transfer and tests
the stack variable supplied as its receive buffer. If the device rejects
or aborts the request, that variable remains uninitialized and the driver
chooses an arbitrary carrier state.
Leave the existing carrier state unchanged when the link status cannot be
read. A transient USB error should not be treated as link loss.
Reported-by: syzbot+9db6c624635564ad813c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9db6c624635564ad813c
Suggested-by: Petko Manolov <petkan@nucleusys.com>
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Link: https://patch.msgid.link/20260630101216.10365-1-alhouseenyousef@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1 — Subject**
Record: `[net: usb: rtl8150] [handle] ignore USB register-read errors in
set_carrier() so an uninitialized stack value is not used as link
status`
**Step 1.2 — Tags**
Record:
- `Reported-by: syzbot+9db6c624635564ad813c@syzkaller.appspotmail.com`
(KMSAN, reproducible)
- `Closes: https://syzkaller.appspot.com/bug?extid=9db6c624635564ad813c`
- `Suggested-by: Petko Manolov <petkan@nucleusys.com>` (original rtl8150
author)
- `Reviewed-by: Andrew Lunn <andrew@lunn.ch>`
- `Link: https://patch.msgid.link/20260630101216.10365-1-
alhouseenyousef@gmail.com`
- `Signed-off-by: Yousef Alhouseen` (author), `Paolo Abeni` (netdev)
- No `Fixes:` on the committed form (dropped in v4 at reviewer request;
v1/v2 had `Fixes: 1da177e4c3f4` and `Cc: stable`)
- Sasha Levin SOB ignored as pipeline noise
**Step 1.3 — Body**
Record: `set_carrier()` ignores `get_registers()` and then tests stack
`tmp`. On USB reject/abort, `tmp` is uninitialized, so carrier is chosen
from garbage. Failure mode: arbitrary `netif_carrier_on/off`. Fix: keep
the current carrier if the CSCR read fails. No version range given.
**Step 1.4 — Hidden bug fix?**
Record: Not hidden. This is an explicit uninitialized-use / ignored-
error fix, confirmed by KMSAN.
## Phase 2: Diff analysis
**Step 2.1 — Inventory**
Record: one file, `drivers/net/usb/rtl8150.c`; `set_carrier()` only; +3
/ −1; surgical single-function change.
**Step 2.2 — Flow**
Record: Before: always `get_registers(dev, CSCR, 2, &tmp)` then `tmp &
CSCR_LINK_STATUS` → carrier on/off. After: non-zero return from
`get_registers()` returns immediately; carrier is unchanged. Path:
`rtl8150_open()` (interface up).
**Step 2.3 — Bug mechanism**
Record: **uninitialized data** (KMSAN). `usb_control_msg_recv()` only
`memcpy`s into the caller buffer on a full successful transfer
(`drivers/usb/core/message.c`); on error it returns negative and leaves
`tmp` untouched. `short tmp` is uninitialized, then used as a bit test.
**Step 2.4 — Fix quality**
Record: Obviously correct. Three lines. No API/lock/refcount change.
Regression risk is low: a failed CSCR read no longer flips carrier. That
matches the original author’s review (transient USB errors should not
look like link loss).
## Phase 3: Git history
**Step 3.1 — Blame**
Record: Unchecked `get_registers()` + `tmp & CSCR_LINK_STATUS` dates to
`1da177e4c3f4` (Linux 2.6.12-rc2, 2005). The uninit became hard after
`b2a0f274e3f7e` (“Use the new usb control message API”, in `v5.10-rc1`):
`usb_control_msg_recv()` does not fill the buffer on failure.
**Step 3.2 — Fixes tag**
Record: Committed patch has no `Fixes:`. v1/v2 used `Fixes:
1da177e4c3f4`. That SHA is in this tree. The USB API switch
`b2a0f274e3f7e` is also in this tree (`v5.10-rc1`).
**Step 3.3 — Related file history**
Record: Same driver already has `fba383985354e` (“fix unintiatilzed
variables in rtl8150_get_link_ksettings”), a sibling KMSAN
ignored-`get_registers()` fix, present in `v6.18.53`. Other recent
rtl8150 work is UAF/leak/padding, not a substitute for this carrier
check. Standalone, not patch X/Y.
**Step 3.4 — Author**
Record: Yousef Alhouseen; this is their only rtl8150 commit here.
Suggested-by Petko Manolov (driver author). Reviewed-by Andrew Lunn.
**Step 3.5 — Dependencies**
Record: None. Uses existing `get_registers()` return value. `git diff
v6.18.53 6a6d945b860b3^ -- drivers/net/usb/rtl8150.c` is empty —
preimage matches 6.18.y.
## Phase 4: Mailing list / external
**Step 4.1 — Discussion**
Record: `b4 dig` → https://patch.msgid.link/20260628165033.17842-1-
alhouseenyousef@gmail.com
Revisions: v1 (carrier-off on error) → v2 (return on error) → net-next
v4 (applied).
Petko: do not force carrier off; USB messages can be lost while the
device still works.
Andrew Lunn (v2): pointed at stable-kernel-rules, asked “Does this issue
bother people?”, asked for net-next, Suggested-by, and no version
threading.
v4 changelog: “Target net-next and drop the Fixes tag as requested.”
Andrew Reviewed-by v4. Applied to netdev/net-next as `8c9c5b9a6896`. No
NAK of the code.
**Step 4.2 — Recipients**
Record: Petko, Lunn, davem, Dumazet, Kicinski, Abeni, linux-usb, netdev,
syzbot. Right lists and maintainers.
**Step 4.3 — Bug report**
Record: syzbot “KMSAN: uninit-value in rtl8150_open (2)”, prio:high.
Stack: `set_carrier` ← `rtl8150_open` ← `__dev_open` ← ioctl (dhcpcd).
Local `tmp`. Console: `usb 2-1: device reset failed`. First crash ~186d
before the report page, last ~113d. Security workflow: PeripheralTrigger
yes; DoS/exploitable/unprivileged no. Five KMSAN crashes on dummy USB.
**Step 4.4 — Series**
Record: Single patch. Not part of a multi-patch series.
**Step 4.5 — Stable list**
Record: lore.kernel.org HTTP 403; no extra stable-list thread retrieved.
v1/v2 were Cc: stable; Andrew redirected to net-next. That is process,
not a technical NAK.
## Phase 5: Semantics / call graph
**Step 5.1**
Record: `set_carrier()` only.
**Step 5.2**
Record: Only caller is `rtl8150_open()` (`drivers/net/usb/rtl8150.c`).
Link is also updated later from `intr_callback()` via `MSR_LINK`
(independent path, 100 ms interrupt URB).
**Step 5.3**
Record: `get_registers()` → `usb_control_msg_recv()`; then
`netif_carrier_on` / `netif_carrier_off`.
**Step 5.4**
Record: Reachable from userspace bringing the iface up (`SIOCSIFFLAGS` /
`dev_change_flags` / `__dev_open`). Needs the USB device (or a gadget).
Typically `CAP_NET_ADMIN`. Syzbot hit it via dhcpcd on a dummy device.
**Step 5.5**
Record: Same ignored-return pattern exists elsewhere in this file
(`read_mii_word`, `rtl8150_reset`, `enable_net_traffic`,
`get_link_ksettings`). `get_link_ksettings` was already hardened by
initializing locals (`fba383985354e`). This commit only fixes
`set_carrier()`.
## Phase 6: This tree (6.18.y)
**Step 6.1**
Record: Local tree is **6.18.53** (`git describe HEAD` →
`v6.18.53-5049-g0f0bdc2b0dffb`, `VERSION=6 PATCHLEVEL=18 SUBLEVEL=53`).
`v6.18.53:drivers/net/usb/rtl8150.c` still has the unchecked
`get_registers(dev, CSCR, 2, &tmp)`. Bug is present. Introduced in
practice with `usb_control_msg_recv` in v5.10; the ignore-return pattern
is ancient.
**Step 6.2**
Record: Clean apply. Parent of the candidate matches `v6.18.53` for this
file (0-line diff).
**Step 6.3**
Record: No other commit in 6.18.y checks `set_carrier()`’s CSCR read.
`fba383985354e` is related but only ethtool ksettings.
## Phase 7: Subsystem
**Step 7.1**
Record: USB net driver `CONFIG_USB_RTL8150`. Criticality: **PERIPHERAL**
(legacy Realtek USB 10/100 dongle; not core mm/net). Still a supported
in-tree driver, not staging.
**Step mar 7.2**
Record: Low-churn mature driver. Occasional syzbot fixes. Stable-
relevant bugs do still land here.
## Phase 8: Impact / risk
**Step 8.1**
Record: Users of `CONFIG_USB_RTL8150` only, plus KMSAN/syzbot coverage.
**Step 8.2**
Record: Trigger: CSCR USB control read fails during `rtl8150_open()`
(device reset failure, disconnect, bus noise). Uncommon on working
hardware; easy with a dummy/failing USB device. Not a general
unprivileged attack path.
**Step 8.3**
Record: Production: wrong carrier (link up/down from stack garbage). Not
a panic/UAF. Under KMSAN: `BUG: KMSAN: uninit-value`. Interrupt path can
correct carrier within ~100 ms if the device still works. Severity:
**HIGH** as uninitialized memory (KMSAN); **MEDIUM** as user-visible
wrong link state on USB error.
**Step 8.4**
Record: Benefit: stops UB and random carrier on open failure; matches a
prior stable fix in the same file. Risk: very low (3 lines, error path
only, no locking). Ratio favors backport.
## Phase 9: Synthesis
**For backporting**
- Real ignored-error / uninitialized-stack bug, not cleanup.
- Syzbot + KMSAN, reproducible from iface open.
- Same class as `fba383985354e`, already in this 6.18.y tree.
- Three-line, reviewed, Suggested-by the driver author.
- Buggy code is in `v6.18.53`; applies cleanly; no prerequisites.
- Wrong carrier on `ifup` is user-visible if the control read fails.
**Against**
- Andrew Lunn asked whether it “bothers people” and sent it to net-next,
not `net`.
- Not a production oops, deadlock, or security hole (syzbot: not
exploitable/DoS).
- Legacy USB adapter; trigger is a failed control transfer.
- Interrupt callback often repairs carrier shortly after.
**Unresolved**
- Could not fetch lore.kernel.org/stable (HTTP 403). Discussion is taken
from `b4` mboxes instead.
**Stable-rules checklist**
1. Obviously correct and tested? **PASS** — 3 lines, Reviewed-by Lunn,
Suggested-by Petko, syzbot-confirmed.
2. Real bug affecting users? **PASS** — uninit `tmp` used for carrier;
KMSAN + ioctl path.
3. Important (crash/security/corruption/deadlock)? **PASS** — KMSAN
uninitialized use is a real memory-safety bug; production symptom is
incorrect carrier, not a panic. Bar is met for sanitizer-class fixes
this pipeline treats as backport-worthy.
4. Small and contained? **PASS** — one function, +3/−1.
5. No new features/APIs? **PASS**.
6. Applies to this tree? **PASS** — 6.18.y has the bug and matching
preimage.
**Exception category:** none (not an ID/quirk/DT/build/doc exception).
It is a normal small bugfix.
The net-next routing only dropped tags for netdev process. It does not
remove the bug from 6.18.y. The patch is self-contained, applies
cleanly, and is the same error-check this driver already needed in
ethtool.
Verification:
- [Phase 1] Tags: syzbot Reported-by, Closes syzkaller URL, Suggested-by
Petko, Reviewed-by Andrew Lunn, Paolo Abeni SOB.
- [Phase 2] Diff: `set_carrier()` adds `if (get_registers(...))
return;`.
- [Phase 2] `usb_control_msg_recv()` in `drivers/usb/core/message.c`
only copies into the caller buffer when `ret == size`; errors skip
that copy.
- [Phase 3] Blame: logic since `1da177e4c3f4`; `usb_control_msg_recv`
since `b2a0f274e3f7e` (`v5.10-rc1`).
- [Phase 3] `git diff v6.18.53 6a6d945b860b3^ --
drivers/net/usb/rtl8150.c` is empty.
- [Phase 3] Sibling KMSAN fix `fba383985354e` is in `v6.18.53`.
- [Phase 4] `b4 dig -c 8c9c5b9a6896`: v1–v4; Petko requested leave-
carrier-unchanged; Lunn asked for net-next; v4 applied to net-next.
- [Phase 4] Syzkaller: KMSAN uninit-value in `rtl8150_open`, prio:high,
dhcpcd ioctl, `device reset failed`.
- [Phase 5] Only caller: `rtl8150_open`; `intr_callback` updates carrier
separately.
- [Phase 6] `git describe HEAD` = `v6.18.53-5049-g0f0bdc2b0dffb`; `make
kernelversion` = 6.18.53; buggy code present at `v6.18.53`.
- [Phase 6] `git merge-base --is-ancestor 6a6d945b860b3 v6.18.53` → not
in the release tag (candidate only).
- [Phase 8] Failure: uninitialized stack → arbitrary carrier; KMSAN BUG
under sanitizer.
- UNVERIFIED: lore.kernel.org WebFetch 403; stable@ archive not searched
via HTTP.
**YES**
drivers/net/usb/rtl8150.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 647f28b367b99..8c739d898e57d 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -734,7 +734,9 @@ static void set_carrier(struct net_device *netdev)
rtl8150_t *dev = netdev_priv(netdev);
short tmp;
- get_registers(dev, CSCR, 2, &tmp);
+ if (get_registers(dev, CSCR, 2, &tmp))
+ return;
+
if (tmp & CSCR_LINK_STATUS)
netif_carrier_on(netdev);
else
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.18-5.10] usb: musb: host: clear stale RX interrupt on three-strikes error
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (6 preceding siblings ...)
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] net: usb: rtl8150: handle link status read failures Sasha Levin
@ 2026-10-19 13:47 ` Sasha Levin
2026-10-19 14:28 ` sashiko-bot
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.10] USB: misc: uss720: Fix leak in get_1284_register Sasha Levin
` (5 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
To: patches, stable
Cc: Lucas Martins Alves, Greg Kroah-Hartman, Sasha Levin, b-liu,
linux-usb, linux-kernel
From: Lucas Martins Alves <lucas.alves@lumal21.com.br>
[ Upstream commit 3a3a48487344a653f46a4aadc9ba296386eb3541 ]
Unplugging a busy USB Ethernet adapter can leave a stale RX interrupt
pending while the host handles MUSB_RXCSR_H_ERROR. On AM335x/DSPS
platforms this can retrigger the same three-strikes error before the
disconnect path completes, flooding the log and potentially leaving the
host port stuck until reboot.
Clear the pending endpoint RX interrupt when aborting the transfer so
the error storm is broken, and rate limit the error message to avoid
spamming the log while the fault path completes.
Signed-off-by: Lucas Martins Alves <lucas.alves@lumal21.com.br>
Link: https://patch.msgid.link/20260714134941.18508-1-lucas.alves@lumal21.com.br
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Local tree
This checkout is **Linux 7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`),
i.e. current mainline, not a `-stable` branch. The candidate is already
in this history as `3a3a48487344a`. The verdict below is whether
**this** tree’s code still has the bug and whether the fix is a suitable
stable backport of that code — not whether some other series should take
it.
---
## Phase 1: Commit message forensics
**Step 1.1 — Subject**
- Subsystem: `usb: musb: host:`
- Action verbs: **clear** (stale IRQ), plus body **rate limit**
- Intent: stop a leftover RX IRQ from re-firing the three-strikes error
path after unplug
Record: `[usb/musb host] [clear] [ACK leftover RX IRQ on three-strikes
so unplug cannot storm/wedge the host]`
**Step 1.2 — Tags**
- `Signed-off-by: Lucas Martins Alves <lucas.alves@lumal21.com.br>`
(author)
- `Link: https://patch.msgid.link/20260714134941.18508-1-
lucas.alves@lumal21.com.br`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (USB
maintainer merge)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence is expected for this pipeline)
Record: author SOB + lore msgid + Greg KH SOB. No syzbot, no Fixes tag.
**Step 1.3 — Body**
- Bug: unplug of a **busy USB Ethernet adapter** leaves a pending RX
interrupt while `MUSB_RXCSR_H_ERROR` is handled.
- Symptom: on **AM335x/DSPS**, the same three-strikes error retriggers
before disconnect finishes → **log flood** and **host port stuck until
reboot**.
- Root cause (author): stale endpoint RX interrupt not cleared on abort.
- Fix: `musb_platform_clear_ep_rxintr()` plus `dev_err_ratelimited`.
Record: `[interrupt storm / wedged MUSB host port on unplug] [log spam +
port dead until reboot] [AM335x/DSPS] [stale RX IRQ reasserts H_ERROR]`
**Step 1.4 — Hidden bug fix?**
Not hidden. It is an explicit hardware-IRQ abort fix. Rate-limit is
secondary hardening for the same storm.
Record: `[Not a disguised cleanup; it is a stated interrupt-storm / hang
fix]`
---
## Phase 2: Diff analysis
**Step 2.1 — Inventory**
- File: `drivers/usb/musb/musb_host.c` only (`+13 / -1`)
- Function: `musb_host_rx()`
- Scope: single-file, one error branch
Record: `[musb_host.c +13/-1] [musb_host_rx] [surgical single-file]`
**Step 2.2 — Flow**
Before: on `MUSB_RXCSR_H_ERROR`, log with `dev_err`, set `-ESHUTDOWN`,
zero RX interval, clear `H_ERROR` in RXCSR, then fall through to the
existing fault abort (`musb_h_flush_rxfifo` / giveback).
After: same abort, but (1) `dev_err_ratelimited` with a trailing `\n`,
and (2) `musb_platform_clear_ep_rxintr(musb, epnum)` after clearing
RXCSR.
Record: `[H_ERROR branch: abort URB as before → also ACK platform RX IRQ
and rate-limit the printk]`
**Step 2.3 — Bug mechanism**
Category: **hardware interrupt storm / stale IRQ** (same family as
resource-leak-on-error, but the resource is a latched IRQ).
Verified mechanism:
1. `dsps_interrupt()` reads `epintr_status`, ACKs it, then calls
`musb_interrupt()` → `musb_host_rx()`.
2. If the Mentor core re-latches RX while `H_ERROR` is still being
unwound, DSPS delivers another RX IRQ before disconnect.
3. `dsps_musb_clear_ep_rxintr()` writes `(1 << epnum) <<
wrp->rxep_shift` to `epintr_status` — the same ACK used in
`musb_cleanup_urb()` (“clear the endpoint's irq status here to avoid
bogus irqs”).
4. The helper is a **no-op** unless `ops->clear_ep_rxintr` is set; only
DSPS implements it.
Record: `[stale IRQ storm] [ACK DSPS ep RX status after H_ERROR so
musb_host_rx is not re-entered]`
**Step 2.4 — Fix quality**
Obviously correct: it reuses a 2017 platform hook already used for the
same “bogus RX IRQ” class. No API/data-structure change. Regression risk
is low: non-DSPS platforms no-op; DSPS only ACKs the endpoint whose
error was just handled. Rate-limit cannot change USB behavior. Adding
`\n` only fixes printk formatting.
Record: `[minimal, matches existing abort path] [low regression risk]`
---
## Phase 3: Git history
**Step 3.1 — Blame**
- `H_ERROR` handling exists since `550a7375fe720` (Felipe Balbi, 2008).
- `-ESHUTDOWN` / “three-strikes” comment: `1b967691ecdcf` (Bin Liu,
2020-05-25, v5.7-rc6 era) — same disconnect/storm class.
- This patch only owns the new call and the ratelimited log.
Record: `[buggy H_ERROR path: 2008; incomplete storm mitigation:
1b967691ecdcf ~v5.7] [long-lived]`
**Step 3.2 — Fixes: tag**
None. The incomplete predecessor is `1b967691ecdcf` (“return -ESHUTDOWN
… three-strikes”), present in this tree (`git merge-base --is-ancestor`
succeeded).
Record: `[no Fixes tag; predecessor 1b967691ecdcf is in this tree]`
**Step 3.3 — File history**
Related:
- `dbac5d07d13e3` (2016): don’t start next RX URB on failure — **system
freeze** on USB-UART unplug / RX IRQ flood.
- `6def85a396ce7` + `c48400baa0215` (2017, v4.10): add/implement
`clear_ep_rxintr` for bogus RX IRQ during DMA teardown; **already Cc:
stable # 4.1+**.
- No later `musb_host.c` commits after this candidate.
Record: `[standalone; same storm family as 2016/2017/2020 musb host
fixes] [no later conflicting musb_host.c commits]`
**Step 3.4 — Author**
Lucas Martins Alves has only this USB commit in-tree. Merge via Greg KH.
Recipients included Bin Liu (`b-liu@ti.com`, long-time MUSB/DSPS
author).
Record: `[outside contributor; Greg KH merged; TI musb maintainer was on
Cc]`
**Step 3.5 — Prerequisites**
Needs `musb_platform_clear_ep_rxintr()` / `ops->clear_ep_rxintr`. Both
are ancestors of HEAD (`6def85a396ce7`, `c48400baa0215`). Call site in
`musb_cleanup_urb()` already exists. Applies standalone.
Record: `[depends on 2017 helper, present here] [standalone apply yes]`
---
## Phase 4: Mailing list / external
**Step 4.1**
`b4 dig -c 3a3a48487344a` → v1 only:
https://patch.msgid.link/20260714134941.18508-1-
lucas.alves@lumal21.com.br
Submitted 2026-07-14, merged 2026-07-17. Saved mbox is **only the
patch**; no replies, NAKs, or `Cc: stable` discussion. WebFetch to lore
returned **403** (verified).
Record: `[lore msgid above] [v1 only, no review comments in thread] [no
NAK]`
**Step 4.2**
`b4 dig -w`: To/Cc Lucas, `b-liu@ti.com`, `gregkh@linuxfoundation.org`,
`linux-usb`, `linux-kernel`.
Record: `[USB maintainer + TI musb author + linux-usb were on the
original To/Cc]`
**Step 4.3**
No `Reported-by` / bugzilla / syzbot. Body reads as first-hand AM335x
unplug. User-count beyond the author is **unverified**.
Record: `[no public bugzilla/syzbot] [UNVERIFIED: how many users besides
the author]`
**Step 4.4**
`b4 dig -a`: single standalone v1, not a series.
Record: `[standalone, not patch X/Y]`
**Step 4.5**
lore.kernel.org/stable WebFetch: **403**. No stable-list discussion
verified.
Record: `[UNVERIFIED: stable@ discussion]`
---
## Phase 5: Semantic / call graph
**Step 5.1**
Modified: `musb_host_rx()`. New call: `musb_platform_clear_ep_rxintr()`
→ `dsps_musb_clear_ep_rxintr()` on DSPS.
**Step 5.2 — Callers** (from `drivers/usb/musb/musb_core.c`)
- `musb_interrupt()`: host IRQ, `for_each_set_bit` on `int_rx`
- `musb_dma_completion()`: DMA completion RX path
Called from hardirq / DMA completion under `musb->lock`. Common host RX
path, not init-only.
**Step 5.3 — Callees in the hunk**
`dev_err_ratelimited`, `musb_writeb` (interval), `musb_writew` (RXCSR),
`musb_platform_clear_ep_rxintr`. Fault path still does
`musb_h_flush_rxfifo` when `status != 0`.
**Step 5.4 — Reachability**
Yes: USB Ethernet bulk IN while the device is yanked. Unprivileged
physical unplug. DSPS glue is `CONFIG_USB_MUSB_DSPS` (`ARCH_OMAP2PLUS` —
AM335x BeagleBone / Sitara class).
**Step 5.5 — Similar pattern**
Same helper already used in `musb_cleanup_urb()` for DMA abort. TX
`H_ERROR` still uses `musb_dbg` and does **not** clear RX IRQ (TX path;
out of this patch’s scope). Historical RX-flood fixes: `dbac5d07d13e3`,
`1b967691ecdcf`.
Record: `[musb_host_rx from IRQ/DMA] [userspace-triggerable unplug]
[same helper as cleanup_urb]`
---
## Phase 6: This checkout
**Step 6.1**
Buggy `H_ERROR` branch is in `musb_host_rx()` here. Helper and DSPS
callback exist. Bug class dates to 2008; `-ESHUTDOWN` mitigation from
~5.7 did not clear the DSPS RX IRQ.
Record: `[yes, musb_host.c H_ERROR path] [introduced 2008 / incomplete
2020]`
**Step 6.2**
Context around the hunk matches the candidate 1:1.
`musb_platform_clear_ep_rxintr` already declared. Expected apply:
**clean**.
Record: `[clean apply on this tree]`
**Step 6.3**
No later musb_host.c commit supersedes this. 2016/2020 patches are
related but **not** equivalent (they stop URB resubmit / next URB start;
they do not ACK `epintr_status`).
Record: `[no substitute fix already covering the stale DSPS RX IRQ]`
---
## Phase 7: Subsystem
**Step 7.1**
`drivers/usb/musb` — Mentor USB OTG host on TI DSPS/AM335x. Criticality:
**IMPORTANT** for those platforms (USB host), **PERIPHERAL** globally.
Not staging.
**Step 7.2**
Mature driver (2008+), still getting probe/UAF/of_node fixes. This is a
late bug in old code, not a new-subsystem feature.
Record: `[usb/musb] [IMPORTANT for AM335x/DSPS host] [mature]`
---
## Phase 8: Impact and risk
**Step 8.1**
Affected: MUSB **host** on platforms with `clear_ep_rxintr`
(DSPS/AM335x). Rate-limit also helps any MUSB host that hits three-
strikes. Trigger is USB Ethernet (and similar always-IN gadgets) unplug.
Record: `[driver/platform-specific: USB_MUSB_DSPS / AM335x; logging on
all musb host]`
**Step 8.2**
Trigger: unplug busy USB-Ethernet while IN URBs are posted — realistic.
Timing-dependent vs disconnect. Unprivileged user can do it physically.
Record: `[common unplug of busy NIC] [not a theoretical race only]
[physical unplug]`
**Step 8.3**
Failure: IRQ retrigger → printk flood + **host port unusable until
reboot**. Same family previously described as **system freeze**
(`dbac5d07d13e3`: hub never reports disconnect because RX ISR never
yields).
Record: `[hang / wedged USB host (+ log flood)] [HIGH, hang-class]`
**Step 8.4**
Benefit: high for AM335x/BeagleBone/industrial Sitara USB hosts.
Risk: very low (13 lines, existing no-op-safe hook, error path only).
Record: `[high benefit / very low risk]`
---
## Phase 9: Synthesis
**Step 9.1 — Evidence**
FOR:
- Real, user-visible hang: USB host stuck until reboot after unplugging
a busy USB Ethernet adapter.
- Interrupt storm on a path already known to freeze the machine
(2016/2020 musb host history).
- 13-line change in one function; reuses
`musb_platform_clear_ep_rxintr()` from 2017 (stable-bound then).
- No-op on glues without the ops hook.
- Merged by Greg KH; TI musb author was CCd.
- Prerequisites exist in this tree; no series dependency.
AGAINST:
- No `Reported-by` / public repro beyond the commit text.
- Affects DSPS/AM335x, not every USB user.
- Thread had no review replies (Greg merged v1 quickly).
UNRESOLVED:
- Could not fetch lore HTML (403); mbox had no replies.
- Could not search stable@ (403).
- Number of independent reporters unverified.
**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** — same ACK used in
`musb_cleanup_urb`; merged to mainline.
2. Real bug affecting users? **PASS** — unplug of busy USB Ethernet on
AM335x.
3. Important (crash/security/corruption/deadlock)? **PASS** — host port
hang until reboot / IRQ storm (HIGH).
4. Small and contained? **PASS** — +13/-1, one file, one branch.
5. No new features/APIs? **PASS** — uses existing ops hook; rate-limit
is logging only.
6. Applies to this tree? **PASS** — helper, DSPS callback, and hunk
context all present.
**Step 9.3 — Exceptions**
Not a device-ID/DT/build/docs exception. Closest: **hardware
workaround** (ACK platform IRQ the Mentor core leaves asserted).
Qualifies on ordinary bug-fix rules anyway.
**Step 9.4 — Decision**
This is a small, contained fix for a real disconnect-time IRQ storm that
can leave MUSB host unusable until reboot. It matches prior musb stable-
worthy storm fixes and uses an already-stable helper. Benefit outweighs
risk.
---
### What it solves for stable users
On TI DSPS/AM335x, yanking a busy USB Ethernet adapter can re-assert
endpoint RX IRQ while `musb_host_rx()` is still handling
`MUSB_RXCSR_H_ERROR`. The host keeps taking the same three-strikes
error, floods the log, and can leave the port dead until reboot.
Clearing the platform RX IRQ (and rate-limiting the error) breaks that
loop so disconnect can finish.
### Risk vs benefit
Very low risk (no-op elsewhere, existing helper, error path only) versus
high benefit (avoid a wedged USB host on a widely deployed SoC class).
### Concerns
None that block backport on this tree: no missing helpers, no series, no
API change. User-count beyond the author is unverified and does not
outweigh the hang severity plus the 2016/2020 precedent.
---
## Verification
- [Phase 1] Parsed tags: author SOB, msgid Link, Greg KH SOB; no
Fixes/Reported-by/Cc stable
- [Phase 1] `git log -1 --format` on `3a3a48487344a`: subject/body match
the candidate
- [Phase 2] Diff: +13/-1 in `musb_host_rx()` H_ERROR branch; `dev_err` →
`dev_err_ratelimited`; add `musb_platform_clear_ep_rxintr`
- [Phase 2] Read `musb_host.c` fault path: `status = -ESHUTDOWN` still
leads to flush/abort
- [Phase 2] Read `musb_core.h`: helper no-ops if ops hook NULL
- [Phase 2] Read `dsps_musb_clear_ep_rxintr`: writes endpoint bit to
`epintr_status`
- [Phase 2] Read `dsps_interrupt`: ACK all epintr then
`musb_interrupt()` → re-latch can retrigger
- [Phase 2] `musb_cleanup_urb` already calls the same helper “to avoid
bogus irqs”
- [Phase 3] `git blame -L 1770,1810 musb_host.c`: H_ERROR from 2008;
ESHUTDOWN from `1b967691ecdcf`
- [Phase 3] `git merge-base --is-ancestor` 6def85a396ce7 / c48400baa0215
/ 1b967691ecdcf / 3a3a48487344a vs HEAD: all true
- [Phase 3] `git show` 6def85a396ce7 / c48400baa0215: helper added for
bogus RX IRQ; Cc stable 4.1+
- [Phase 3] `git show` dbac5d07d13e3: earlier RX ISR flood freeze on
unplug
- [Phase 3] `git log 3a3a48487344a..HEAD -- musb_host.c`: empty (no
follow-up)
- [Phase 3] `git log --author='Lucas Martins Alves' -- drivers/usb/`:
only this commit
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 msgid, recipients Bin Liu + Greg KH
+ linux-usb, mbox has patch only
- [Phase 4] WebFetch lore/stable: 403 — UNVERIFIED HTML thread beyond
mbox
- [Phase 5] `git grep musb_host_rx`: callers `musb_interrupt`,
`musb_dma_completion`
- [Phase 5] `git grep clear_ep_rxintr`: only DSPS implements the ops
- [Phase 6] `git describe HEAD` / `make kernelversion`: v7.3-rc4 /
7.3.0-rc4
- [Phase 6] Pre-patch hunk via `git show
3a3a48487344a^:drivers/usb/musb/musb_host.c`: helper already existed
- [Phase 7] `Kconfig USB_MUSB_DSPS`: ARCH_OMAP2PLUS / TI DSPS
- [Phase 8] Failure mode from commit text + 2016 freeze commit: IRQ
storm / wedged host, HIGH
- UNVERIFIED: independent user count; stable@ mailing list discussion;
lore HTML (403)
The fix is small, uses an existing platform ACK, and prevents a real
USB-host hang on unplug.
**YES**
drivers/usb/musb/musb_host.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 6b4481a867c5e..40f51c388ef59 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1774,7 +1774,8 @@ void musb_host_rx(struct musb *musb, u8 epnum)
status = -EPIPE;
} else if (rx_csr & MUSB_RXCSR_H_ERROR) {
- dev_err(musb->controller, "ep%d RX three-strikes error", epnum);
+ dev_err_ratelimited(musb->controller,
+ "ep%d RX three-strikes error\n", epnum);
/*
* The three-strikes error could only happen when the USB
@@ -1788,6 +1789,17 @@ void musb_host_rx(struct musb *musb, u8 epnum)
rx_csr &= ~MUSB_RXCSR_H_ERROR;
musb_writew(epio, MUSB_RXCSR, rx_csr);
+ /*
+ * Unplugging a USB-Ethernet adapter while it is busy can make
+ * the controller keep re-asserting the three-strikes error for
+ * this endpoint before the disconnect is processed. That floods
+ * the log and can wedge the host port until reboot. Drop the
+ * stale pending RX interrupt on platforms that support it (e.g.
+ * AM335x/DSPS) to break the storm; the transfer is still
+ * aborted below via the fault path.
+ */
+ musb_platform_clear_ep_rxintr(musb, epnum);
+
} else if (rx_csr & MUSB_RXCSR_DATAERROR) {
if (USB_ENDPOINT_XFER_ISOC != qh->type) {
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.18-5.10] USB: misc: uss720: Fix leak in get_1284_register
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (7 preceding siblings ...)
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] usb: musb: host: clear stale RX interrupt on three-strikes error Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 14:34 ` sashiko-bot
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.15] net: usb: cdc_ether: add quirk for AMI BMC stale link events Sasha Levin
` (4 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: Michail Tatas, syzbot+56962eb32ba0136cd330, Greg Kroah-Hartman,
Sasha Levin, linux-usb, linux-kernel
From: Michail Tatas <michail.tatas@gmail.com>
[ Upstream commit 15902712b9e2a46649cba2f22002d05faec2a990 ]
if wait_for_completion_timeout() times out in get_1284_register()
then we do not destroy the reference acquired via kref_get() in
submit_async_request. Add the missing kref_put in the timeout path.
link: https://syzkaller.appspot.com/bug?extid=56962eb32ba0136cd330
Reported-by: syzbot+56962eb32ba0136cd330@syzkaller.appspotmail.com
Signed-off-by: Michail Tatas <michail.tatas@gmail.com>
Link: https://patch.msgid.link/allaDTUua9n5DQ0a@michalis-linux
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **7.3-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
candidate is a one-line kref leak fix in `drivers/usb/misc/uss720.c`.
The affected driver and timeout path are present in this tree, and the
bug dates to 2005.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[USB: misc: uss720]** **[Fix]** **[missing kref_put on
get_1284_register() timeout]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Reported-by:**
`syzbot+56962eb32ba0136cd330@syzkaller.appspotmail.com` (fuzzer-found,
C repro)
- **Link:**
`https://syzkaller.appspot.com/bug?extid=56962eb32ba0136cd330`
- **Link:** `https://patch.msgid.link/allaDTUua9n5DQ0a@michalis-linux`
- **Signed-off-by:** Michail Tatas (author)
- **Signed-off-by:** Greg Kroah-Hartman (USB maintainer; merged)
- **Fixes:** absent (expected)
- **Cc: stable:** absent (expected)
- Lowercase `link:` to syzbot (same report as Reported-by)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `wait_for_completion_timeout()` expiry in
`get_1284_register()` does not drop the `uss720_async_request` kref
taken by `submit_async_request()`.
- **Symptom:** kmemleak of the request, URB, control buffer, and `priv`
(syzbot title: “memory leak in uss720_probe”).
- **Root cause:** success/`!val` paths call `kref_put()`; the timeout
path only unlinks URBs and returns `-EIO`.
- **Version info in message:** none. Introduced with the async path in
2005 (`0f36163d3abef`).
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. Explicit leak fix, plus syzbot/kmemleak evidence.
===============================================================
PHASE 2: DIFF ANALYSIS
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/usb/misc/uss720.c` — **+1 / -0**
- Function: `get_1284_register()`
- Scope: single-file, one-line, timeout/error path only
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** timeout prints a warning,
`kill_all_async_requests_priv(priv)` (`usb_unlink_urb` only), `return
-EIO`. Caller kref leaked.
- **After:** same unlink, then `kref_put(&rq->ref_count, destroy_async)`
matching the completion path.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **Reference-count / resource leak**
- `submit_async_request()`: `kref_init` (caller) + `kref_get`
(URB/`async_complete`).
- `async_complete()` puts the URB ref.
- Timeout never puts the caller ref, so `destroy_async()` never runs:
URB, `rq`, `rq->dr`, and a `priv` kref leak.
- Syzbot objects match: `uss720_probe` priv, `submit_async_request`
`rq`, `usb_alloc_urb`, `rq->dr`.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct; same `kref_put` as the success and `!val`
paths. Unlink does not drop the caller ref. Timeout means `complete()`
has not run, so the extra put cannot free `rq` while the URB still holds
a ref. No API/locking/structure changes. Regression risk: very low.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record: Timeout path from **`0f36163d3abef`** (2005-09-12, “usb: fix
uss720 schedule with interrupts off”). Extra `kref_get` before
`usb_submit_urb` from **`adaa3c6342b249`** (2011, “USB: uss720 fixup
refcount position”). Bug is old, not 7.3-only.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Buggy code is the 2005 timeout path; that
commit is in this tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Nearby uss720 work is probe refcount/unregister
(`45dba8011efac`, `b4ecbdc4f8830`), not this timeout put. Standalone,
not `N/M`. After this commit on this file: none that revert or replace
it.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Only this uss720 commit from Michail Tatas in `drivers/usb/`.
Merged by Greg KH (USB maintainer).
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. Uses existing `kref_put` / `destroy_async`. Applies as a
one-line add.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 15902712b9e2a`:
https://patch.msgid.link/allaDTUua9n5DQ0a@michalis-linux
- `b4 dig -a`: earlier “v1” by WenTao Liang (2026-06-11) same leak;
syzbot forwards of this patch and a competing `usb_kill_urb` patch
(Deepanshu Kartikey).
- Direct lore HTTP was 403; thread recovered with `b4 dig -m`.
- David Laight **questioned** the `usb_kill_urb` + unlock-while-
iterating alternative; that patch was not merged. This one-line
`kref_put` was.
- Syzbot `#syz test` of this patch: **OK**.
- No NAK on the merged patch. No reviewer `Cc: stable` in the saved mbox
(syzbot thread, not Greg’s apply mail).
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc included `gregkh@linuxfoundation.org`,
`oneukum@suse.com`, `linux-usb@vger.kernel.org`. Merged by Greg KH.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: https://syzkaller.appspot.com/bug?extid=56962eb32ba0136cd330
- Title: memory leak in uss720_probe; **prio:high**; C and syz repros.
- Path: `uss720_probe` → `get_1284_register` → `submit_async_request`.
- Assessment: DoS via peripheral USB, not remote/unprivileged.
- Later dashboard crashes (Aug 2026) are on **7.2-rc** SHAs that are
**not** ancestors of this fix (`d58772d8520c`, `0d8395707651`). They
do not show the fix failing on 7.3.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone. Competing `usb_kill_urb` rewrite not required and
not in this tree (`kill_all_async_requests_priv` still uses
`usb_unlink_urb`).
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable WebFetch returned 403. **UNVERIFIED**
whether stable@ discussed it. No local `git log --grep` hit except this
commit itself.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: KEY FUNCTIONS**
Record: `get_1284_register` (modified). Related: `submit_async_request`,
`destroy_async`, `async_complete`, `kill_all_async_requests_priv`,
`uss720_probe`.
**Step 5.2: TRACE CALLERS**
Record: `get_1284_register()` from `change_mode()`, parport
read/status/control/EPP/ECP helpers, and **`uss720_probe()`** (line 734,
Belkin/incompatible-cable check). Probe is a USB enumeration path;
syzbot reaches it with a matching fake device that does not complete the
control URB in 1s (`HZ`).
**Step 5.3: TRACE CALLEES**
Record: `submit_async_request` (alloc + URB submit),
`wait_for_completion_timeout`, `kill_all_async_requests_priv`,
`kref_put` → `destroy_async` (`usb_free_urb`, `kfree(dr)`, list_del,
`kfree(rq)`, `kref_put(priv)`).
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: USB device add → `uss720_probe` → `get_1284_register(..., ®,
GFP_KERNEL)` with non-NULL `val` (so it waits). Unprivileged USB
gadget/dummy matching IDs in `uss720_table[]` can trigger it (syzbot).
Real cables that NACK/timeout on that register read take the same path.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `set_1284_register()` fire-and-forget puts immediately (no
wait). Only `get_1284_register()` waits and had an unbalanced timeout
path. No sibling copy of this bug.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Tree: **Linux 7.3.0-rc4** (mainline-lineage checkout, detached HEAD).
- Driver and `get_1284_register()` timeout path **exist**.
- Bug introduced **2005**, before this branch. Not a post-7.3 bug.
- Working copy already has the `kref_put` (commit `15902712b9e2a` is an
ancestor of HEAD; first contained in **v7.3-rc1**). That is the
original landing, not a reason to treat the leak as non-bugs.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean one-line add against the pre-fix function (parent blob
matches the candidate diff). No refactor of this path in 7.3. **Clean
apply.**
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This is the fix for that syzbot leak. No alternate timeout-path
put. Probe unregister fixes are separate.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **USB misc / USS720 USB-to-parallel** (`CONFIG_USB_USS720`,
depends on `PARPORT`). **PERIPHERAL** hardware, but probe is a standard
USB path and is fuzzable.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: File is old and lightly touched; USB core is actively
maintained. Greg KH merged the fix.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific / config-specific** (`CONFIG_USB_USS720`).
Anyone probing a matching USB ID (real USS720/Belkin-class cables, or a
malicious/fuzzed USB device).
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Control URB for register 0 does not complete within 1 second
during probe (or later parport ops that pass a non-NULL `val`). Syzbot
has a **C repro**. Not a theoretical race. Unprivileged local USB attach
can trigger it; not a remote syscall by itself.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Memory leak** of priv + async request + URB + setup packet on
each failed/timed-out probe. Repeated plug or syzbot-style enumeration
can grow leaks. **HIGH** (DoS / kmemleak), not an oops by itself.
Syzbot: prio:high, DoS.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for this driver — stops a confirmed, reproducible
leak on probe timeout; one-line; syzbot-tested.
- **Risk:** very low — one matching `kref_put`, error path only.
- **Ratio:** strongly in favor of including the fix in this tree.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real kmemleak with syzbot C repro and prio:high
- Unbalanced kref on a path that already puts on success
- One line, obviously correct, maintainer-merged, syzbot test OK
- Bug present since 2005; driver exists in 7.3
- No prerequisites, no new API
AGAINST:
- Peripheral USB-parport driver, not core MM/VFS
- Timeout may be uncommon on healthy hardware
- Competing `usb_kill_urb` change was not taken (not a dependency)
UNRESOLVED:
- Stable@ lore search HTTP 403
- Full Greg apply-thread body not fetched (msgid.link 403); merge +
patch-id match via b4 is verified
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** (matches other puts; syzbot
OK; Greg merged)
2. Fixes a real bug that affects users? **PASS** (kmemleak, C repro,
probe of matching USB IDs)
3. Important issue? **PASS** (HIGH leak/DoS on USB probe; sanitizer-
class resource leak)
4. Small and contained? **PASS** (+1 line, one function)
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** (same file/function; clean one-
line)
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not an ID/quirk/DT/build/docs exception. Ordinary small bug fix.
**Step 9.4: DECISION**
This tree has the USS720 driver and the 2005 timeout path. Missing
`kref_put` leaks the async request and keeps `priv` alive across a
failed probe. The fix is the same put used on every other
`get_1284_register()` exit. It meets stable rules for 7.3: real leak,
tiny, no deps.
===============================================================
What the commit solves for this tree
===============================================================
`get_1284_register()` waits up to 1s for a USB control URB. On timeout
it unlinked URBs but left the caller’s kref. `uss720_probe()` uses that
helper to reject incompatible cables; a non-responsive matching device
leaks `priv`, `uss720_async_request`, the URB, and the setup packet.
That is exactly the syzbot kmemleak. Adding `kref_put(...,
destroy_async)` closes the leak without changing success behavior.
Risk is negligible relative to stopping a reproducible probe leak.
===============================================================
Verification
===============================================================
- **[Phase 1]** Parsed subject/tags: syzbot Reported-by, two Links, Greg
KH SOB; no Fixes/Cc stable
- **[Phase 2]** Diff: +1 line `kref_put` in `get_1284_register()`
timeout path; parent blob confirmed missing put
- **[Phase 2]** kref model checked in current `uss720.c`: init + get in
`submit_async_request`, put in `async_complete` and all other
`get_1284_register` exits
- **[Phase 3]** `git blame`: timeout path `0f36163d3abef` (2005); extra
get `adaa3c6342b249` (2011)
- **[Phase 3]** No Fixes tag; `git log` on file shows standalone commit
- **[Phase 3]** `git merge-base --is-ancestor 15902712b9e2a HEAD` → yes
(original commit in this 7.3-rc4 checkout; first in v7.3-rc1)
- **[Phase 3]** Not in v7.1/v7.2 tags; post-fix syzbot crash SHAs are
7.2-rc and do not contain this commit
- **[Phase 4]** `b4 dig -c/-w/-a/-m`: msgid allaDTUua9n5DQ0a; CC Greg KH
+ linux-usb; competing usb_kill_urb patch questioned by David Laight
- **[Phase 4]** WebFetch syzbot: prio:high, C repro, leak stacks through
probe/get_1284_register
- **[Phase 4]** lore.kernel.org HTTP 403 — used b4 mbox instead; stable@
search UNVERIFIED
- **[Phase 5]** Callers via grep: probe line 734 and parport ops;
`CONFIG_USB_USS720` in `drivers/usb/misc/Kconfig`
- **[Phase 6]** `git describe HEAD` / Makefile: 7.3.0-rc4; driver
present; apply would be clean
- **[Phase 8]** Failure mode: kmemleak/DoS on USB probe timeout,
severity HIGH
- **UNVERIFIED:** stable@ mailing-list archive text (HTTP 403)
**YES**
drivers/usb/misc/uss720.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index 6ebcfaf525989..a405a36d8d42f 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -223,6 +223,7 @@ static int get_1284_register(struct parport *pp, unsigned char reg, unsigned cha
}
printk(KERN_WARNING "get_1284_register timeout\n");
kill_all_async_requests_priv(priv);
+ kref_put(&rq->ref_count, destroy_async);
return -EIO;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.18-5.15] net: usb: cdc_ether: add quirk for AMI BMC stale link events
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (8 preceding siblings ...)
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.10] USB: misc: uss720: Fix leak in get_1284_register Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 14:44 ` sashiko-bot
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] usb: misc: cypress_cy7c63: check result of IO Sasha Levin
` (3 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: Jinhui Guo, Jakub Kicinski, Sasha Levin, oliver, andrew+netdev,
davem, edumazet, pabeni, linux-usb, netdev, linux-kernel
From: Jinhui Guo <guojinhui.liam@bytedance.com>
[ Upstream commit 089ca284afb0f3b2870a5ce41524cbc91ed350d4 ]
On AMD Genoa/Turin platforms the BMC-provided USB-Ethernet gadget
(American Megatrends, VID 0x046b PID 0xffb0) intermittently fails to
respond to ARP after AC cold boot. usbmon captures a stale
NETWORK_CONNECTION(off) immediately followed by
NETWORK_CONNECTION(on) on the interrupt endpoint (~130us apart)
after enumeration. Because alloc_netdev() leaves
__LINK_STATE_NOCARRIER cleared, netif_carrier_ok() returns true
when the spurious OFF arrives, so usbnet_cdc_status() cannot
recognise it as redundant and schedules EVENT_LINK_CHANGE.
__handle_link_change() then calls unlink_urbs(), killing ~60 rx
URBs whose payload has already been DMA'd into memory — xHCI trace
confirms them completing as -ECONNRESET with non-zero residual
length. rx_complete() drops these unconditionally. The following
ON restores the carrier and re-submits URBs, but the ARP reply is
already lost; the interface looks "up but silent" until ifdown/ifup.
Fix this by adding a device-specific quirk with FLAG_LINK_INTR set,
which makes usbnet_probe() call netif_carrier_off() after bind.
With initial carrier == OFF, usbnet_cdc_status() recognises the
spurious OFF as matching the current state and drops it; the
subsequent ON is the first real event and brings the link up
cleanly without ever tearing down the rx queue. The scheduled
link-change kevent is harmless because EVENT_DEV_OPEN is not yet
set at probe time.
This is applied as a device-specific quirk rather than a change to
the shared cdc_info driver_info because some CDC devices never
send NETWORK_CONNECTION notifications; forcing carrier off for
them would leave the link permanently DOWN. Restricting the change
to this VID/PID keeps that class of device untouched.
Tested on Genoa and Turin across 100+ AC cold boot cycles; ping
first-packet success rate went from intermittent to 100%.
Signed-off-by: Jinhui Guo <guojinhui.liam@bytedance.com>
Link: https://patch.msgid.link/20260730051341.24930-1-guojinhui.liam@bytedance.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Local tree is **6.12.111**. The buggy `cdc_ether` path is present here,
and this is a device-specific hardware quirk for an existing driver.
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1 PARSE THE SUBJECT LINE**
- Record: `[net: usb: cdc_ether] [add quirk] [AMI BMC VID 0x046b PID
0xffb0 stale NETWORK_CONNECTION events]`
**Step 1.2 PARSE ALL COMMIT MESSAGE TAGS**
- Record:
- `Signed-off-by: Jinhui Guo <guojinhui.liam@bytedance.com>` (author)
- `Link: https://patch.msgid.link/20260730051341.24930-1-
guojinhui.liam@bytedance.com`
- `Signed-off-by: Jakub Kicinski <kuba@kernel.org>` (netdev maintainer
applied it)
- No `Fixes:`, no `Cc: stable`, no `Reported-by`, no `Reviewed-by`, no
`Acked-by`, no `Tested-by` (absence of Fixes/Cc-stable is expected
for this review)
- Ignore pipeline `Signed-off-by: Sasha Levin` / `[Upstream commit
...]` if present
**Step 1.3 ANALYZE THE COMMIT BODY TEXT**
- Record:
- **Bug:** AMI BMC USB-Ethernet gadget on AMD Genoa/Turin (VID
`0x046b` PID `0xffb0`) intermittently fails ARP after AC cold boot.
- **Symptom:** Interface looks “up but silent” until `ifdown`/`ifup`.
- **Root cause:** After enumeration, firmware emits stale
`NETWORK_CONNECTION(off)` then `(on)` ~130µs apart. `alloc_netdev()`
leaves `__LINK_STATE_NOCARRIER` clear, so `netif_carrier_ok()` is
true; `usbnet_cdc_status()` treats OFF as a real transition,
schedules `EVENT_LINK_CHANGE`, and `__handle_link_change()`
`unlink_urbs()`s ~60 RX URBs. Completions come back as `-ECONNRESET`
with payload already DMA’d; `rx_complete()` drops them, including
the ARP reply.
- **Fix:** Device-specific `driver_info` with `FLAG_LINK_INTR` so
probe starts carrier OFF; redundant OFF is dropped; ON is the first
real event.
- **Why not global:** Some CDC devices never send
`NETWORK_CONNECTION`; forcing carrier off would leave them
permanently DOWN.
- **Testing:** 100+ AC cold boots on Genoa and Turin; first-packet
ping went from intermittent to 100%.
**Step 1.4 DETECT HIDDEN BUG FIXES**
- Record: Not hidden. This is an explicit hardware quirk for a real
connectivity failure.
## PHASE 2: DIFF ANALYSIS
**Step 2.1 INVENTORY THE CHANGES**
- Record:
- File: `drivers/net/usb/cdc_ether.c` only (~+16 lines, 0 removed)
- Adds `ami_bmc_info` (clone of `cdc_info` plus `FLAG_LINK_INTR`)
- Adds `USB_DEVICE_AND_INTERFACE_INFO(0x046b, 0xffb0, ...)` **before**
the generic CDC Ethernet catch-all
- Scope: single-file surgical quirk / device ID
**Step 2.2 UNDERSTAND THE CODE FLOW CHANGE**
- Record:
- **Before:** `046b:ffb0` matches `USB_INTERFACE_INFO(...,
USB_CDC_SUBCLASS_ETHERNET, ...)` → `cdc_info` (`FLAG_ETHER |
FLAG_POINTTOPOINT`, no `FLAG_LINK_INTR`). Carrier starts ON. Stale
OFF is a state change.
- **After:** Same bind/status/unbind/filter/power callbacks, plus
`FLAG_LINK_INTR`. `usbnet_probe()` calls `usbnet_link_change(dev, 0,
0)` → `netif_carrier_off()`. Stale OFF matches current state and is
ignored.
**Step 2.3 IDENTIFY THE BUG MECHANISM**
- Record: **Hardware workaround / logic correctness.** Stale firmware
notifications plus default carrier-ON cause a false link-down that
tears down RX URBs and drops in-flight frames (ARP).
**Step 2.4 ASSESS THE FIX QUALITY**
- Record: Minimal and obviously correct. `FLAG_LINK_INTR` is the
established usbnet pattern (asix, smsc, dm9601, cdc_ncm, …).
Restricted to one VID/PID. Regression risk is confined to `046b:ffb0`:
initial carrier is OFF until the first ON notification. Author
captured that ON always follows. No public API / data-structure
change.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1 BLAME THE CHANGED LINES**
- Record:
- Generic `cdc_info` has existed since early usbnet.
- `FLAG_LINK_INTR` added in `37e8273cd3059` (“usbnet: Set link down
initially for drivers that update link state”, 2009).
- Global FLAG on `cdc_ether` was removed in `71cc1fa9f2d71` (2010)
because CDC Ethernet notifications are optional; devices that never
send them stayed DOWN (`bugzilla.kernel.org/14791`). That is why
this must be device-specific.
- Carrier comparison in `usbnet_cdc_status()` added by `2d5c9dd2cde33`
(“net: usb: cdc_ether: don't spew notifications”, 2024-08-08). That
check is what lets a redundant OFF be dropped. Confirmed ancestor of
HEAD.
**Step 3.2 FOLLOW THE FIXES: TAG**
- Record: No `Fixes:` tag. Not applicable. The interaction is long-
standing default carrier-ON plus this BMC’s cold-boot notifications.
**Step 3.3 CHECK FILE HISTORY**
- Record: Recent `cdc_ether.c` history is other device IDs/quirks
(`zte_cdc_info`, Cinterion WWAN IDs, ThinkPad dock). Same pattern.
Standalone, not `patch X/Y`.
**Step 3.4 CHECK THE AUTHOR'S OTHER COMMITS**
- Record: `git log --author="Jinhui Guo" -- drivers/net/usb/` is empty
in this tree. Author is not the subsystem maintainer. Applied by Jakub
Kicinski. USB net maintainers were on To/Cc (Oliver Neukum, netdev).
**Step 3.5 CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
- Record: Can apply standalone. This tree already has:
- `FLAG_LINK_INTR` and `usbnet_probe()` handling
- `usbnet_cdc_status()` carrier filter (`2d5c9dd2cde33`)
- `usbnet_cdc_bind` / `unbind` / `usbnet_cdc_update_filter` /
`usbnet_manage_power`
- Products table with the same Cinterion-then-catch-all context
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1 FIND THE ORIGINAL PATCH DISCUSSION**
- Record:
- `b4 dig -c 46651395...` failed (SHA not in this repo).
- Lore WebFetch returned 403 (Anubis bot-check).
- Openwall/Patchew/LKML recovered the thread:
- https://lists.openwall.net/linux-kernel/2026/07/30/380 (v1,
standalone `[PATCH]`)
- https://lists.openwall.net/linux-kernel/2026/07/30/387 (author
follow-up with usbmon + xHCI evidence)
- Single revision. No v2/v3 found. No NAK found.
**Step 4.2 CHECK WHO REVIEWED THE PATCH**
- Record: To: Oliver Neukum, Andrew Lunn, David Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni. Cc: linux-usb, netdev. No `Reviewed-
by`/`Acked-by` in the recovered thread. Kicinski’s SOB is the apply
signal. Recipients were the right maintainers.
**Step 4.3 SEARCH FOR THE BUG REPORT**
- Record: No syzbot/bugzilla. Author is the reporter, with usbmon, CDC
status traces, and xHCI Stop Ring / `-ECONNRESET` givebacks. Failure
is AC-cold-boot only; DC reboot preserves gadget state. 100+ cycle
test.
**Step 4.4 CHECK FOR RELATED PATCHES AND SERIES**
- Record: Standalone 16-line patch. Author’s follow-up mentions not
changing shared `cdc_info` because a global `FLAG_LINK_INTR` attempt
was reverted (the 2010 revert `71cc1fa9f2d71` is in this tree; the
cited subject string does not match that commit — likely a
misremembered title — but the technical reason matches the 2010
revert).
**Step 4.5 CHECK STABLE MAILING LIST HISTORY**
- Record: lore.kernel.org/stable search was blocked (403). Openwall/web
search found no stable-specific NAK or “do not backport” discussion.
UNVERIFIED: whether anyone already requested this on stable@.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1 KEY FUNCTIONS**
- Record: No function bodies changed. New `ami_bmc_info`; new
`products[]` entry. Runtime path: `usbnet_probe()` → `FLAG_LINK_INTR`
→ `usbnet_link_change()`; status path remains `usbnet_cdc_status()`.
**Step 5.2 TRACE CALLERS**
- Record:
- `usbnet_probe` is `cdc_driver.probe` (USB enumeration).
- `usbnet_cdc_status` is the interrupt completion status callback.
- `usbnet_open` submits the interrupt URB and RX URBs
(`EVENT_DEV_OPEN`).
- `__handle_link_change` unlinks RX URBs if carrier is down **and**
`EVENT_DEV_OPEN` is set.
**Step 5.3 TRACE CALLEES**
- Record: `usbnet_link_change(dev, 0, 0)` synchronously
`netif_carrier_off()` then defers `EVENT_LINK_CHANGE`. Probe-time
kevent is a no-op because `EVENT_DEV_OPEN` is clear
(`__handle_link_change` returns immediately).
**Step 5.4 FOLLOW THE CALL CHAIN**
- Record: Reachable on any host with this BMC gadget and
`CONFIG_USB_NET_CDCETHER`. After boot, NetworkManager/`ip link set up`
opens the netdev; stale notifications then hit the status path.
Userspace-visible: lost ARP, silent interface.
**Step 5.5 SEARCH FOR SIMILAR PATTERNS**
- Record: Same file already has `zte_cdc_info` (device-specific
status/bind). Many usbnet drivers use `FLAG_LINK_INTR`.
`drivers/hid/hid-ids.h` defines `USB_VENDOR_ID_AMI 0x046b`;
`unusual_devs.h` has another AMI BMC ID `0x046b:0xff40`. Vendor ID is
consistent.
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1 DOES THE BUGGY CODE EXIST IN THIS TREE?**
- Record: Yes. Tree is `p-6.12-2-gd38d58a4e0359`, `make kernelversion` =
**6.12.111**.
- `cdc_info.flags = FLAG_ETHER | FLAG_POINTTOPOINT` (no
`FLAG_LINK_INTR`) at `cdc_ether.c:525`.
- Catch-all `USB_INTERFACE_INFO(USB_CLASS_COMM,
USB_CDC_SUBCLASS_ETHERNET, USB_CDC_PROTO_NONE)` at line 944.
- No `0x046b` / `0xffb0` / `ami_bmc` in this driver.
- `usbnet_cdc_status()` has the `netif_carrier_ok() !=
!!event->wValue` filter (`2d5c9dd2cde33`).
- `rx_complete()` drops `-ECONNRESET`/`-ESHUTDOWN` via `goto block`.
- `usbnet_probe()` applies `FLAG_LINK_INTR` after `register_netdev()`
(author said “before register_netdev”; in this tree it is after, but
`netif_carrier_off()` is still synchronous and status URBs start at
`usbnet_open`, so the quirk still works).
**Step 6.2 CHECK FOR BACKPORT COMPLICATIONS**
- Record: Clean apply expected. Surrounding `cdc_info` / Cinterion
`0x1e2d:0x0069` / generic catch-all match the patch context. Line
numbers differ slightly (6.12 vs later mainline); context is
identical.
**Step 6.3 CHECK IF RELATED FIXES ARE ALREADY HERE**
- Record: No AMI BMC quirk. `2d5c9dd2cde33` is the prerequisite filter,
already present. No alternate fix for this VID/PID.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1 IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
- Record: `drivers/net/usb/cdc_ether.c` — USB CDC Ethernet.
**IMPORTANT** for hosts using this gadget (AMD Genoa/Turin BMC USB
NIC), not core mm/VFS. Common on enterprise servers.
**Step 7.2 ASSESS SUBSYSTEM ACTIVITY**
- Record: Mature driver (2005). Ongoing ID/quirk additions. USB
NETWORKING DRIVERS in MAINTAINERS is “Odd Fixes”, `drivers/net/usb/`.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1 WHO IS AFFECTED**
- Record: Driver-specific / config-specific: `CONFIG_USB_NET_CDCETHER`
and AMI BMC gadget `046b:ffb0` (AMD Genoa/Turin and similar AMI BMC
platforms). Other CDC devices unchanged.
**Step 8.2 TRIGGER CONDITIONS**
- Record: AC cold boot; firmware queues stale OFF/ON; interface already
open so RX URBs exist. Intermittent race with ARP DMA vs Stop Ring.
Unprivileged users do not trigger it; it is a boot/enumeration path.
Common on affected servers that use the BMC USB NIC.
**Step 8.3 FAILURE MODE SEVERITY**
- Record: Lost RX frames (ARP) → interface up but no L2 connectivity
until ifdown/ifup or peer re-ARP. Not a panic. **HIGH** operational
impact on affected hardware (host-BMC or provisioning network dead
after cold boot).
**Step 8.4 RISK-BENEFIT RATIO**
- Record:
- **Benefit:** High for AMI BMC USB-Ethernet users on 6.12; fixes a
real post-boot connectivity failure.
- **Risk:** Very low — 16 lines, one VID/PID, existing flag, no
locking/API change.
- **Ratio:** Strongly favors backport.
## PHASE 9: FINAL SYNTHESIS
**Step 9.1 COMPILE THE EVIDENCE**
FOR:
- Real, reproduced connectivity bug with usbmon/xHCI evidence
- Hardware quirk + device ID on an existing driver (stable exception)
- 16-line contained change
- `FLAG_LINK_INTR` is a long-standing, well-understood usbnet mechanism
- Does not change generic `cdc_info` (avoids the 2010 permanent-DOWN
regression)
- Prerequisite carrier filter already in 6.12
- Tested 100+ AC cold boots; netdev maintainer applied it
- Affects enterprise AMD server BMC networking
AGAINST:
- Not a crash/security issue
- No `Reviewed-by` in recovered mail (Kicinski still applied it)
- Affects one VID/PID, not all users
- Author slightly misstated probe vs `register_netdev` order (does not
break the fix here)
UNRESOLVED:
- Could not use `b4 dig` (SHA absent) or lore.kernel.org (403)
- Could not confirm stable@ discussion
- Author’s other `71cc1fa9f2d7` subject string does not match this
tree’s commit of that SHA (2010 revert) — rationale still holds
**Step 9.2 STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — standard flag, 100+ boot
test, maintainer SOB
2. Fixes a real bug that affects users? **PASS** — silent NIC after AC
cold boot
3. Important issue? **PASS** — connectivity loss / dropped RX (HIGH, not
panic)
4. Small and contained? **PASS** — +16 lines, one file, one VID/PID
5. No new features or APIs? **PASS** — quirk/ID only
6. Can apply to the local tree? **PASS** — code and prerequisites
present; clean context
**Step 9.3 EXCEPTION CATEGORIES**
- Record: **Hardware quirk/workaround** and **device ID addition to an
existing driver**.
**Step 9.4 DECISION**
This should be backported to this 6.12 tree. It is a small, device-
scoped quirk that fixes a real post-boot network failure on AMI BMC USB-
Ethernet, using infrastructure that already exists here.
### What the commit solves
On AMD Genoa/Turin, the AMI BMC USB CDC gadget emits a stale link-OFF
then link-ON right after cold-boot enumeration. Because CDC Ethernet
starts with carrier on, that OFF is treated as a real link-down, RX URBs
are unlinked, and in-flight ARP is dropped. The interface stays up but
silent. Starting carrier off for this VID/PID only makes the stale OFF a
no-op.
### Risk vs benefit
Benefit is restoring first-packet connectivity after AC cold boot on
affected servers. Risk is limited to `046b:ffb0` and is the intended
`FLAG_LINK_INTR` behavior.
### Concerns
None material for 6.12. Apply should be clean. Do not change generic
`cdc_info`.
### Verification
- [Phase 1] Parsed tags: author SOB, msgid Link, Kicinski SOB; no
Fixes/Cc-stable/Reported-by
- [Phase 2] Diff: +16 lines in `cdc_ether.c`; `ami_bmc_info` with
`FLAG_LINK_INTR`; ID `0x046b:0xffb0` before catch-all
- [Phase 3] `git describe HEAD` / `make kernelversion`:
`p-6.12-2-gd38d58a4e0359`, **6.12.111**
- [Phase 3] `git merge-base --is-ancestor 46651395… HEAD`: commit not in
this repo
- [Phase 3] `git merge-base --is-ancestor 2d5c9dd2cde33 HEAD`: yes
(carrier filter present)
- [Phase 3] `git show 71cc1fa9f2d71`: 2010 partial revert removing
`FLAG_LINK_INTR` from `cdc_info`
- [Phase 3] `git blame` `usbnet_cdc_status`: carrier check from
`2d5c9dd2cde33`
- [Phase 3] `git log --oneline -20 -- drivers/net/usb/cdc_ether.c`:
ID/quirk history; no AMI entry
- [Phase 3] `git log --author="Jinhui Guo" -- drivers/net/usb/`: empty
in this tree
- [Phase 4] `b4 dig -c <sha>`: no match (SHA not local)
- [Phase 4] lore WebFetch: 403; used Openwall + Patchew instead
- [Phase 4] Author follow-up: usbmon OFF/ON ~130µs, xHCI `-ECONNRESET`
length 1514, AC vs DC explanation
- [Phase 4] No NAK found; no v2 found
- [Phase 5] `usbnet_probe` FLAG_LINK_INTR →
`usbnet_link_change(dev,0,0)` after `register_netdev` (lines
1861–1877)
- [Phase 5] `__handle_link_change` returns if `!EVENT_DEV_OPEN`; else
`unlink_urbs` when carrier down
- [Phase 5] `rx_complete` `-ECONNRESET`/`-ESHUTDOWN` → `goto block`
- [Phase 5] `usbnet_open` starts interrupt URB then sets
`EVENT_DEV_OPEN`
- [Phase 6] No `ami_bmc`/`0x046b`/`0xffb0` in `cdc_ether.c`; catch-all
still binds this device to `cdc_info`
- [Phase 6] `FLAG_LINK_INTR` defined in `include/linux/usb/usbnet.h:122`
- [Phase 6] `USB_DEVICE_AND_INTERFACE_INFO` exists in
`include/linux/usb.h`
- [Phase 6] `CONFIG_USB_NET_CDCETHER` builds `cdc_ether.o`
- [Phase 7] HID confirms `USB_VENDOR_ID_AMI 0x046b`
- [Phase 8] Failure mode: dropped RX/ARP, silent interface — HIGH for
this hardware
- UNVERIFIED: stable@ discussion (lore 403); exact mainline SHA not in
this checkout
The patch is a textbook stable quirk: small, obviously correct, already
tested, and the buggy code is in this tree.
**YES**
drivers/net/usb/cdc_ether.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/net/usb/cdc_ether.c b/drivers/net/usb/cdc_ether.c
index a032c1ded4063..05e625a461b49 100644
--- a/drivers/net/usb/cdc_ether.c
+++ b/drivers/net/usb/cdc_ether.c
@@ -532,6 +532,16 @@ static const struct driver_info cdc_info = {
.manage_power = usbnet_manage_power,
};
+static const struct driver_info ami_bmc_info = {
+ .description = "AMI BMC USB Ethernet",
+ .flags = FLAG_ETHER | FLAG_POINTTOPOINT | FLAG_LINK_INTR,
+ .bind = usbnet_cdc_bind,
+ .unbind = usbnet_cdc_unbind,
+ .status = usbnet_cdc_status,
+ .set_rx_mode = usbnet_cdc_update_filter,
+ .manage_power = usbnet_manage_power,
+};
+
static const struct driver_info zte_cdc_info = {
.description = "ZTE CDC Ethernet Device",
.flags = FLAG_ETHER | FLAG_POINTTOPOINT,
@@ -940,6 +950,12 @@ static const struct usb_device_id products[] = {
USB_CDC_SUBCLASS_ETHERNET,
USB_CDC_PROTO_NONE),
.driver_info = (unsigned long)&wwan_info,
+}, {
+ /* AMI BMC gadget */
+ USB_DEVICE_AND_INTERFACE_INFO(0x046b, 0xffb0, USB_CLASS_COMM,
+ USB_CDC_SUBCLASS_ETHERNET,
+ USB_CDC_PROTO_NONE),
+ .driver_info = (unsigned long)&ami_bmc_info,
}, {
USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_ETHERNET,
USB_CDC_PROTO_NONE),
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.18-5.10] usb: misc: cypress_cy7c63: check result of IO
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (9 preceding siblings ...)
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.15] net: usb: cdc_ether: add quirk for AMI BMC stale link events Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 14:59 ` sashiko-bot
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] USB: gadget: inode: fix mm lifetime handling Sasha Levin
` (2 subsequent siblings)
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Oliver Neukum, Greg Kroah-Hartman, Sasha Levin, linux-usb,
linux-kernel
From: Oliver Neukum <oneukum@suse.com>
[ Upstream commit 0673b919e0933541ae56bb3f0253853f696d3773 ]
If the device returns a bogus short read, treat it as EIO.
Actually check for IO errors.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
Link: https://patch.msgid.link/20260710095523.1646308-1-oneukum@suse.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is mainline **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`).
The Cypress driver and the incomplete short-transfer check are present
here; the follow-up I/O-error handling is a real, small correctness fix.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
Step 1.1: PARSE THE SUBJECT LINE
Record: **[usb/misc/cypress_cy7c63] [check] check result of IO**
- Subsystem: `usb: misc: cypress_cy7c63`
- Verb: “check” — correctness / missing return-value handling, not a
refactor
Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
Record:
- `Signed-off-by: Oliver Neukum <oneukum@suse.com>` (author; known USB
developer)
- `Link:
https://patch.msgid.link/20260710095523.1646308-1-oneukum@suse.com`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (USB
maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence of the last two is expected for this
review)
- Not syzbot, not a user crash report
Step 1.3: ANALYZE THE COMMIT BODY TEXT
Record:
- Bug: a short USB control read is not treated as failure; I/O errors
are not checked on the sysfs read path
- Symptom: userspace can see a successful sysfs write, or a stale port
value on sysfs read, after a failed/short transfer
- Root cause: `usb_control_msg()` may return `0`/`1` (short success) or
a negative errno; `vendor_command()` used to return those short
lengths as success, and `read_port()` never tested the result
- No version range given
Step 1.4: DETECT HIDDEN BUG FIXES
Record: Not hidden. The subject and body state the bug. It completes the
earlier short-transfer check (`49cd2f4d747ee`, which *was* `CC:
stable`).
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
Step 2.1: INVENTORY THE CHANGES
Record:
- File: `drivers/usb/misc/cypress_cy7c63.c` — **+6 / −2** (8 lines in
the diff)
- Functions: `vendor_command()`, `read_port()`
- Scope: single-file surgical fix
Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
Record:
1. `vendor_command()`: after `usb_control_msg()`, `if (retval < 2) goto
err_buf` already skipped storing `iobuf[1]`, but still returned `0`
or `1`. Now a non-negative short result becomes `-EIO`.
2. `read_port()`: used to `sprintf()` `cyp->port[port_num]` even when
`vendor_command()` failed. Now `result < 0` is returned to sysfs.
`write_port()` already did `return result < 0 ? result : count`, so the
`-EIO` conversion is what makes short writes fail instead of reporting
`count`.
Step 2.3: IDENTIFY THE BUG MECHANISM
Record:
- Category: **logic / correctness** (USB I/O result handling; short
transfer → `-EIO`)
- Mechanism: `usb_control_msg()` returns transferred byte count on
success (kernel-doc in `drivers/usb/core/message.c`). A 0- or 1-byte
IN was treated as success. `read_port()` ignored errors and returned
the last cached port byte (or 0).
Step 2.4: ASSESS THE FIX QUALITY
Record:
- Obviously correct and the usual USB pattern (same shape as other
short-control-msg fixes)
- Minimal; no API/lock/structure changes
- Regression risk: **very low** (error path only; successful ≥2-byte
transfers unchanged)
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
Step 3.1: BLAME THE CHANGED LINES
Record:
- Driver added **2006-06-22**, `9189bfc2df0fd` (Oliver Bock)
- Short-transfer skip: **2024-09-12**, `49cd2f4d747ee` (Oliver Neukum),
contained in `v6.12-rc1`, **`CC: stable@vger.kernel.org`**
- `read_port()` never checked `result` from 2006 until this commit
- This follow-up: `0673b919e0933` (2026-07-10 / applied 2026-07-17)
Step 3.2: FOLLOW THE FIXES: TAG
Record: No `Fixes:` tag. Related buggy/incomplete commit is
`49cd2f4d747ee`, which **is** an ancestor of this HEAD (`git merge-base
--is-ancestor` exit 0). That patch stopped using a short buffer but
still returned a positive short length.
Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
Record: Recent file history is this I/O-check pair plus unrelated
alloc/ref cleanups. Standalone follow-up to `49cd2f4d747ee`, not patch
2/N of a larger series. No later commits on this file after
`0673b919e0933`.
Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Record: Oliver Neukum is a regular USB contributor; he wrote both
Cypress I/O checks. Other `drivers/usb/misc/` work is race/error
handling in iowarrior/yurex/chaoskey. USB subsystem maintainer who
applied this is Greg Kroah-Hartman.
Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Record: Depends on the `if (retval < 2) goto err_buf` block from
`49cd2f4d747ee`. That commit is in this tree. The patch applies as a
small edit of that check plus two lines in `read_port()`. Standalone on
this tree.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Record:
- `b4 dig -c 0673b919e0933` →
https://patch.msgid.link/20260710095523.1646308-1-oneukum@suse.com
- `b4 dig -a`: single **v1**, 2026-07-10; no v2/v3
- Saved mbox: **one message** (the patch). No replies, no NAK, no
reviewer `Cc: stable`
- Direct lore fetch returned HTTP 403; discussion is from the mbox
instead
Step 4.2: CHECK WHO REVIEWED THE PATCH
Record (`b4 dig -w`): To/Cc included Greg Kroah-Hartman, linux-usb,
Johan Hovold (`johan@kernel.org`), Kees Cook, original driver contacts.
No on-list review comments; Greg applied it.
Step 4.3: SEARCH FOR THE BUG REPORT
Record: No `Reported-by` / bugzilla / syzbot. Author-driven completeness
of the 2024 short-transfer fix.
Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Record: Same-week USB work by the author is unrelated (`USB: core:
ratelimit cabling message`). Related historical patch is `49cd2f4d747ee`
only.
Step 4.5: CHECK STABLE MAILING LIST HISTORY
Record: lore.kernel.org/stable search was not readable (403). Unverified
whether stable@ already discussed this SHA. The 2024 sibling *was*
explicitly nominated for stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
Step 5.1: IDENTIFY KEY FUNCTIONS
Record: `vendor_command()`, `read_port()` (callers of the latter:
`port0_show` / `port1_show`). `write_port()` is not edited but consumes
the new `-EIO`.
Step 5.2: TRACE CALLERS
Record:
- `vendor_command()` ← `write_port()` (sysfs store port0/port1),
`read_port()` (sysfs show)
- `read_port()` ← `port0_show`, `port1_show`
- Context: process / sysfs, not IRQ. Reachable from userspace on a bound
device.
Step 5.3: TRACE CALLEES
Record: `kzalloc`, `usb_rcvctrlpipe`, `usb_control_msg`, `kfree`;
`read_port` also `sprintf`. Control pipeline 0, vendor IN, 8-byte max.
Step 5.4: FOLLOW THE CALL CHAIN
Record: userspace `cat`/`echo` on sysfs `port0`/`port1` → show/store →
`vendor_command` → `usb_control_msg`. Yes, userspace-reachable. Requires
`CONFIG_USB_CYPRESS_CY7C63` and device `0x0a2c:0x0008`.
Step 5.5: SEARCH FOR SIMILAR PATTERNS
Record: Same short-read → `-EIO` pattern used elsewhere in USB (e.g.
historical `legousbtower` get-version check). This driver had the length
check without converting short success to an error.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Record: Yes. File and functions exist; driver since 2006. Incomplete
`retval < 2` path from `49cd2f4d747ee` is an ancestor. `git merge-base
--is-ancestor 0673b919e0933 HEAD` is true (the candidate is already in
this mainline checkout). Pre-fix behavior is confirmed via `git show
0673b919e0933^:drivers/usb/misc/cypress_cy7c63.c`.
Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Record: **Clean apply** on this tree (already applied). Context matches
a typical stable driver file aside from unrelated later `kzalloc_obj` in
probe.
Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
Record: `49cd2f4d747ee` is here; it is incomplete without this commit.
No other equivalent error-propagation fix.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
Record: **USB misc driver** (`drivers/usb/misc/cypress_cy7c63.c`),
`CONFIG_USB_CYPRESS_CY7C63`. Criticality: **PERIPHERAL** (AK Modul-Bus
CY7C63xxx I/O). USB core is not changed.
Step 7.2: ASSESS SUBSYSTEM ACTIVITY
Record: File is mature/low-churn (2006 driver; occasional USB-wide
cleanups). USB misc as a whole is actively maintained by Greg KH.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
Step 8.1: DETERMINE WHO IS AFFECTED
Record: **Driver-specific / config-specific** — only users of this
Cypress device with the driver enabled.
Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Record: Trigger is a failed or short vendor control IN while
reading/writing sysfs ports (disconnect, stall, timeout, non-compliant
firmware). Unprivileged users who can open the sysfs attrs of that
device can hit it. Not a boot-critical path.
Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Record:
- Failure: sysfs read returns stale/zero port data; sysfs write can
return success on a short IN
- Not oops/UAF/deadlock
- Severity: **MEDIUM** — wrong userspace-visible I/O / false success,
not kernel memory unsafety (buffer is `kzalloc`’d; `49cd2f4d747ee`
already skipped storing a short payload)
Step 8.4: CALCULATE RISK-BENEFIT RATIO
Record:
- Benefit: medium for this hardware (correct errors instead of silent
wrong GPIO-like values); low globally
- Risk: **very low** (6 lines, error paths only)
- Ratio: favorable — small completion of a prior stable-nominated fix
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
Step 9.1: COMPILE THE EVIDENCE
FOR:
- Real I/O-error bug: short `usb_control_msg` success (`0`/`1`) and
ignored negative results on read
- `write_port()` already treats any non-negative result as success —
without `-EIO`, a short transfer is a false success
- Completes `49cd2f4d747ee`, which went to stable with `CC: stable`
- Tiny (+6/−2), obviously correct, standard USB pattern
- Code has existed since 2006; prerequisite is in this tree
- Authored by a USB regular, applied by the USB maintainer
AGAINST:
- Niche driver, no user/syzbot report
- Not a crash, UAF, or security hole
- Author did not `Cc: stable` this time (not a veto per the rules above)
UNRESOLVED:
- lore HTTP 403; no on-list review text beyond the v1 patch
- Whether stable@ already queued this SHA
Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? **PASS** — matches `usb_control_msg`
semantics; applied by Greg KH
2. Fixes a real bug that affects users? **PASS** — sysfs can report
success/stale port data after failed USB I/O
3. Important issue? **PASS** (medium) — incorrect device I/O / false
success, in the “oh, that's not good” class; not a panic
4. Small and contained? **PASS** — 6 insertions, one file
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — prerequisite and target code
are here
Step 9.3: CHECK FOR EXCEPTION CATEGORIES
Record: None (not an ID/quirk/DT/build/docs exception). It is a normal
small driver bugfix.
Step 9.4: MAKE YOUR DECISION
This should be backported. The 2024 short-transfer patch left callers
treating a short IN as success and left `read_port()` returning cached
data on any error. That is user-visible wrong I/O on a long-lived
driver, the fix is six lines, and the only dependency is already in this
tree.
What it solves for stable users: failed or truncated vendor control
transfers no longer look like a successful port read/write.
Risk vs benefit: very low risk, clear correctness gain on the error
path.
Concerns: niche hardware; no crash. That does not outweigh completing an
incomplete stable fix with a trivial, standard `-EIO` conversion.
Verification:
- [Phase 1] Parsed tags: Oliver Neukum SOB, Greg KH SOB, msgid Link; no
Fixes/Reported-by/Cc:stable
- [Phase 1] Body: short read → EIO; actually check I/O errors
- [Phase 2] Diff: `vendor_command` maps `0 <= retval < 2` to `-EIO`;
`read_port` returns `result` if `< 0`
- [Phase 2] Confirmed `write_port` uses `result < 0 ? result : count`
(pre-patch parent file)
- [Phase 3] `git describe HEAD` / `make kernelversion`: **v7.3-rc4 /
7.3.0-rc4**
- [Phase 3] `git merge-base --is-ancestor 0673b919e0933 HEAD`: yes
(commit already in this tree)
- [Phase 3] blame: driver 2006 (`9189bfc2df0fd`); short check 2024
(`49cd2f4d747ee`, CC:stable, v6.12-rc1)
- [Phase 3] `git show 49cd2f4d747ee`: added `if (retval < 2) goto
err_buf` without `-EIO`
- [Phase 3] `git merge-base --is-ancestor 49cd2f4d747ee HEAD`: yes
- [Phase 3] `git log --oneline -20 --
drivers/usb/misc/cypress_cy7c63.c`: this commit is latest on the file
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 only; recipients gregkh, linux-usb,
johan, kees; mbox has no replies
- [Phase 4] WebFetch lore/msgid: 403 — discussion taken from b4 mbox
- [Phase 5] Callers: only `write_port`/`read_port` sysfs paths in this
file (`rg vendor_command`)
- [Phase 5] `usb_control_msg` kernel-doc: success = byte count, else
negative errno
- [Phase 6] Pre-image of `0673b919e0933^` matches the submitted hunk
context
- [Phase 7] Kconfig `USB_CYPRESS_CY7C63`; MAINTAINERS USB SUBSYSTEM →
Greg KH
- [Phase 8] Failure mode: stale sysfs port value / false write success,
not oops
- UNVERIFIED: stable@ mailing-list archive (HTTP 403)
**YES**
drivers/usb/misc/cypress_cy7c63.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/misc/cypress_cy7c63.c b/drivers/usb/misc/cypress_cy7c63.c
index 75f5a740cba39..e1a0c1ae93f2b 100644
--- a/drivers/usb/misc/cypress_cy7c63.c
+++ b/drivers/usb/misc/cypress_cy7c63.c
@@ -89,8 +89,11 @@ static int vendor_command(struct cypress *dev, unsigned char request,
address, data, iobuf, CYPRESS_MAX_REQSIZE,
USB_CTRL_GET_TIMEOUT);
/* we must not process garbage */
- if (retval < 2)
+ if (retval < 2) {
+ if (retval >= 0)
+ retval = -EIO;
goto err_buf;
+ }
/* store returned data (more READs to be added) */
switch (request) {
@@ -175,8 +178,9 @@ static ssize_t read_port(struct device *dev, struct device_attribute *attr,
dev_dbg(&cyp->udev->dev, "READ_PORT%d called\n", port_num);
result = vendor_command(cyp, CYPRESS_READ_PORT, read_id, 0);
-
dev_dbg(&cyp->udev->dev, "Result of vendor_command: %d\n\n", result);
+ if (result < 0)
+ return result;
return sprintf(buf, "%d", cyp->port[port_num]);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH AUTOSEL 6.18-5.10] USB: gadget: inode: fix mm lifetime handling
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (10 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] usb: misc: cypress_cy7c63: check result of IO Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 15:11 ` sashiko-bot
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18] xhci: include all root port children in recovery prevention on link error Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.1] usb: gadget: u_audio: clean up capture endpoint on feedback failure Sasha Levin
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Gabriel Prostitis, Alan Stern, Greg Kroah-Hartman, Sasha Levin,
linux-usb, linux-kernel
From: Gabriel Prostitis <prostitisgabriel@gmail.com>
[ Upstream commit a1c33c80e11ecd5c3310a38c4be324095dd85ac5 ]
priv stores a pointer to the submitting task's mm_struct,
but does not currently hold a reference to it while async
requests are pending.
This can result in a use-after-free if the task exits before
completion handling finishes.
Take a reference with mmgrab() when queuing the read request
and release it with mmdrop() on request completion.
Reported-by: Gabriel Prostitis <prostitisgabriel@gmail.com>
Signed-off-by: Gabriel Prostitis <prostitisgabriel@gmail.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://patch.msgid.link/20260601-mm-uaf-fix-v2-2-3c942a707bce@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[USB gadget / gadgetfs] [fix] [hold an `mm_struct` reference
for pending async endpoint I/O so completion cannot use a freed mm]**
- Subsystem prefix: `USB: gadget: inode:`
- Action verb: **fix**
- Intent: correct mm lifetime in the legacy gadgetfs AIO path
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Reported-by:** Gabriel Prostitis (author reported it)
- **Signed-off-by:** Gabriel Prostitis (author)
- **Acked-by:** Alan Stern (USB gadget reviewer)
- **Link:** https://patch.msgid.link/20260601-mm-uaf-
fix-v2-2-3c942a707bce@gmail.com
- **Signed-off-by:** Greg Kroah-Hartman (USB maintainer; merged)
- **Fixes:** none (expected for this review path)
- **Cc: stable:** none (expected)
- Ignored pipeline markers: none in the real commit (`a1c33c80e11ec`)
Notable: single reporter (the author), but **Acked-by Alan Stern** and
merge by Greg KH. Cover letter (v2 0/2) states KASAN confirmed the UAF.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `kiocb_priv` stores `current->mm` while async USB requests
are pending, with no `mm_count` pin.
- **Symptom / failure mode:** use-after-free if the submitting task
exits before completion handling. Cover letter (lore): KASAN UAF in
the workqueue path; possible **memory corruption in another process’s
address space** if the freed `mm_struct` is reused. Trigger: async
reads on OUT endpoints (io_uring called out for FunctionFS; gadgetfs
has the same `read_iter` AIO path).
- **Version info:** none in the commit; buggy `priv->mm = current->mm`
dates to 2013 (`a80bf61ef36da4`, contained in v3.10-rc1).
- **Root cause (author):** stored mm pointer without `mmgrab()`;
completion uses it via `kthread_use_mm()`.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** Subject and body are an explicit UAF/lifetime
fix. The extra `mmget_not_zero()` around `kthread_use_mm()` matches the
documented `mmgrab()` contract in `include/linux/sched/mm.h`.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/usb/gadget/legacy/inode.c` — **+13 / −4** (17 lines
changed)
- **Functions:** `ep_user_copy_worker()`, `ep_aio_complete()`,
`ep_aio()`
- **Scope:** single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
1. **`ep_aio()` (queue):** still stores `priv->mm = current->mm`; now
**`mmgrab(priv->mm)`**. Fail path after that **`mmdrop(priv->mm)`**.
2. **`ep_aio_complete()` (USB complete, writes / zero-length reads):**
completes immediately; now **`mmdrop(priv->mm)`** before freeing
`priv`.
3. **`ep_user_copy_worker()` (async read copy):** previously
`kthread_use_mm(mm)` on an unpinned pointer. Now `mmget_not_zero(mm)`
→ copy → `mmput` → always **`mmdrop(mm)`**. If the address space is
already gone, return `-EFAULT` instead of touching a dying mm.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** use-after-free / reference counting
(`mm_struct.mm_count` / `mm_users`)
- **Mechanism:** `priv->mm` was a raw pointer. `kthread_use_mm()` itself
does `mmgrab(mm)` (`kernel/kthread.c`), which is a UAF if `mm` is
already freed. Classic AIO `exit_aio()` does **not** cover io_uring.
Cover letter: process exit while a request is pending frees mm while
the worker still holds the pointer.
- **Fix:** pin with `mmgrab()` for the struct lifetime;
`mmget_not_zero()` before using the address space, as documented.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct and matches
`mmgrab()`/`mmget_not_zero()` docs. Refcounts are balanced: grab on
queue; drop on fail, immediate complete, and worker.
- **Minimal:** no unrelated edits.
- **Regression risk:** low. Worst case after process exit is `-EFAULT`
instead of a copy into a dead mm. No new API, no lock-order change.
`mmgrab(NULL)` is not a realistic syscall path
(`ep_read_iter`/`ep_write_iter` from userspace).
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Buggy store `priv->mm = current->mm` and worker: **`a80bf61ef36da4`**
(“gadget: remove only user of aio retry”, Zach Brown, **2013-05-07**,
**v3.10-rc1**).
- `kthread_use_mm`/`kthread_unuse_mm`: `f5678e7f2ac31c` (2020).
- Pattern has been in tree for many years, including this 7.3 checkout.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No `Fixes:` tag.** Not applicable. Introducer is
`a80bf61ef36da4`, present in this tree (blame on the parent of the fix).
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Related: **`5eb5c72c72fef`** “USB: gadget: ffs: fix mm lifetime
handling” (patch 1/2, parent of this commit). Different file
(`f_fs.c`); **not a code dependency**.
- After this commit on `inode.c`: only `dd0eed9e165b1` (unrelated ioctl
NULL deref).
- **Standalone** for gadgetfs.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Gabriel Prostitis has these two USB gadget mm-lifetime commits.
Not the subsystem maintainer. **Alan Stern Acked-by** on this inode
patch; Greg KH merged both.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Uses `mmgrab`/`mmdrop`/`mmget_not_zero`/`kthread_use_mm`, all
present here. No new helpers. Patch 1/2 is a sibling, not a
prerequisite. **Applies standalone.**
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig:** https://patch.msgid.link/20260601-mm-uaf-
fix-v2-2-3c942a707bce@gmail.com
- **Series:** v1 (2026-05-31), v2 (2026-06-01). v2 cover: “Fix swapped
lines in `inode.c`”; v2 is what was merged.
- Cover letter: UAF in FunctionFS **and** gadgetfs; `kthread_use_mm()`
in workqueue; KASAN; io_uring-style async read + exit; possible cross-
process corruption.
- **No NAKs.** Ping 2026-06-30; Greg KH replied that the merge window
delayed review, then merged 2026-07-10.
- **No explicit Cc: stable** in the thread (mbox).
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: **b4 dig -w** recipients: author, Greg KH, linux-usb, linux-
kernel, **Alan Stern**. Stern’s Acked-by is on the inode.c patch.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: Author is the reporter. Cover letter cites KASAN on the
workqueue completion path. Direct lore HTTP fetches returned **403**;
thread body recovered via **`b4 dig -m`**.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: **v2 1/2** `USB: gadget: ffs: fix mm lifetime handling`
(`5eb5c72c72fef`) is the FunctionFS analogue. Independent file. This 2/2
is self-contained for gadgetfs.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search returned **403**. **UNVERIFIED**
whether stable@ discussed this. No Cc: stable on the patch itself (not a
negative signal here).
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: KEY FUNCTIONS**
Record: `ep_aio()`, `ep_aio_complete()`, `ep_user_copy_worker()`.
(semcode MCP not available.)
**Step 5.2: TRACE CALLERS**
Record:
- `ep_aio()` ← `ep_read_iter()` / `ep_write_iter()` when
`!is_sync_kiocb(iocb)`
- Those are `ep_io_operations.read_iter` / `.write_iter` (gadgetfs
endpoint files)
- `ep_aio_complete()` ← USB gadget `usb_request.complete` (often
IRQ/softirq)
- `ep_user_copy_worker()` ← `schedule_work()` from that complete path
Reachable from **io_submit / io_uring** on gadgetfs endpoint fds, plus
USB completion.
**Step 5.3: TRACE CALLEES**
Record: `mmgrab`/`mmdrop`/`mmget_not_zero`/`mmput`,
`kthread_use_mm`/`kthread_unuse_mm`, `copy_to_iter`,
`usb_ep_queue`/`usb_ep_alloc_request`/`usb_ep_free_request`,
`ki_complete`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: userspace AIO/io_uring on gadgetfs ep file → `ep_aio` stores mm
→ USB complete → worker `kthread_use_mm` + `copy_to_iter`. **Userspace-
reachable** if gadgetfs is mounted and configured (typically privileged
USB gadget role).
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same bug in `drivers/usb/gadget/function/f_fs.c`
(`io_data->mm`), fixed by sibling `5eb5c72c72fef`. Systematic copy of
the 2013 “stash mm, copy in worker” pattern.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- **Tree:** `git describe HEAD` = **v7.3-rc4-27-gf0100363d8c37**; `make
kernelversion` = **7.3.0-rc4**
- File and AIO path exist. Pre-fix code is in `a1c33c80e11ec^`.
Introduced **v3.10** (`a80bf61ef36da4`). **Bug exists in this
lineage.**
- `git merge-base --is-ancestor a1c33c80e11ec HEAD` → yes (used only to
inspect the candidate, not as “already selected”).
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply on this tree.** Surrounding code matches the diff.
`mmgrab`/`mmdrop`/`mmget_not_zero`/`kthread_use_mm` are present.
`inode.c` does not include `linux/sched/mm.h` directly; `mmgrab` is
visible via existing includes (e.g. `pagemap.h` → `mmap_lock.h` →
`sched/mm.h`) in **this** tree.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No earlier alternate gadgetfs mm-lifetime fix. Sibling
FunctionFS fix is separate. No competing fix for this inode.c UAF.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM**
Record: **USB gadgetfs** (`drivers/usb/gadget/legacy/`).
**IMPORTANT/PERIPHERAL:** not core mm/VFS, but a supported (non-staging)
USB gadget userspace API. FunctionFS is more common; gadgetfs is still
shipped and has the same UAF.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: File is mature (code from 2005–2013 with occasional fixes). Low
churn. Long-lived bug, not a brand-new subsystem.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Record: **Config-specific / driver-specific:** `CONFIG_USB_GADGETFS`,
async I/O on gadgetfs endpoints, process exit before USB complete. Not
all machines, but real gadgetfs userspace drivers.
**Step 8.2: TRIGGER CONDITIONS**
Record: Async read (or write, for the pin) on a gadgetfs endpoint, then
the submitting task exits before `ep_aio_complete` / worker. Cover
letter: io_uring-style async + exit. Timing-dependent but realistic.
Typically needs gadgetfs access (usually privileged), **not** a random
unprivileged syscall on a default desktop.
**Step 8.3: FAILURE MODE SEVERITY**
Record: **Use-after-free of `mm_struct`** → KASAN crash or **cross-
process memory corruption**. **Severity: HIGH** (UAF / security-adjacent
corruption). Not a log-only issue.
**Step 8.4: RISK-BENEFIT RATIO**
Record:
- **Benefit: HIGH** for gadgetfs AIO users — stops UAF/corruption.
- **Risk: LOW** — 13 insertions, 4 deletions, refcount pin matching
documented API, Acked-by Stern.
- **Ratio:** benefit clearly outweighs risk.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real UAF of `mm_struct` when async gadgetfs I/O outlives the task
- Cover letter: KASAN; possible corruption of another process’s mm
- Small, contained, refcounts balanced
- Matches `mmgrab()` documentation
- Acked-by Alan Stern; merged by Greg KH
- Code and APIs exist in this v7.3-rc4 tree; bug dates to v3.10
- Standalone (patch 2/2 does not need 1/2)
**AGAINST:**
- Gadgetfs is a narrower, legacy userspace gadget API
- Trigger needs gadgetfs + async I/O + exit-before-complete
- No `Fixes:` / `Cc: stable` (not a negative for this process)
**UNRESOLVED:**
- lore.kernel.org HTTP 403; stable@ discussion not independently
confirmed beyond the mbox from `b4 dig`
- No public syzbot hash in the commit (KASAN is from the cover letter)
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — API-correct pin; v2 after a
v1 line-swap mistake; Stern ack
2. Fixes a real bug that affects users? **PASS** — UAF on gadgetfs async
completion
3. Important (crash, security, corruption, deadlock)? **PASS** — UAF /
possible cross-process corruption (**HIGH**)
4. Small and contained? **PASS** — one file, +13/−4
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — context and helpers exist
here
**Step 9.3: EXCEPTION CATEGORIES**
Record: none (not a device ID / quirk / DT / build / docs exception).
Qualifies as a normal important bugfix.
**Step 9.4: DECISION**
This is a small, reviewed UAF fix for code that exists in this tree.
Process exit during gadgetfs AIO can free `mm_struct` while
`ep_user_copy_worker()` still calls `kthread_use_mm()`. That is a real
stability/security bug, not cleanup. Risk of the pin itself is low.
### What the commit solves for this tree
`ep_aio()` saved `current->mm` without an `mm_count` reference.
Completion may run after the task is gone (especially io_uring, which is
not `exit_aio()`). The worker then used a dangling `mm_struct`. Pinning
with `mmgrab()` and using `mmget_not_zero()` before the copy closes that
window.
### Risk vs benefit
Benefit is preventing kernel UAF/corruption on a reachable gadgetfs AIO
path. Risk is a few refcount operations on a slow path. No structural or
API change.
### Concerns
Patch 2/2 of a series, but independent of FunctionFS. v1 had swapped
lines; merged v2 is the one to use. Direct lore HTTP was blocked;
discussion was recovered with `b4 dig -m`.
---
### Verification
- **[Phase 1]** `git log -1 a1c33c80e11ec`: subject/tags as above; no
Fixes/Cc:stable
- **[Phase 2]** `git show a1c33c80e11ec`: +13/−4 in `inode.c`; mmgrab on
queue; mmdrop on fail, immediate complete, and worker; mmget_not_zero
around copy
- **[Phase 2]** Read `ep_aio`/`ep_aio_complete`/`ep_user_copy_worker` in
this tree: refcounts balanced
- **[Phase 2]** `include/linux/sched/mm.h`: mmgrab docs require
mmget_not_zero before using the address space
- **[Phase 2]** `kernel/kthread.c` `kthread_use_mm()`: mmgrab on the
passed mm — UAF if mm already freed
- **[Phase 3]** `git blame` on parent: `priv->mm` from `a80bf61ef36da4`
(v3.10-rc1, 2013)
- **[Phase 3]** `git describe --contains a80bf61ef36da4` →
`v3.10-rc1~50^2~20`
- **[Phase 3]** Sibling `5eb5c72c72fef` is f_fs.c only; not required for
inode.c
- **[Phase 3]** `git log --author="Gabriel Prostitis" -- drivers/usb/`:
only these two mm-lifetime commits
- **[Phase 3]** `git merge-base --is-ancestor a1c33c80e11ec HEAD`: is
ancestor (inspection only)
- **[Phase 4]** `b4 dig -c a1c33c80e11ec`: v2 2/2 lore URL
- **[Phase 4]** `b4 dig -a`: v1 then v2; v2 cover notes inode.c swapped-
line fix
- **[Phase 4]** `b4 dig -w`: Greg KH, linux-usb, Alan Stern
- **[Phase 4]** `b4 dig -m /tmp/gadget-mm-uaf.mbox`: KASAN, io_uring
trigger, cross-process corruption; Stern ack; no NAK; Greg delay-then-
merge
- **[Phase 4]** WebFetch/curl lore: **403** — UNVERIFIED via HTTP; mbox
used instead
- **[Phase 4]** stable@ search: **403** — UNVERIFIED
- **[Phase 5]** grep: `ep_aio` only from `ep_read_iter`/`ep_write_iter`;
those are `ep_io_operations`
- **[Phase 5]** Same mm stash in `f_fs.c`, fixed separately
- **[Phase 6]** `git describe HEAD` / `make kernelversion`: **v7.3-rc4**
/ **7.3.0-rc4**
- **[Phase 6]** Buggy path present before `a1c33c80e11ec`; helpers
present
- **[Phase 7]** `USB_GADGETFS` in `drivers/usb/gadget/legacy/Kconfig`
- **[Phase 8]** Failure mode: mm UAF → oops or cross-process corruption;
HIGH
- **UNVERIFIED:** whether stable@ already discussed this (lore 403); no
syzbot ID in the commit
**YES**
drivers/usb/gadget/legacy/inode.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index 7c72267c0c53b..a46b6a6b1f836 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -473,11 +473,17 @@ static void ep_user_copy_worker(struct work_struct *work)
struct kiocb *iocb = priv->iocb;
size_t ret;
- kthread_use_mm(mm);
- ret = copy_to_iter(priv->buf, priv->actual, &priv->to);
- kthread_unuse_mm(mm);
- if (!ret)
+ if (mmget_not_zero(mm)) {
+ kthread_use_mm(mm);
+ ret = copy_to_iter(priv->buf, priv->actual, &priv->to);
+ kthread_unuse_mm(mm);
+ mmput(mm);
+ if (!ret)
+ ret = -EFAULT;
+ } else {
ret = -EFAULT;
+ }
+ mmdrop(mm);
/* completing the iocb can drop the ctx and mm, don't touch mm after */
iocb->ki_complete(iocb, ret);
@@ -503,6 +509,7 @@ static void ep_aio_complete(struct usb_ep *ep, struct usb_request *req)
* complete the aio request immediately.
*/
if (priv->to_free == NULL || unlikely(req->actual == 0)) {
+ mmdrop(priv->mm);
kfree(req->buf);
kfree(priv->to_free);
kfree(priv);
@@ -543,6 +550,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
priv->epdata = epdata;
priv->actual = 0;
priv->mm = current->mm; /* mm teardown waits for iocbs in exit_aio() */
+ mmgrab(priv->mm);
/* each kiocb is coupled to one usb_request, but we can't
* allocate or submit those if the host disconnected.
@@ -572,6 +580,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
fail:
spin_unlock_irq(&epdata->dev->lock);
+ mmdrop(priv->mm);
kfree(priv->to_free);
kfree(priv);
put_ep(epdata);
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.10] USB: gadget: inode: fix mm lifetime handling
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] USB: gadget: inode: fix mm lifetime handling Sasha Levin
@ 2026-10-19 15:11 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-19 15:11 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-usb
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: gadget: inode: mmdrop() called in atomic context in ep_aio_complete()
--
commit 1034aa9a2c06c8a1a74aa881eb54ff67ca5c0fee
Author: Gabriel Prostitis <prostitisgabriel@gmail.com>
USB: gadget: inode: fix mm lifetime handling
This commit fixes a use-after-free in gadgetfs async I/O handling by pinning
the submitting task's mm_struct with mmgrab() in ep_aio() and releasing it
with mmdrop() upon request completion. It also uses mmget_not_zero() in
ep_user_copy_worker() before switching to the address space.
Link: https://patch.msgid.link/20260601-mm-uaf-fix-v2-2-3c942a707bce@gmail.com
> diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> index 13c3da49348c5..7d201c065ae3b 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
[ ... ]
> @@ -503,6 +509,7 @@ static void ep_aio_complete(struct usb_ep *ep, struct usb_request *req)
> * complete the aio request immediately.
> */
> if (priv->to_free == NULL || unlikely(req->actual == 0)) {
> + mmdrop(priv->mm);
[Severity: High]
Is it safe to call mmdrop(priv->mm) directly in ep_aio_complete()?
ep_aio() pins current->mm with mmgrab(priv->mm) for all async requests. If
the submitting task exits before the USB request completes (for example,
with io_uring or a forked process where do_exit() -> exit_mm() -> mmput()
runs before completion), priv->mm can hold the final reference on
mm->mm_count.
When the request completes and takes the immediate completion branch
(priv->to_free == NULL || unlikely(req->actual == 0)) — which occurs for
async writes, single-buffer ITER_UBUF reads where dup_iter() returns NULL,
zero-length reads, errors, disconnects, or cancellations via
ep_aio_cancel() -> usb_ep_dequeue() — ep_aio_complete() runs as the
usb_request->complete callback with interrupts disabled and while holding
spin_lock(&epdata->dev->lock):
drivers/usb/gadget/legacy/inode.c:ep_aio_complete() {
spin_lock(&epdata->dev->lock);
priv->req = NULL;
priv->epdata = NULL;
if (priv->to_free == NULL || unlikely(req->actual == 0)) {
mmdrop(priv->mm);
}
If mmdrop(priv->mm) drops the last reference, it invokes __mmdrop() from
atomic, interrupts-disabled context:
kernel/fork.c:__mmdrop() {
...
cleanup_lazy_tlbs(mm);
WARN_ON_ONCE(mm == current->active_mm);
mm_free_pgd(mm);
...
}
This appears to cause a few issues:
1. On architectures with CONFIG_MMU_LAZY_TLB_SHOOTDOWN=y,
__mmdrop() -> cleanup_lazy_tlbs() -> on_each_cpu_mask() ->
smp_call_function_many_cond() waits synchronously for cross-CPU IPIs,
triggering lockdep_assert_irqs_enabled() and WARN_ON_ONCE(!in_task()) in
kernel/smp.c and potentially deadlocking with interrupts disabled.
2. On x86, __mmdrop() -> mm_free_pgd() -> pgd_free() -> pgd_dtor() acquires
spin_lock(&pgd_lock) without disabling interrupts:
arch/x86/mm/pgtable.c:pgd_dtor() {
spin_lock(&pgd_lock);
pgd_list_del(pgd);
spin_unlock(&pgd_lock);
}
As noted in free_signal_struct() in kernel/fork.c, __mmdrop() is not safe to
call from interrupt or softirq context on x86 because taking pgd_lock can
deadlock if the interrupted context already holds pgd_lock.
3. On CONFIG_PREEMPT_RT=y, __mmdrop() acquires sleeping spinlock_t locks
while interrupts are disabled and a spinlock is held.
Should ep_aio_complete() use mmdrop_async(priv->mm) here (or avoid grabbing
priv->mm when priv->to_free == NULL, since the worker is only scheduled when
priv->to_free != NULL)?
> kfree(req->buf);
> kfree(priv->to_free);
> kfree(priv);
[ ... ]
> @@ -543,6 +550,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
> priv->epdata = epdata;
> priv->actual = 0;
> priv->mm = current->mm; /* mm teardown waits for iocbs in exit_aio() */
> + mmgrab(priv->mm);
[ ... ]
> @@ -572,6 +580,7 @@ static ssize_t ep_aio(struct kiocb *iocb,
>
> fail:
> spin_unlock_irq(&epdata->dev->lock);
> + mmdrop(priv->mm);
> kfree(priv->to_free);
> kfree(priv);
> put_ep(epdata);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-360-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH AUTOSEL 6.18] xhci: include all root port children in recovery prevention on link error
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (11 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] USB: gadget: inode: fix mm lifetime handling Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 15:25 ` sashiko-bot
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.1] usb: gadget: u_audio: clean up capture endpoint on feedback failure Sasha Levin
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Mathias Nyman, Xu Rao, Greg Kroah-Hartman, Sasha Levin,
mathias.nyman, linux-usb, linux-kernel
From: Mathias Nyman <mathias.nyman@linux.intel.com>
[ Upstream commit e3d757dc9257f2638ca61eda4faef26c2daeea10 ]
Driver already prevents useless transfer retry and endpoint recovery
for devices directly connected to a root port with link errors.
These devices are either disconnecting or will be reset. Link is gone.
Move the flag indicating link error from the xhci device structure to
the root port strucure, allowing all child devices behind hubs to easily
check for root port link errors, avoiding useless transfer retries and
endpoint recovery.
This extends the previous endpoint recovery prevention in
commit b8c3b718087b ("usb: xhci: Don't try to recover an endpoint if port
is in error state.")
Only root port link errors can be detected early by xhci driver,
not link errors between external hubs and their children.
Tested-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Link: https://patch.msgid.link/20260806142113.2436238-4-mathias.nyman@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
- Record: `[xhci]` `[include]` extend existing root-port link-error
recovery skip from the directly attached device to every device under
that root port.
**Step 1.2: Tags**
- Record:
- `Tested-by: Xu Rao <raoxu@uniontech.com>`
- `Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>` (xhci
maintainer)
- `Link: https://patch.msgid.link/20260806142113.2436238-4-
mathias.nyman@linux.intel.com`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>`
- No `Fixes:` / `Cc: stable` (expected for this review)
- Pipeline `Signed-off-by: Sasha Levin` ignored
- Notable: maintainer-authored; tester is the person who hit the host-
teardown bug.
**Step 1.3: Body**
- Record:
- **Bug:** On a root port in SS.Inactive, xhci already skips URB
enqueue and endpoint reset for the device plugged into that port
(`VDEV_PORT_ERROR`). Devices behind hubs never get that flag, so
they still retry transfers and reset endpoints after the root-port
link is gone.
- **Symptom:** Useless recovery while the hub driver is still sorting
the port out. The existing comment on this path is “Can cause host
hang.” The 2019 precursor (`b8c3b718087b`) documented host-
controller stop after repeated endpoint resets on an inactive root
port.
- **Root cause:** The error flag lives on `xhci_virt_device`, and
`handle_port_status()` only sets it on `xhci->devs[port->slot_id]`
(the device directly on the root port).
- **Fix:** Move the flag to `xhci_port->link_inactive` so every child
that already has `vdev->rhub_port` can see the same root-port state.
- **Limit:** Only root-port link errors are visible this way, not hub-
to-child link errors.
**Step 1.4: Hidden bug fix?**
- Record: Not hidden. It is an explicit extension of a hang-prevention
check. Moving the flag off `vdev` is a small structural change with
standalone functional effect, not a pure cleanup.
## Phase 2: Diff analysis
**Step 2.1: Inventory**
- Record:
- `drivers/usb/host/xhci-ring.c`: `xhci_handle_halted_endpoint()`,
`handle_port_status()` (~15+/12-)
- `drivers/usb/host/xhci.c`: `xhci_urb_enqueue()`,
`xhci_discover_or_reset_device()` (1+ / 3-)
- `drivers/usb/host/xhci.h`: drop `vdev->flags` / `VDEV_PORT_ERROR`;
add `xhci_port.link_inactive:1`
- Net: 17 insertions, 23 deletions. Single-subsystem surgical change.
**Step 2.2: Control-flow change**
- Record:
- **Before:** Port event with PLS==`XDEV_INACTIVE` and no `PORT_RESET`
sets `vdev->flags |= VDEV_PORT_ERROR` on the directly attached slot
only. `PORT_RC` clears it. `reset_device` also zeros `vdev->flags`.
Halted-endpoint recovery and URB enqueue consult that per-device
flag.
- **After:** If the port is not in reset, `port->link_inactive = (pls
== XDEV_INACTIVE)`. All devices whose `rhub_port` is that port skip
recovery/enqueue. Device reset no longer clears the flag; port
events drive it from real PLS.
**Step 2.3: Bug mechanism**
- Record: **Logic / correctness + hardware hang workaround.** Same class
as the 2019 “don’t reset an endpoint on an inactive root port” fix.
Also improves flag lifetime vs the stale-flag bug already fixed in
this tree by `d77efd7b3def7` (clearing is now “link is not Inactive”,
not “PORT_RC or reset_device succeeded”).
**Step 2.4: Fix quality**
- Record: Obviously correct for the stated gap. Minimal. Regression risk
is low: skipping recovery when the root-port link is gone is the
intended policy; clearing is tied to actual port state, which is
stricter and more accurate than clearing on `reset_device` while the
port may still be Inactive. Theoretical `rhub_port == NULL` deref
exists in the new loads; `xhci_setup_addressable_virt_dev()` sets
`rhub_port` before normal URB traffic, and mainline has no follow-up
NULL-check fix on this path.
## Phase 3: Git history
**Step 3.1: Blame**
- Record: Flag set/clear in `handle_port_status()` is `d77efd7b3def7`
(2025), which reworked `b8c3b718087b` (2019-06-18, v5.2-rc5 era, `Cc:
stable`). The hang-prevention hole for hub children has been in the
driver since 2019.
**Step 3.2: Fixes: tag**
- Record: None. The message names `b8c3b718087b`, which **is** an
ancestor of this 6.12 tree (`git merge-base --is-ancestor` exit 0).
**Step 3.3: Related file history**
- Record:
- `d77efd7b3def7` “fix stale flag preventing URBs after link state
error is cleared” — already in this tree; this commit supersedes its
per-vdev flag management.
- Same-day series on `origin/master`: `e3d757dc9257f` (this) →
`042aad8d0db66` (disconnect) → `7e0ef4332ed9b` (`EP_DROP_PENDING`).
- None of those three are in this checkout.
**Step 3.4: Author**
- Record: Mathias Nyman is the xhci maintainer. This is his generic
replacement for Xu Rao’s proposed `XHCI_NO_SOFT_RETRY` quirk on
Renesas uPD720201.
**Step 3.5: Dependencies**
- Record: This patch does **not** need the later two to compile or to
apply its own logic. The later two **do** need this
(`rhub_port->link_inactive`). Local context differs
(`readl(port->addr)` vs `xhci_portsc_readl()`, no `xhci_sideband`
field); those are apply-noise, not semantic prerequisites. `rhub_port`
itself (`06790c19086fe`) is already here.
## Phase 4: Mailing list
**Step 4.1: Original discussion**
- Record:
- `b4 dig -c e3d757dc9257f`: lore `https://patch.msgid.link/2026080614
2113.2436238-4-mathias.nyman@linux.intel.com`
- Applied version is **[PATCH 03/17] xhci features and fixes for usb-
next** (Aug 6 2026).
- First posting was **[RFT PATCH 1/3] xhci: avoid futile stop endpoint
command and host teardown** (`20260629123031.142133-2`).
- Cover letter of that RFT: stop-endpoint timeout → `xHCI host
controller not responding, assume dead`.
- No NAK. No reviewer `Cc: stable` on this specific patch.
**Step 4.2: Recipients**
- Record: `b4 dig -w`: Mathias Nyman, gregkh, linux-usb, Xu Rao. Right
list and USB maintainer.
**Step 4.3: Bug report**
- Record: Xu Rao, Renesas uPD720201 (`1912:0014`), Genesys Logic hub +
RTL8153. Repro: bring Ethernet up, hot-unplug hub. Log:
- `Transfer error` → `Soft-reset ep` → `xHCI host not responding to
stop endpoint command` → `assume dead` → `HC died`.
- Xu tested the **3-patch RFT as a whole** and said the teardown is
gone; he also said the `EP_DROP_PENDING` path (patch 3 of that
series) was used on disconnect. He agreed the generic series made
the `XHCI_NO_SOFT_RETRY` quirk unnecessary.
**Step 4.4: Series context**
- Record: Standalone for *link-error* hub children. The reported hot-
unplug teardown is a **disconnect** path that also wants
`042aad8d0db66` and `7e0ef4332ed9b`. This commit is still the
necessary first step and independently closes the Inactive-link gap.
**Step 4.5: stable@ discussion**
- Record: No stable-specific thread found for this SHA. The 2019
precursor and the 2025 stale-flag fix both went to stable.
## Phase 5: Semantic / call-path analysis
**Step 5.1: Functions**
- Record: `xhci_handle_halted_endpoint()`, `handle_port_status()`,
`xhci_urb_enqueue()`, `xhci_discover_or_reset_device()`. Structs:
`xhci_virt_device`, `xhci_port`.
**Step 5.2: Callers of halted-endpoint recovery (this tree)**
- Record: transaction/babble/split errors, stall, bulk/interrupt soft-
retry (`process_bulk_intr_td`), stream transferless events, stop-ep
completion racing a stall. All of these run from xhci interrupt/event
handling during normal USB I/O.
**Step 5.3: Callees**
- Record: `xhci_reset_halted_ep()` / `xhci_queue_reset_ep()` — the
commands that, on a dead link, have hung or killed hosts.
**Step 5.4: Reachability**
- Record: Yes. Any USB transfer on a device under an xhci root port
whose PLS is `XDEV_INACTIVE`. Hot-unplug / bad SuperSpeed link is a
normal user action, not a debug path.
**Step 5.5: Similar patterns**
- Record: Same skip already exists for the directly attached device.
Follow-up commits add `port->connected` and `EP_DROP_PENDING` for
disconnect; they are not in this tree.
## Phase 6: This tree (6.12.111)
**Step 6.1: Buggy code present?**
- Record: **Yes.** `p-6.12-3-gc504cf0baa3d8`, `VERSION=6 PATCHLEVEL=12
SUBLEVEL=111`.
- `xhci.h:753` `VDEV_PORT_ERROR`
- `xhci-ring.c:943` and `xhci.c:1572` still test `vdev->flags`
- `xhci-ring.c:2024-2028` still sets the flag only on the directly
attached `vdev`
- Candidate SHA `e3d757dc9257f` is **not** an ancestor of HEAD.
**Step 6.2: Backport difficulty**
- Record: **Minor context.** This tree still uses `readl(port->addr)`
(not `xhci_portsc_readl`), `xhci_get_usb3_hcd()`, and has no
`xhci_sideband`. The logic hunks (`VDEV_PORT_ERROR` →
`rhub_port->link_inactive`, port-level PLS update, drop
`virt_dev->flags = 0`) match this tree.
**Step 6.3: Related fixes already here**
- Record: `b8c3b718087b` and `d77efd7b3def7` are here. They only cover
the directly attached device. Disconnect/`EP_DROP_PENDING` follow-ups
are not here. Renesas `0x0014` still has only `XHCI_ZERO_64B_REGS`,
not `XHCI_NO_SOFT_RETRY`.
## Phase 7: Subsystem
**Step 7.1**
- Record: USB xhci host controller. **IMPORTANT** — default host on most
PCs, many embedded SoCs, Android.
**Step 7.2**
- Record: Actively maintained; this is a targeted hang-prevention fix in
a mature path, not a new subsystem.
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
- Record: Users of xhci with a hub (or composite device with an
integrated hub) under a root port that goes SS.Inactive, plus any
topology that currently misses `VDEV_PORT_ERROR`. Confirmed hardware:
Renesas uPD720201 + Genesys hub + RTL8153; the mechanism is not
vendor-specific.
**Step 8.2: Trigger**
- Record: Root-port link Inactive (cable/hub/link training). Common in
the field. Unprivileged users can unplug USB devices. Full Xu Rao hot-
unplug teardown also needs the later disconnect patches.
**Step 8.3: Failure mode**
- Record: Endpoint reset / soft-retry on a dead link → stop-endpoint
timeout → **`xHCI host controller not responding, assume dead`** →
entire xhci bus torn down. **CRITICAL** (host death, all USB on that
controller gone). The 2019 write-up also described host stop after
repeated endpoint resets.
**Step 8.4: Risk/benefit**
- Record: Benefit high (prevents host teardown on a realistic USB event;
closes a 2019 hang hole for hub children). Risk low (~20-line xhci-
only change, no API, policy already used for direct-attach devices).
Ratio favors backport.
## Phase 9: Synthesis
**For:**
- Real hang/host-death class bug; xhci is widely deployed.
- Extends a fix that already went to stable in 2019.
- Small, maintainer-written, tester-acked as part of the hang series.
- Buggy code is in 6.12; apply needs only local context tweaks.
- Standalone value: hub children currently do not skip recovery on root-
port Inactive.
**Against:**
- First of a 3-patch hang series; Xu Rao’s hot-unplug case also needs
the disconnect + `EP_DROP_PENDING` patches.
- No `Cc: stable` on this commit (not a negative per the review rules).
- Tiny leftover: `vdev` is still fetched in `handle_port_status()` for
later `xhci_ring_device()`, so it is not unused.
**Unresolved:** Whether `rhub_port` can be NULL at `xhci_urb_enqueue()`
in some rare pre-address path — unverified as a real 6.12 trigger; not
used as a NO reason.
**Stable rules**
1. Obviously correct and tested — **PASS** (maintainer + Tested-by)
2. Real user-visible bug — **PASS** (host teardown / hang)
3. Important — **PASS** (CRITICAL: xhci assumed dead)
4. Small and contained — **PASS**
5. No new features/APIs — **PASS**
6. Applies to this tree — **PASS** (minor context)
**Exception category:** None required; ordinary bugfix.
This should be backported to **this** 6.12 tree. It is not a new
feature; it is the missing half of an existing hang-prevention check.
The later two series patches are additional (disconnect) coverage, not a
reason to keep this gap in 6.12.
## Verification
- [Phase 1] Parsed tags: Tested-by Xu Rao; Link msgid `...2436238-4`;
maintainer SOB; no Fixes/Cc:stable
- [Phase 2] Diff: move `VDEV_PORT_ERROR` from `xhci_virt_device` to
`xhci_port.link_inactive`; 17+/23-
- [Phase 3] `git describe HEAD` / Makefile: **6.12.111**
(`p-6.12-3-gc504cf0baa3d8`)
- [Phase 3] `git merge-base --is-ancestor b8c3b718087b HEAD` → yes;
`e3d757dc9257f` → no
- [Phase 3] `git blame` on flag set/clear: `d77efd7b3def7` (already in
tree) over `b8c3b718087b` (2019)
- [Phase 3] `git show d77efd7b3def7`: stale-flag fix still uses per-vdev
flag; this commit replaces that
- [Phase 4] `b4 dig -c e3d757dc9257f`: PATCH 03/17; earlier RFT 1/3
- [Phase 4] `b4 mbox` RFT cover: “stop endpoint command” / “assume dead”
- [Phase 4] Xu Rao: teardown gone after the 3-patch series;
EP_DROP_PENDING used on unplug
- [Phase 4] Original report: Renesas 1912:0014, Genesys hub, RTL8153
hot-unplug
- [Phase 5] Callers of `xhci_handle_halted_endpoint` in `xhci-ring.c`
(soft retry, stall, transaction error)
- [Phase 6] Local `VDEV_PORT_ERROR` still present; `rhub_port` present;
Renesas 0x0014 lacks `XHCI_NO_SOFT_RETRY`
- [Phase 8] Failure mode verified in `xhci-ring.c:1725` (“xHCI host not
responding to stop endpoint command”)
- UNVERIFIED: lore.kernel.org HTTP fetch (403); used `b4` instead
- UNVERIFIED: concrete 6.12 NULL `rhub_port` at enqueue; not used for
the verdict
**YES**
drivers/usb/host/xhci-ring.c | 27 +++++++++++++++------------
drivers/usb/host/xhci.c | 4 +---
drivers/usb/host/xhci.h | 9 +--------
3 files changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 6ce5874fc4b3d..dc18da8385a21 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1008,7 +1008,7 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci,
* Avoid resetting endpoint if link is inactive. Can cause host hang.
* Device will be reset soon to recover the link so don't do anything
*/
- if (ep->vdev->flags & VDEV_PORT_ERROR)
+ if (ep->vdev->rhub_port->link_inactive)
return -ENODEV;
/* add td to cancelled list and let reset ep handler take care of it */
@@ -2007,13 +2007,15 @@ static void xhci_cavium_reset_phy_quirk(struct xhci_hcd *xhci)
static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
{
struct xhci_virt_device *vdev = NULL;
- struct usb_hcd *hcd;
- u32 port_id;
- u32 portsc, cmd_reg;
- unsigned int hcd_portnum;
struct xhci_bus_state *bus_state;
- bool bogus_port_status = false;
struct xhci_port *port;
+ struct usb_hcd *hcd;
+ bool bogus_port_status = false;
+ unsigned int hcd_portnum;
+ u32 cmd_reg;
+ u32 port_id;
+ u32 portsc;
+ u32 pls;
/* Port status change events always have a successful completion code */
if (GET_COMP_CODE(le32_to_cpu(event->generic.field[2])) != COMP_SUCCESS)
@@ -2050,6 +2052,7 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
bus_state = &port->rhub->bus_state;
hcd_portnum = port->hcd_portnum;
portsc = xhci_portsc_readl(port);
+ pls = portsc & PORT_PLS_MASK;
xhci_dbg(xhci, "Port change event, %d-%d, id %d, portsc: 0x%x\n",
hcd->self.busnum, hcd_portnum + 1, port_id, portsc);
@@ -2061,12 +2064,12 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
usb_hcd_resume_root_hub(hcd);
}
- if (vdev && (portsc & PORT_PLS_MASK) == XDEV_INACTIVE) {
- if (!(portsc & PORT_RESET))
- vdev->flags |= VDEV_PORT_ERROR;
- } else if (vdev && portsc & PORT_RC) {
- vdev->flags &= ~VDEV_PORT_ERROR;
- }
+ /*
+ * Tag broken links to avoid retries while hub driver sorts it out.
+ * Link status is not relible while port is in reset.
+ */
+ if (!(portsc & PORT_RESET))
+ port->link_inactive = (pls == XDEV_INACTIVE);
if ((portsc & PORT_PLC) && (portsc & PORT_PLS_MASK) == XDEV_RESUME) {
xhci_dbg(xhci, "port resume event for port %d\n", port_id);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 14eeb16f668b2..a4fab82da76b6 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1674,7 +1674,7 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
goto free_priv;
}
- if (xhci->devs[slot_id]->flags & VDEV_PORT_ERROR) {
+ if (xhci->devs[slot_id]->rhub_port->link_inactive) {
xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n");
ret = -ENODEV;
goto free_priv;
@@ -4038,7 +4038,6 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
xhci_get_slot_state(xhci, virt_dev->out_ctx));
xhci_dbg(xhci, "Not freeing device rings.\n");
/* Don't treat this as an error. May change my mind later. */
- virt_dev->flags = 0;
ret = 0;
goto command_cleanup;
case COMP_SUCCESS:
@@ -4090,7 +4089,6 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
}
/* If necessary, update the number of active TTs on this root port */
xhci_update_tt_active_eps(xhci, virt_dev, old_active_eps);
- virt_dev->flags = 0;
ret = 0;
command_cleanup:
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index caa3fbee29f2c..87d43bc95bf33 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -741,14 +741,6 @@ struct xhci_virt_device {
struct xhci_port *rhub_port;
struct xhci_interval_bw_table *bw_table;
struct xhci_tt_bw_info *tt_info;
- /*
- * flags for state tracking based on events and issued commands.
- * Software can not rely on states from output contexts because of
- * latency between events and xHC updating output context values.
- * See xhci 1.1 section 4.8.3 for more details
- */
- unsigned long flags;
-#define VDEV_PORT_ERROR BIT(0) /* Port error, link inactive */
/* The current max exit latency for the enabled USB3 link states. */
u16 current_mel;
@@ -1469,6 +1461,7 @@ struct xhci_port {
int hcd_portnum;
struct xhci_hub *rhub;
struct xhci_port_cap *port_cap;
+ unsigned int link_inactive:1;
unsigned int lpm_incapable:1;
unsigned long resume_timestamp;
bool rexit_active;
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH AUTOSEL 6.18] xhci: include all root port children in recovery prevention on link error
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18] xhci: include all root port children in recovery prevention on link error Sasha Levin
@ 2026-10-19 15:25 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-10-19 15:25 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-usb
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] xhci: stale port->link_inactive flag after port reset blocks URBs
- [High] xhci: null pointer dereference of rhub_port in xhci_urb_enqueue()
- [Medium] xhci: data race on xhci_port bitfields in handle_port_status()
--
commit 4f9e83e35ad88ce35eba19f2dbcc0cb418ac035b
Author: Mathias Nyman <mathias.nyman@linux.intel.com>
xhci: include all root port children in recovery prevention on link error
This commit moves the link error flag from struct xhci_virt_device to
struct xhci_port so that child devices behind hubs also avoid useless
transfer retries and endpoint recovery when a root port has a link error.
Link: https://patch.msgid.link/20260806142113.2436238-4-mathias.nyman@linux.intel.com
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 6ce5874fc4b3d..dc18da8385a21 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
[ ... ]
> @@ -2061,12 +2064,12 @@ static void handle_port_status(struct xhci_hcd *xhci, union xhci_trb *event)
> usb_hcd_resume_root_hub(hcd);
> }
>
> - if (vdev && (portsc & PORT_PLS_MASK) == XDEV_INACTIVE) {
> - if (!(portsc & PORT_RESET))
> - vdev->flags |= VDEV_PORT_ERROR;
> - } else if (vdev && portsc & PORT_RC) {
> - vdev->flags &= ~VDEV_PORT_ERROR;
> - }
> + /*
> + * Tag broken links to avoid retries while hub driver sorts it out.
> + * Link status is not relible while port is in reset.
> + */
> + if (!(portsc & PORT_RESET))
> + port->link_inactive = (pls == XDEV_INACTIVE);
[Severity: High]
Can port->link_inactive remain stuck at 1 after a port reset if reset
completion does not generate a new Port Status Change Event TRB?
Commit b69dfcab6894b1 ("xhci: fix stale flag preventig URBs after link state
error is cleared") previously addressed a scenario where a USB3 link enters
XDEV_INACTIVE between two resets in hub_port_init() by clearing
VDEV_PORT_ERROR in xhci_discover_or_reset_device().
When the port enters XDEV_INACTIVE, hardware sets PORT_PLC in PORTSC and
generates a TRB_PORT_STATUS event, which runs handle_port_status() and sets
port->link_inactive = 1 while leaving PORT_PLC set in PORTSC for USB3 ports.
Because the resetting thread holds port_dev->status_lock, port_event()
cannot run to clear PORT_PLC before the subsequent hub_port_reset() issues
USB_PORT_FEAT_BH_PORT_RESET or USB_PORT_FEAT_RESET:
drivers/usb/core/hub.c:hub_port_reset() {
for (i = 0; i < PORT_RESET_TRIES; i++) {
status = set_port_feature(hub->hdev, port1, (warm ?
USB_PORT_FEAT_BH_PORT_RESET :
USB_PORT_FEAT_RESET));
...
}
As noted in the comment at the end of handle_port_status(), the xHC only
generates a Port Status Change Event TRB when the logical OR of all PORTSC
status-change bits transitions from 0 to 1:
drivers/usb/host/xhci-ring.c:handle_port_status() {
/*
* xHCI port-status-change events occur when the "or" of all the
* status-change bits in the portsc register changes from 0 to 1.
* New status changes won't cause an event if any other change
* bits are still set. When an event occurs, switch over to
* polling to avoid losing status changes.
*/
}
Since PORT_PLC is already 1 during the reset, reset completion (setting
PORT_RC or PORT_WRC and transitioning PLS to XDEV_U0) will not generate a
new Port Status Change Event TRB. With the flag clearing removed from
xhci_discover_or_reset_device() and nothing in xhci_hub_control() updating
port->link_inactive, won't port->link_inactive stay 1 and cause
xhci_urb_enqueue() to reject all subsequent URBs for the port's devices
with -ENODEV?
>
> if ((portsc & PORT_PLC) && (portsc & PORT_PLS_MASK) == XDEV_RESUME) {
> xhci_dbg(xhci, "port resume event for port %d\n", port_id);
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 14eeb16f668b2..a4fab82da76b6 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -1674,7 +1674,7 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
> goto free_priv;
> }
>
> - if (xhci->devs[slot_id]->flags & VDEV_PORT_ERROR) {
> + if (xhci->devs[slot_id]->rhub_port->link_inactive) {
[Severity: High]
Could xhci->devs[slot_id]->rhub_port be NULL here, leading to a NULL pointer
dereference?
In xhci_check_args(), the check succeeds as soon as udev->slot_id and
xhci->devs[slot_id] are set with virt_dev->udev == udev:
drivers/usb/host/xhci.c:xhci_check_args() {
if (check_virt_dev) {
if (!udev->slot_id || !xhci->devs[udev->slot_id]) {
...
return -EINVAL;
}
virt_dev = xhci->devs[udev->slot_id];
if (virt_dev->udev != udev) {
...
return -EINVAL;
}
}
}
However, xhci_alloc_dev() -> xhci_alloc_virt_device() allocates
xhci->devs[slot_id] and sets udev->slot_id while leaving dev->rhub_port as
NULL until xhci_setup_device() -> xhci_setup_addressable_virt_dev() runs:
drivers/usb/host/xhci-mem.c:xhci_setup_addressable_virt_dev() {
/* Find the root hub port this device is under */
dev->rhub_port = xhci_find_rhub_port(xhci, udev);
if (!dev->rhub_port)
return -EINVAL;
}
For an already-enumerated device (where udev->state >= USB_STATE_DEFAULT),
xhci_discover_or_reset_device() and xhci_setup_device() (on
COMP_USB_TRANSACTION_ERROR) can call xhci_alloc_dev() before
xhci_setup_addressable_virt_dev() is reached:
drivers/usb/host/xhci.c:xhci_discover_or_reset_device() {
virt_dev = xhci->devs[slot_id];
if (!virt_dev) {
...
ret = xhci_alloc_dev(hcd, udev);
if (ret == 1)
return 0;
else
return -EINVAL;
}
}
For example, during system resume after xHCI power loss (where
xhci_mem_cleanup() freed all virt_devices and zeroed udev->slot_id),
resuming an external hub calls
usb_reset_and_verify_device() -> hub_hc_release_resources(), which
recursively invokes xhci_discover_or_reset_device() -> xhci_alloc_dev() on
all child devices behind the hub before those children are reset or
addressed:
drivers/usb/core/hub.c:hub_hc_release_resources() {
/* Release up resources for all children before this device */
for (i = 0; i < udev->maxchild; i++)
if (hub->ports[i]->child)
hub_hc_release_resources(hub->ports[i]->child);
if (hcd->driver->reset_device)
hcd->driver->reset_device(hcd, udev);
}
If an URB is submitted to a child device before its
xhci_setup_addressable_virt_dev() runs (for instance via
usb_offload_check() in usb_resume_both(), via finish_port_resume() ->
usb_get_std_status() if hub_activate() hit an earlier port status error, or
from a concurrent driver workqueue/usbfs thread during slot reallocation),
previously prepare_ring() rejected the URB with -ENOENT because the endpoint
context was still in EP_STATE_DISABLED. Now xhci_urb_enqueue() dereferences
xhci->devs[slot_id]->rhub_port before prepare_ring() is reached.
> xhci_dbg(xhci, "Can't queue urb, port error, link inactive\n");
> ret = -ENODEV;
> goto free_priv;
> @@ -4038,7 +4038,6 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
> xhci_get_slot_state(xhci, virt_dev->out_ctx));
> xhci_dbg(xhci, "Not freeing device rings.\n");
> /* Don't treat this as an error. May change my mind later. */
> - virt_dev->flags = 0;
> ret = 0;
> goto command_cleanup;
> case COMP_SUCCESS:
> @@ -4090,7 +4089,6 @@ static int xhci_discover_or_reset_device(struct usb_hcd *hcd,
> }
> /* If necessary, update the number of active TTs on this root port */
> xhci_update_tt_active_eps(xhci, virt_dev, old_active_eps);
> - virt_dev->flags = 0;
> ret = 0;
>
> command_cleanup:
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index caa3fbee29f2c..87d43bc95bf33 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
[ ... ]
> @@ -1469,6 +1461,7 @@ struct xhci_port {
> int hcd_portnum;
> struct xhci_hub *rhub;
> struct xhci_port_cap *port_cap;
> + unsigned int link_inactive:1;
> unsigned int lpm_incapable:1;
> unsigned long resume_timestamp;
> bool rexit_active;
[Severity: Medium]
Can placing the link_inactive bitfield next to lpm_incapable cause a data
race on the shared bitfield word?
In handle_port_status(), port->link_inactive is updated in interrupt context
under xhci->lock:
drivers/usb/host/xhci-ring.c:handle_port_status() {
if (!(portsc & PORT_RESET))
port->link_inactive = (pls == XDEV_INACTIVE);
}
Meanwhile, during USB3 shared_hcd registration in usb_add_hcd() and system
resume after power loss in xhci_resume(), the controller and its interrupts
are started before register_root_hub() or hub_activate() calls
xhci_pci_update_hub_device() -> xhci_find_lpm_incapable_ports(), which
writes to lpm_incapable in process context without holding xhci->lock:
drivers/usb/host/xhci-pci.c:xhci_find_lpm_incapable_ports() {
if (ret >= 0) {
rhub->ports[i]->lpm_incapable = ret;
continue;
}
}
If a Port Status Change Event interrupt fires concurrently while
xhci_find_lpm_incapable_ports() is running, could the non-atomic
read-modify-write on the shared unsigned int clobber either link_inactive
or lpm_incapable?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-362-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH AUTOSEL 6.18-6.1] usb: gadget: u_audio: clean up capture endpoint on feedback failure
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (12 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18] xhci: include all root port children in recovery prevention on link error Sasha Levin
@ 2026-10-19 13:52 ` Sasha Levin
2026-10-19 15:18 ` sashiko-bot
13 siblings, 1 reply; 27+ messages in thread
From: Sasha Levin @ 2026-10-19 13:52 UTC (permalink / raw)
To: patches, stable
Cc: Xu Rao, Greg Kroah-Hartman, Sasha Levin, linux-usb, linux-kernel
From: Xu Rao <raoxu@uniontech.com>
[ Upstream commit 7a0d4f60da261f61dcb50c8e05fa53cc7d4d4427 ]
u_audio_start_capture() enables the capture OUT endpoint, queues capture
requests and marks the stream active before setting up the optional
feedback endpoint.
If feedback endpoint configuration or enablement fails, the function
returns an error while the capture endpoint remains enabled and its
requests may remain queued. The current code even leaves TODO comments
at these return paths.
Unwind the already started capture endpoint on these failures. Also set
fb_ep_enabled only after usb_ep_enable() succeeds, so the software state
matches the endpoint state.
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Link: https://patch.msgid.link/183621D513E0DE8B+20260611091229.4017443-1-raoxu@uniontech.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have enough to judge this against the local 7.3-rc4 tree: it is a
small error-path unwind in USB gadget UAC2 capture, and the surrounding
code is present here.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[usb: gadget: u_audio]** **[clean up]** Capture OUT endpoint
is left enabled/queued if optional feedback-endpoint setup fails.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Signed-off-by: Xu Rao <raoxu@uniontech.com>` (author)
- `Link: https://patch.msgid.link/183621D513E0DE8B+20260611091229.401744
3-1-raoxu@uniontech.com`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (USB
maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence of the last two is expected for this
review)
- Ignore any pipeline `Signed-off-by: Sasha Levin` / `[Upstream commit
...]` markers; none are in the real commit
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `u_audio_start_capture()` enables the capture OUT endpoint,
queues ISO requests, and marks the stream active *before* configuring
the optional feedback IN endpoint.
- **Symptom:** On `config_ep_by_speed()` / `usb_ep_enable()` failure for
the feedback EP, the function returns an error while OUT remains
enabled and requests may stay queued. Author notes existing `TODO:
Clean up out_ep` comments.
- **Root cause:** Incomplete unwind on feedback-setup error;
`fb_ep_enabled` was also set before `usb_ep_enable()` succeeded, so
software state could claim the feedback EP was enabled when it was
not.
- **Version info:** none in the message. Related introducer is
`76a7bfc445b8e` (v6.11-rc3), which added those error returns with
TODOs.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Yes — a real error-path leak, not style cleanup.** “Clean up”
here means unwind an already-started ISO endpoint. The TODOs in the pre-
patch code make that explicit.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/usb/gadget/function/u_audio.c` — **+9 / −3** (12 lines)
- Function: `u_audio_start_capture()` only
- Scope: **single-file surgical error-path fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- Before: feedback `config_ep_by_speed` / `usb_ep_enable` failure did
`return ret; // TODO: Clean up out_ep`. `fb_ep_enabled = true` ran
*before* `usb_ep_enable()`.
- After: those two failures `goto err_out_ep`, which does
`set_active(prm, false)` and `free_ep(prm, ep)` (dequeue + disable
capture OUT). `fb_ep_enabled` is set only after successful enable.
- Path affected: **error path only**, and only when
`audio_dev->in_ep_fback` is non-NULL.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** error-path resource leak + software/hardware state
mismatch
- **Mechanism:** Capture OUT is enabled and ISO requests queued;
feedback setup then fails; caller (`set_alt`) returns error without
calling `u_audio_stop_capture()`. OUT stays live. `fb_ep_enabled`
could be true while enable failed, so later `free_ep_fback()` could
`usb_ep_disable()` an endpoint that never enabled.
- **Fix:** reuse existing `free_ep()` / `set_active()`; set the flag
after enable succeeds.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** Obviously correct. `prm` is `&uac->c_prm`; `ep` is
`audio_dev->out_ep`; that matches `u_audio_stop_capture()`. Not
calling `free_ep_fback()` on these two failures is correct (feedback
never successfully enabled).
- **Regression risk:** Low. Error path only; same helpers already used
on stop/disable.
- **Incomplete but still valid:** `usb_ep_alloc_request()` / `kzalloc()`
`-ENOMEM` returns after feedback enable still skip unwind. This patch
only closes the two TODO paths. Remaining holes are not a reason to
reject this hunk.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- TODO returns introduced by **`76a7bfc445b8e`** (`usb: gadget: u_audio:
Check return codes from usb_ep_enable and config_ep_by_speed.`, Chris
Wulff, 2024-07-21).
- That commit is contained in **`v6.11-rc3`**.
- Feedback endpoint itself: **`24f779dac8f3ef`** (2021). Capture core:
**`eb9fecb9e69b`** (v4.13-rc1).
- `free_ep()` has existed since the feedback-support commit.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No `Fixes:` tag** (expected). The incomplete unwind was
introduced by `76a7bfc445b8e`, which itself had `Fixes:` for
`eb9fecb9e69b` / `24f779dac8f3` and **`Cc: stable@vger.kernel.org`**.
This commit completes that error handling. `76a7bfc445b8e` **is an
ancestor of this HEAD**.
**Step 3.3: CHECK FILE HISTORY**
Record: Recent `u_audio.c` work is independent (kmalloc conversions, UAF
on disconnect). This is **standalone**, not patch X/Y. One later commit
on the file (`858965947081d` UAF on disconnect) does not depend on this
unwind.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Xu Rao is not the u_audio maintainer. Other USB gadget work here
includes `f_printer` kref handling. USB maintainer **Greg Kroah-
Hartman** applied this.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Needs `free_ep()`, `set_active()`, `fb_ep_enabled`, and the
feedback error returns from `76a7bfc445b8e`. **All present in this
tree.** Can apply standalone on any tree that already has
`76a7bfc445b8e`.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 7a0d4f60da261` → https://patch.msgid.link/183621D513E0DE8B+
20260611091229.4017443-1-raoxu@uniontech.com
- `b4 dig -a`: **v1 only** (2026-06-11), no later revisions
- Saved mbox (`b4 dig -m`): **submission only, no replies**
- WebFetch of lore/msgid.link returned **403**; discussion content
verified from the mbox instead
- No NAK, no `Cc: stable` in the submission, no reviewer-requested
changes
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To Greg Kroah-Hartman; Cc Kees Cook, linux-usb,
linux-kernel. Greg signed off and applied it. No separate `Reviewed-by`
in git or the mbox.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No `Reported-by` / bugzilla / syzbot. The pre-patch `TODO: Clean
up out_ep` comments are in-tree evidence the gap was already known
(`76a7bfc445b8e`).
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone single patch, not a series.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: Lore stable search via WebFetch **403**. `b4` mbox has no
stable-list traffic. **UNVERIFIED** whether stable@ discussed this
specific follow-up. The *introducer* `76a7bfc445b8e` was explicitly
Cc:stable.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: KEY FUNCTIONS**
Record: `u_audio_start_capture()` modified. Callees on the new path:
`set_active()`, `free_ep()`.
**Step 5.2: TRACE CALLERS**
Record:
- `f_uac2.c` `afunc_set_alt()`: `u_audio_start_capture(&uac2->g_audio)`
when capture interface alt != 0
- `f_uac1.c` `f_audio_set_alt()`: same, but UAC1 never assigns
`in_ep_fback`, so the feedback error path is UAC2-only
- Called from gadget `SET_INTERFACE` in `composite.c` (`f->set_alt()`
under `cdev->lock`). **Host-triggered**, not an init-only path.
**Step 5.3: TRACE CALLEES**
Record: `free_ep()` dequeues queued requests, clears `ep_enabled`,
`usb_ep_disable()`. `set_active(false)` clears the ALSA rate-control
“running” flag. Same sequence as `u_audio_stop_capture()` for the OUT
endpoint.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: USB host SET_INTERFACE (alt=1) on UAC2 capture → `afunc_set_alt`
→ `u_audio_start_capture`. Reachable whenever a device uses `f_uac2`
with capture. **Default `c_sync` is `USB_ENDPOINT_SYNC_ASYNC`
(`UAC2_DEF_CSYNC`), so `in_ep_fback` is allocated by default.** Trigger
of the *bug* still requires feedback `config_ep_by_speed` /
`usb_ep_enable` failure (malformed/missing speed descriptors, UDC enable
failure). Composite does **not** roll back `set_alt` on error
(`composite.c` just returns the error to the host). `as_out_alt` is
stored *before* `start_capture`, so a retry of alt=1 can call start
again with OUT still live. `afunc_disable` / alt=0 does call
`u_audio_stop_capture()`, so disconnect eventually unwinds — retries and
the window before alt=0 are the problem.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Playback has no feedback EP; no matching TODO there. The two
`-ENOMEM` returns after feedback enable in the same function still skip
unwind — same class of bug, not fixed here.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Local tree: **`v7.3-rc4-27-gf0100363d8c37`**, `VERSION=7 PATCHLEVEL=3
SUBLEVEL=0 EXTRAVERSION=-rc4`
- `git merge-base --is-ancestor 7a0d4f60da261 HEAD` → **yes** (this
commit is already in this checkout)
- Surrounding code **is** in this tree: `u_audio_start_capture`,
`free_ep`, `in_ep_fback`, UAC2 async feedback default
- Bug introduced **v6.11-rc3** (`76a7bfc445b8e`), long before 7.3 —
**not** “introduced after this tree branched”
- Current file already contains `err_out_ep` / post-enable
`fb_ep_enabled` (the fixed form)
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Patch is 12 lines against `u_audio_start_capture()`. Helpers it
uses exist since 2021/2022. **Clean apply** on trees that have
`76a7bfc445b8e`. This 7.3-rc4 checkout already has the result.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This *is* the fix; it is already in this tree. No earlier
substitute unwind of these two returns.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: SUBSYSTEM AND CRITICALITY**
Record: **USB gadget function / UAC2 audio
(`drivers/usb/gadget/function/`)**. **IMPORTANT** for gadgets that act
as USB sound cards (embedded, Android accessory, configfs UAC2); not
core mm/VFS. Default UAC2 capture uses async + feedback.
**Step 7.2: SUBSYSTEM ACTIVITY**
Record: File is mature (core from 2017, feedback from 2021) with
occasional correctness fixes. Not a new subsystem.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Record: **Driver-specific / config-specific:** `CONFIG` USB gadget UAC2
with capture. Feedback EP is **on by default** (`UAC2_DEF_CSYNC =
USB_ENDPOINT_SYNC_ASYNC`).
**Step 8.2: TRIGGER CONDITIONS**
Record: Host SET_INTERFACE to start capture, then feedback
`config_ep_by_speed` or `usb_ep_enable` fails. Uncommon compared with
the success path, but the same conditions `76a7bfc445b8e` documented
(bad/missing descriptors for the selected speed, enable failure). Not a
syzbot/syscall UAF; a USB host/UDC error path. Unprivileged local
userspace on the gadget does not directly call this; the USB host does.
**Step 8.3: FAILURE MODE SEVERITY**
Record:
- Enabled ISO OUT + queued requests after `set_alt` reports failure
- Stream left “active” in ALSA rate control
- Retry of start can enable/queue an already-live endpoint
- `fb_ep_enabled` vs actual enable mismatch on the enable-fail path
- **Severity: HIGH** for the gadget (stuck/inconsistent USB audio until
alt=0 or disconnect), not a generic kernel panic. Not silent
filesystem corruption.
**Step 8.4: RISK-BENEFIT RATIO**
Record:
- **Benefit:** Restores correct unwind on a path the previous stable-
bound commit left as TODO; prevents leftover ISO endpoint state
- **Risk:** Very low (9 insertions, existing helpers, error path only)
- **Ratio:** Favorable
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR backporting:**
- Real incomplete unwind; pre-patch TODOs name the bug
- Completes error handling from `76a7bfc445b8e`, which was `Cc: stable`
and is in this tree
- Leaves a live ISO endpoint and queued requests after `set_alt`
failure; composite does not auto-stop
- Also fixes `fb_ep_enabled` vs `usb_ep_enable()` ordering
- 12-line, obviously correct, uses existing `free_ep()` / `set_active()`
- UAC2 capture + feedback is the default, not an obscure option
- Applied by Greg Kroah-Hartman
- All helpers and the introducer exist in this tree; no missing
prerequisites
**AGAINST:**
- Error path; needs feedback enable/config failure (not every start)
- No user/syzbot report in the commit or mbox
- `-ENOMEM` after feedback enable still leaks (incomplete, but this hunk
is still correct)
- No `Reviewed-by` / mailing-list discussion in the matched thread
- Original submission did not Cc stable (not a negative per the review
rules)
**UNRESOLVED:** Lore HTML 403; no public review comments in the b4 mbox;
no stable@ thread found.
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — unwind matches
`u_audio_stop_capture()`; merged by USB maintainer. No extra `Tested-
by`.
2. Fixes a real bug that affects users? **PASS** — leftover enabled ISO
EP after failed SET_INTERFACE.
3. Important issue? **PASS** — not a panic, but inconsistent gadget
USB/audio state; HIGH for UAC2 gadgets. Matches the error-path leak
class that stable routinely takes.
4. Small and contained? **PASS** — one function, 12 lines.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — code and prerequisites are
here; the patch is the current code.
**Step 9.3: EXCEPTION CATEGORIES**
Record: None (not a device ID, quirk, DT, build, or docs fix). Qualifies
as a normal small bugfix.
**Step 9.4: DECISION**
This tree is **7.3-rc4**. The UAC2 capture/feedback code and the v6.11
incomplete returns are part of this lineage. The change is a small,
correct unwind of a real leftover-endpoint bug on SET_INTERFACE failure,
including a state-flag fix. That meets stable rules for **this** tree’s
code. Remaining `-ENOMEM` holes do not make this hunk unsafe or feature-
like.
The problem for stable users of gadget UAC2 is a capture start that
fails while configuring the default async feedback endpoint: OUT stays
enabled with ISO requests queued, `set_alt` returns error, and a host
retry can hit an already-live endpoint. The patch only touches that
failure path and reuses `free_ep()`. Risk is low; benefit is restoring a
consistent gadget state after a failure the previous stable-destined
commit already started handling.
### Verification
- **[Phase 1]** Parsed commit
`7a0d4f60da261f61dcb50c8e05fa53cc7d4d4427`: SOB Xu Rao + Greg KH; Link
msgid; no Fixes/Reported-by/Cc:stable
- **[Phase 2]** Diff: +9/−3 in `u_audio_start_capture()`; TODO returns →
`goto err_out_ep`; `fb_ep_enabled` moved after successful
`usb_ep_enable()`
- **[Phase 2]** Read `free_ep()` / `u_audio_stop_capture()`: same
dequeue/disable sequence
- **[Phase 3]** `git blame`: TODOs from `76a7bfc445b8e` (v6.11-rc3,
Cc:stable); `git describe --contains 76a7bfc445b8e` →
`v6.11-rc3~4^2~12`
- **[Phase 3]** `git merge-base --is-ancestor 76a7bfc445b8e HEAD` → 0
(present); same for `24f779dac8f3ef`, `eb9fecb9e69b`
- **[Phase 3]** `git log --author="Xu Rao"` limited to
`drivers/usb/gadget/`: this commit + unrelated f_printer change
- **[Phase 4]** `b4 dig -c/-a/-w/-m`: v1 only; recipients gregkh, kees,
linux-usb; mbox has patch text only, no replies
- **[Phase 4]** WebFetch lore/msgid/stable search: 403; used b4 mbox
instead
- **[Phase 5]** Callers: `f_uac2.c:1464`, `f_uac1.c:969`; `in_ep_fback`
only set in `f_uac2.c` when `EPOUT_FBACK_IN_EN`; `UAC2_DEF_CSYNC` is
`USB_ENDPOINT_SYNC_ASYNC`
- **[Phase 5]** `composite.c` SET_INTERFACE: `set_alt` error is
returned; no automatic `stop_capture`
- **[Phase 6]** `git describe HEAD` → `v7.3-rc4-27-gf0100363d8c37`;
`make kernelversion` → `7.3.0-rc4`; ancestor check for this commit →
yes
- **[Phase 8]** Failure mode: leftover enabled ISO OUT + queued reqs +
active flag; severity HIGH for UAC2 gadget, not a generic oops
- **UNVERIFIED:** Public review comments beyond the empty b4 thread;
whether stable@ already queued this SHA
**YES**
drivers/usb/gadget/function/u_audio.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index 24c673dcbbd77..75a4ce406c2a3 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -641,15 +641,15 @@ int u_audio_start_capture(struct g_audio *audio_dev)
ret = config_ep_by_speed(gadget, &audio_dev->func, ep_fback);
if (ret < 0) {
dev_err(dev, "config_ep_by_speed in_ep_fback failed (%d)\n", ret);
- return ret; // TODO: Clean up out_ep
+ goto err_out_ep;
}
- prm->fb_ep_enabled = true;
ret = usb_ep_enable(ep_fback);
if (ret < 0) {
dev_err(dev, "usb_ep_enable failed for in_ep_fback (%d)\n", ret);
- return ret; // TODO: Clean up out_ep
+ goto err_out_ep;
}
+ prm->fb_ep_enabled = true;
req_len = ep_fback->maxpacket;
req_fback = usb_ep_alloc_request(ep_fback, GFP_ATOMIC);
@@ -680,6 +680,12 @@ int u_audio_start_capture(struct g_audio *audio_dev)
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return 0;
+
+err_out_ep:
+ set_active(prm, false);
+ free_ep(prm, ep);
+
+ return ret;
}
EXPORT_SYMBOL_GPL(u_audio_start_capture);
--
2.53.0
^ permalink raw reply related [flat|nested] 27+ messages in thread