* [PATCH AUTOSEL 7.0-6.1] thunderbolt: Disable CLx on Titan Ridge-based devices with old firmware
[not found] <20260428104133.2858589-1-sashal@kernel.org>
@ 2026-04-28 10:40 ` Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.12] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Sasha Levin
` (4 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Rene Sapiens, Mika Westerberg, Sasha Levin, andreas.noever,
michael.jamet, YehezkelShB, linux-usb, linux-kernel
From: Rene Sapiens <rene.sapiens@linux.intel.com>
[ Upstream commit 59b03d12b1f6d14d936a3ebec225f8d914dc3b70 ]
Thunderbolt 3 devices based on Titan Ridge routers with NVM firmware
version < 0x65 have been observed to become unstable when CL states are
enabled. This can lead to link disconnect events and the device failing
to enumerate.
Enable CLx on Titan Ridge only when the running NVM firmware version
is >= 0x65.
Signed-off-by: Rene Sapiens <rene.sapiens@linux.intel.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:
Now I have all the information needed. Let me compile the analysis.
---
# Analysis: thunderbolt: Disable CLx on Titan Ridge-based devices with
old firmware
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1 - Subject line parsed:**
Record: [thunderbolt (subsystem)] [Disable (action verb - indicates a
bug workaround)] [CLx on Titan Ridge-based devices with old NVM
firmware]
**Step 1.2 - Tags parsed:**
Record: Two `Signed-off-by:` tags — Rene Sapiens (author) and Mika
Westerberg (subsystem maintainer). NO `Fixes:`, NO `Cc: stable`, NO
`Reported-by:`, NO `Link:`, NO `Reviewed-by:`, NO `Tested-by:`.
(Expected — that's why this commit needs review.)
**Step 1.3 - Commit body:**
Record: Bug description: "Thunderbolt 3 devices based on Titan Ridge
routers with NVM firmware version < 0x65 have been observed to become
unstable when CL states are enabled." Symptoms: "link disconnect events
and the device failing to enumerate." Failure mode: link instability,
device enumeration failure (user-visible — Thunderbolt device doesn't
work). Author's mechanism: old NVM firmware has a hardware/firmware bug
triggered when CL (low-power link) states are entered.
**Step 1.4 - Hidden bug fix detection:**
Record: Not hidden — the commit is clearly a bug-triggered hardware
workaround/quirk. "Disable" here means "disable the broken low-power
states on broken hardware/firmware combinations."
## PHASE 2: DIFF ANALYSIS - LINE BY LINE
**Step 2.1 - Inventory:**
Record: 1 file changed: `drivers/thunderbolt/quirks.c`, +7 / -0.
Functions modified: `quirk_clx_disable()` (3 lines added); table
`tb_quirks[]` (1 new entry, 4 lines including blank and comment). Scope:
single-file surgical fix.
**Step 2.2 - Code flow change:**
Record:
- Before: `quirk_clx_disable()` was only invoked from AMD Yellow Carp /
Pink Sardine table entries. When invoked, it unconditionally set
`QUIRK_NO_CLX`.
- After: A new table entry matches Intel Titan Ridge DD bridge
(0x8086:0x15ef) and invokes `quirk_clx_disable()`. Inside, if the
switch is Titan Ridge AND `sw->nvm->major >= 0x65`, the function
returns early without applying `QUIRK_NO_CLX`; otherwise it applies it
as before. AMD path behavior is preserved
(`tb_switch_is_titan_ridge(sw)` returns false for AMD parts).
**Step 2.3 - Bug mechanism:**
Record: Category (h) Hardware workaround — a vendor-identified firmware
bug on the device causes link instability under CL states. Fix adds a
device-specific quirk table entry plus a firmware-version guard.
**Step 2.4 - Fix quality:**
Record: Small, contained, obviously correct IF `sw->nvm` is populated at
`tb_check_quirks()` time. The fix itself cannot cause regressions on AMD
devices or non-Titan-Ridge Intel devices, since the new check is guarded
by `tb_switch_is_titan_ridge(sw)`. Key concern: whether `sw->nvm` is
populated when this runs — see Phase 3.5.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1 - Blame:**
Record: CLx support on Titan Ridge was introduced by `43f977bc60b1c`
("thunderbolt: Enable CL0s for Intel Titan Ridge") in v5.17-rc1 and
expanded by `b017a46d486cd` ("thunderbolt: Add CL1 support for USB4 and
Titan Ridge routers") in v6.0-rc1. So the bug window (CLx enabled with
no firmware guard on Titan Ridge DD) is v5.17+.
**Step 3.2 - Fixes: tag:**
Record: No `Fixes:` tag. Implicit target would be `43f977bc60b1c` /
`b017a46d486cd` (v5.17 / v6.0). The `QUIRK_NO_CLX` infrastructure itself
came from `7af9da8ce8f9a` ("thunderbolt: Add quirk to disable CLx") in
v6.3-rc1 and was explicitly tagged `Cc: stable@vger.kernel.org`.
**Step 3.3 - Related file history:**
Record: Recent `quirks.c` changes (`a75e0684`, `0c35ac18`, `f2bfa944`,
`ccdb0900`, `f14d177e`, `f0a57dd3`, `7af9da8c`) are all similar tiny
additions of hardware quirks/logging — the file has low churn and stable
structure.
**Step 3.4 - Author context:**
Record: Rene Sapiens (author) has recent thunderbolt work (margining,
structure cleanups). Mika Westerberg (co-Signed-off-by) is the
Thunderbolt subsystem maintainer and applied the patch directly to
`thunderbolt.git/next`. Strong authority signal.
**Step 3.5 - Dependencies (CRITICAL):**
Record: This commit is PART 2 of a 2-patch series. The prerequisite is
commit `4573add760b8d` ("thunderbolt: Read router NVM version before
applying quirks"). That prerequisite splits `tb_switch_nvm_add()` into
`tb_switch_nvm_init()` (populates `sw->nvm` / reads version) and
`tb_switch_nvm_add()` (registers nvmem), and calls
`tb_switch_nvm_init()` from `tb_switch_add()` BEFORE
`tb_check_quirks()`. Without the prerequisite, `sw->nvm` is NULL when
`tb_check_quirks()` runs, so the new guard `sw->nvm && sw->nvm->major >=
0x65` is always false, and `QUIRK_NO_CLX` is applied to ALL Titan Ridge
DD devices regardless of firmware version. The commit still fixes the
bug (pessimistically) but loses power-savings on newer firmware.
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1 - b4 dig:**
Record: `b4 dig -c 59b03d12b1f6d` could not find a match (lore search
returned nothing). Manual search via `yhbt.net/lore/linux-usb`
succeeded: cover letter at
`20260224070150.3320641-1-mika.westerberg@linux.intel.com`, patch 2/2 at
`20260224070150.3320641-3-...`, patch 1/2 at
`20260224070150.3320641-2-...`. Only v1 was submitted — no v2/v3.
**Step 4.2 - Reviewers:**
Record: CC list: Yehezkel Bernat, Lukas Wunner, Andreas Noever (past
Thunderbolt co-maintainers), Rene Sapiens (author). Mailing list: linux-
usb@vger.kernel.org. Mika Westerberg (current Thunderbolt maintainer)
submitted and applied. No Reviewed-by/Acked-by/Tested-by tags were added
in the mailing-list discussion before application, but the maintainer
applied it directly.
**Step 4.3 - Bug report:**
Record: No public Link: tag. Cover letter: "There is known issue on
Titan Ridge with older firmware that makes the link unstable if CL
states are enabled." Phrasing indicates this is a vendor-known hardware
issue (Intel internal knowledge), not a user-reported bug traceable via
lore.
**Step 4.4 - Related patches / series:**
Record: 2-patch series. Patch 1/2 (prerequisite) is `4573add760b8d` —
refactor making NVM version available early. Patch 2/2 is this commit.
Both applied as the series by Mika on 2026-03-02.
**Step 4.5 - Stable ML:**
Record: No prior stable@ discussion found for this fix. No explicit
stable nomination by reviewers (thread only has cover-letter "Applied"
reply).
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1 - Key functions:**
Record: `quirk_clx_disable()` (modified), and the `tb_quirks[]` table
(new entry).
**Step 5.2 - Callers:**
Record: `quirk_clx_disable` is called by `tb_check_quirks()` in
`drivers/thunderbolt/quirks.c` at line 135 via the table dispatch.
`tb_check_quirks()` is called once by `tb_switch_add()` at
`drivers/thunderbolt/switch.c:3341`. `tb_switch_add()` is the mandatory
router-addition path — reachable on every Thunderbolt device
enumeration.
**Step 5.3 - Callees:**
Record: `tb_switch_is_titan_ridge(sw)` (inline predicate on
vendor/device IDs). Access to `sw->nvm->major`. `tb_sw_dbg()` for the
debug message. No locking / no allocation / no I/O — safe.
**Step 5.4 - Call chain reachability:**
Record: User plugs a Thunderbolt device → PCI enumeration →
`tb_switch_add()` → `tb_check_quirks()` → this quirk. Reachable on every
plug/unplug and on every boot for integrated routers. Universal trigger
for affected hardware.
**Step 5.5 - Similar patterns:**
Record: The same `quirk_clx_disable` is already used for AMD Yellow Carp
/ Pink Sardine (by `7af9da8ce8f9a`, which was tagged `Cc:
stable@vger.kernel.org`) — direct precedent of this exact quirk being
stable-worthy.
## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
**Step 6.1 - Code in stable:**
Record: `quirks.c` with `quirk_clx_disable()` exists in stable since 6.3
(7af9da8ce8f9a). Titan Ridge CLx support in tree since 5.17. All active
stable trees (6.1.y, 6.6.y, 6.12.y, 6.16.y, etc.) have both the CLx-
enablement code AND the `quirk_clx_disable` infrastructure needed to
apply this patch.
**Step 6.2 - Backport difficulty:**
Record: The `quirks.c` hunk will apply cleanly to all modern stable
trees — the file's structure is unchanged in the surrounding context.
HOWEVER, the fix depends on `sw->nvm` being populated at
`tb_check_quirks()` time, which requires the prerequisite
`4573add760b8d` to be applied as well. If only this commit is
backported, `sw->nvm` will be NULL and the firmware-version guard will
always be false, causing the quirk to apply to ALL Titan Ridge DD
devices (over-aggressive but functionally safe — bug is still fixed for
affected users).
**Step 6.3 - Related fixes already in stable:**
Record: No earlier form of this fix exists in stable. The related
`quirk_clx_disable` for AMD is in stable trees.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1 - Subsystem/criticality:**
Record: `drivers/thunderbolt/` — PERIPHERAL driver. Affects users with
Thunderbolt 3 hardware based on Intel Titan Ridge DD bridge (0x15ef).
Users include many Intel-based laptops, eGPU docks, Thunderbolt 3 AICs
with older shipped firmware.
**Step 7.2 - Subsystem activity:**
Record: Thunderbolt is moderately active — regular fixes, hardware
quirks. Mature enough that a firmware-specific quirk is plausible long-
term.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1 - Affected users:**
Record: Hardware-specific — owners of Thunderbolt 3 devices that use the
Intel Titan Ridge DD bridge (0x15ef) variant with NVM firmware < 0x65.
Also affected: host controllers that haven't had their NVM updated.
**Step 8.2 - Trigger conditions:**
Record: Triggered whenever CL states are entered on an affected Titan
Ridge DD device. Does not require privileges — user just needs to have
affected hardware plugged in.
**Step 8.3 - Failure mode:**
Record: "Link disconnect events and the device failing to enumerate."
Severity: HIGH — the Thunderbolt device becomes unusable. Not a
crash/security issue, but data-path failure for peripheral connectivity
(including storage/display/networking that goes over Thunderbolt).
**Step 8.4 - Risk-benefit:**
Record: Benefit = medium-high (fixes device-unusable bug for real Titan
Ridge users). Risk = very low (7-line hardware quirk, guarded by a
device-ID match, cannot affect non-matching devices; already-existing
`quirk_clx_disable` precedent is in stable). Net: favorable.
## PHASE 9: FINAL SYNTHESIS
**Step 9.1 - Evidence compiled:**
FOR:
- Fixes real hardware-exposed bug causing device enumeration failure on
TB3 hardware.
- Hardware-quirk exception category — standard stable-worthy class.
- Extremely small (7 lines), device-ID guarded, cannot affect non-
matching devices.
- Applied by subsystem maintainer (Mika Westerberg) directly.
- Direct precedent in stable (7af9da8ce8f9a used same
`quirk_clx_disable` pattern, tagged `Cc: stable`).
- The bug has existed since v5.17 (CLx enabled on Titan Ridge) — broad
stable-tree applicability.
AGAINST / Concerns:
- No `Cc: stable`, no `Fixes:`, no Reported-by — maintainer chose not to
nominate explicitly.
- Depends on prerequisite `4573add760b8d` (refactor) to realize the
intended firmware-version selectivity. Without it, the commit still
fixes the bug but applies the quirk more broadly than intended (loses
CLx power savings on new-firmware Titan Ridge DD; not a correctness
regression).
**Step 9.2 - Stable rules checklist:**
1. Obviously correct and tested? Yes — logic is trivial; tested and
applied to thunderbolt tree; build-tested by kernel test robot.
2. Real bug affecting users? Yes — link disconnects and enumeration
failures on affected hardware.
3. Important issue? Yes — device-unusable issue for affected hardware
(HIGH severity for those users).
4. Small and contained? Yes — 7 lines in one file.
5. No new features/APIs? Yes — just a quirk-table entry plus a guard.
6. Applies to stable? The quirks.c hunk itself applies cleanly; the
intended semantics require the prerequisite to also be backported.
Without the prerequisite the fix degrades gracefully to "always
disable CLx on Titan Ridge DD" — still a functional fix.
**Step 9.3 - Exception category:**
Record: Hardware quirk/workaround for buggy firmware/hardware —
explicitly allowed exception for stable.
**Step 9.4 - Decision:**
YES — this is a hardware quirk for a real, device-unusable firmware bug
on Thunderbolt 3 Titan Ridge DD hardware. It is small, surgical, applied
by the subsystem maintainer, and has direct precedent (the identical
`quirk_clx_disable` mechanism was already backported to stable with `Cc:
stable` for AMD CLx-incompatible hardware). Note for the selection
pipeline: the prerequisite `4573add760b8d` should also be backported to
get the intended selective behavior; without it the fix still works but
applies more broadly than intended.
## Verification
- [Phase 1] Parsed tags from `git show 59b03d12b1f6d`: only Signed-off-
by (Rene Sapiens, Mika Westerberg). No Fixes:, Cc: stable, Reported-
by, Link:.
- [Phase 2] Diff analysis: confirmed 1 file
(`drivers/thunderbolt/quirks.c`) +7/-0. Read current file — confirmed
table entry uses `0x8086, PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_BRIDGE`.
- [Phase 2] Verified `tb_switch_is_titan_ridge()` definition in
`drivers/thunderbolt/tb.h:982-993` uses `PCI_VENDOR_ID_INTEL` (0x8086)
and matches TITAN_RIDGE 2C/4C/DD bridges — confirms AMD path
unaffected.
- [Phase 2] Verified `PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_BRIDGE` =
0x15ef in `drivers/thunderbolt/nhi.h:75`.
- [Phase 2] Verified test code (`drivers/thunderbolt/test.c:201-202`)
uses `sw->config.vendor_id = 0x8086; sw->config.device_id = 0x15ef;`
for Titan Ridge DD — confirms quirk-table vendor ID 0x8086 is correct.
- [Phase 3.1] `git describe --contains 43f977bc60b1c` → v5.17-rc1; `git
describe --contains b017a46d486cd` → v6.0-rc1: confirms CLx on Titan
Ridge has been in kernel since v5.17.
- [Phase 3.2] `git show 7af9da8ce8f9a` confirms the `quirk_clx_disable`
/ `QUIRK_NO_CLX` infrastructure came with explicit `Cc:
stable@vger.kernel.org` — direct precedent.
- [Phase 3.3] `git log --oneline v6.1..v6.6 --
drivers/thunderbolt/quirks.c` and `v6.6..v7.0` show low churn and
stable structure.
- [Phase 3.5] Verified prerequisite: `git show 4573add760b8d` confirms
`tb_switch_nvm_init()` is a NEW function split off from
`tb_switch_nvm_add()` and a new call site was added in
`tb_switch_add()` before `tb_check_quirks()`. Verified current `HEAD`
(`Linux 7.0`) still has only `tb_switch_nvm_add()` (no
`tb_switch_nvm_init()`) — confirms the dependency.
- [Phase 3.5] Read `switch.c:3297-3407` to confirm ordering:
`tb_check_quirks()` at line 3341 runs BEFORE `tb_switch_nvm_add()` at
line 3384, so `sw->nvm` is NULL at quirk time without the
prerequisite.
- [Phase 4.1] `b4 dig -c 59b03d12b1f6d` → no match; fell back to
yhbt.net/lore search which returned the series (cover + 2 patches +
Mika's "Applied" reply + kernel-test-robot build success).
- [Phase 4.1/4.4] Fetched raw mbox of cover letter
(`20260224070150.3320641-1-...`), patch 1/2 (`-2-`), patch 2/2
(`-3-`), and Mika's apply-reply
(`20260302065244.GP2275908@black.igk.intel.com`) — confirmed only v1
was submitted, applied as-is.
- [Phase 5.2] `rg tb_check_quirks` → single caller `tb_switch_add()`;
single definition in quirks.c.
- [Phase 6.1] Verified `quirk_clx_disable()` and `QUIRK_NO_CLX` came
with `7af9da8ce8f9a` tagged `Cc: stable`, so the infrastructure is
present in stable trees 6.1.y and later.
- [Phase 8.3] Failure mode taken directly from commit message: "link
disconnect events and the device failing to enumerate."
- UNVERIFIED: Could not independently confirm the exact user-reporting
trail for this bug (the mailing-list thread doesn't reference a
specific user report or syzkaller); relied on maintainer and cover-
letter wording ("known issue").
- UNVERIFIED: Could not test whether the commit applies cleanly to each
stable branch without actually running `git cherry-pick` on each
stable tree (only a HEAD worktree is present).
The commit is a small, targeted hardware quirk that fixes a real device-
usability bug on Thunderbolt 3 Titan Ridge DD hardware with old
firmware, applied by the subsystem maintainer, and directly analogous to
a previously-stable-tagged quirk for AMD CLx-incompatible parts.
**YES**
drivers/thunderbolt/quirks.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/thunderbolt/quirks.c b/drivers/thunderbolt/quirks.c
index e81de9c30eac9..9f7914ac2f48c 100644
--- a/drivers/thunderbolt/quirks.c
+++ b/drivers/thunderbolt/quirks.c
@@ -23,6 +23,9 @@ static void quirk_dp_credit_allocation(struct tb_switch *sw)
static void quirk_clx_disable(struct tb_switch *sw)
{
+ if (tb_switch_is_titan_ridge(sw) && sw->nvm && sw->nvm->major >= 0x65)
+ return;
+
sw->quirks |= QUIRK_NO_CLX;
tb_sw_dbg(sw, "disabling CL states\n");
}
@@ -61,6 +64,10 @@ static const struct tb_quirk tb_quirks[] = {
/* Dell WD19TB supports self-authentication on unplug */
{ 0x0000, 0x0000, 0x00d4, 0xb070, quirk_force_power_link },
{ 0x0000, 0x0000, 0x00d4, 0xb071, quirk_force_power_link },
+
+ /* Intel Titan Ridge CLx is unstable on early firmware versions */
+ { 0x8086, PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_BRIDGE, 0x0000, 0x0000,
+ quirk_clx_disable },
/*
* Intel Goshen Ridge NVM 27 and before report wrong number of
* DP buffers.
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 7.0-6.12] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation.
[not found] <20260428104133.2858589-1-sashal@kernel.org>
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.1] thunderbolt: Disable CLx on Titan Ridge-based devices with old firmware Sasha Levin
@ 2026-04-28 10:40 ` Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-5.10] usb: usbip: fix OOB read/write in usbip_pad_iso() Sasha Levin
` (3 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Ingo Rohloff, Thinh Nguyen, Greg Kroah-Hartman, Sasha Levin,
linux-usb, linux-kernel
From: Ingo Rohloff <ingo.rohloff@lauterbach.com>
[ Upstream commit a28de63356575612954d4e5d5f48a2488f50e16d ]
The Microchip USB3340x ULPI PHY requires a delay when switching to the
high-speed transmitter. See:
http://ww1.microchip.com/downloads/en/DeviceDoc/80000645A.pdf
Module 2 "Device Enumeration Failure with Link IP Systems"
For details on the behavior and fix, refer to the AMD (formerly Xilinx)
forum post: "USB stuck in full speed mode with USB3340 ULPI PHY, ZynqMP."
This patch uses the USB PHY Vendor-ID and Product-ID to detect the
USB3340 PHY and then applies the necessary fix if this PHY is found.
Signed-off-by: Ingo Rohloff <ingo.rohloff@lauterbach.com>
Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://patch.msgid.link/20260305121452.54082-2-ingo.rohloff@lauterbach.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:
Now I have enough information to complete the full phase-by-phase
analysis.
# Full 9-Phase Analysis
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject line parsing**
- Subsystem: `usb: dwc3:` (DesignWare USB3 controller driver)
- Action verb: "Support" (often hides a bug fix – here it is supporting
HS negotiation that is broken)
- Record: `usb: dwc3` – "Support" – one-line: enable HS chirp delay
workaround for Microchip USB3340x ULPI PHY
**Step 1.2: Tag parsing**
- Fixes: (absent – expected for candidate review)
- Reported-by: (absent)
- Tested-by: (absent)
- Reviewed-by: (absent)
- Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com> (the DWC3 subsystem
maintainer)
- Link: patch.msgid.link to v5 submission
- Cc: stable: absent
- Signed-off-by: author Ingo Rohloff + Greg KH (USB subsystem
maintainer)
- Record: The dwc3 maintainer explicitly Acked this; Greg KH merged it.
Strong quality signal.
**Step 1.3: Body analysis**
- Bug described: Microchip USB3340x ULPI PHY fails HS negotiation – USB
stays stuck in full-speed
- References Microchip erratum doc 80000645A.pdf, Module 2 "Device
Enumeration Failure with Link IP Systems"
- References AMD/Xilinx forum post "USB stuck in full speed mode with
USB3340 ULPI PHY, ZynqMP"
- Mechanism: delay needed when switching to the high-speed transmitter
(TxValid during HS Chirp)
- Record: Real hardware-level bug documented by silicon vendor, with
user-visible symptom (USB devices don't work at high speed on ZynqMP +
USB3340 boards).
**Step 1.4: Hidden bug fix detection**
- "Support" often hides a functional fix – the PHY doesn't work without
this patch on affected boards
- Record: This IS a bug fix presented as hardware enablement. The
underlying bug is a hardware silicon erratum that requires a software
workaround (controller-side XCVRDLY bit).
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- `drivers/usb/dwc3/core.c`: +20 lines (new `dwc3_ulpi_setup()` helper +
one call site)
- `drivers/usb/dwc3/core.h`: +4 lines (new `DWC3_GUSB2PHYCFG_XCVRDLY
BIT(9)`, new struct field `enable_usb2_transceiver_delay:1`,
kerneldoc)
- `drivers/usb/dwc3/ulpi.c`: +25 lines (new `dwc3_ulpi_detect_config()`,
`#include <linux/ulpi/driver.h>`, `USB_VENDOR_MICROCHIP` macro, call
in `dwc3_ulpi_init()`)
- Total: +49 / -0, contained in 3 files of one driver subdir
- Record: Single-driver surgical change, no cross-subsystem impact.
**Step 2.2: Code flow**
- `dwc3_ulpi_init()`: after registering ULPI interface, now calls
`dwc3_ulpi_detect_config()` which reads the ULPI PHY vendor/product
IDs and sets `dwc->enable_usb2_transceiver_delay` only when it sees
Microchip (0x0424) product 0x0009.
- `dwc3_core_init()`: after ULPI init, calls new `dwc3_ulpi_setup()`
which, for the specific PHY only, sets `DWC3_GUSB2PHYCFG_XCVRDLY`
(BIT(9)) on every USB2 port's GUSB2PHYCFG register.
- Record: Default behavior is completely unchanged; the new code path is
gated by a specific vendor/product ID and by presence of an ULPI PHY
(`if (!dwc->ulpi) return;`).
**Step 2.3: Bug mechanism class**
- Category (h) Hardware workaround: quirk activated via ULPI
vendor/product match, writes a controller-side delay bit per the
Microchip erratum.
- Record: Pure hardware quirk – no locking, no refcount, no allocation
changes.
**Step 2.4: Fix quality**
- Obviously correct: reads the register, ORs the bit, writes back (per
erratum's documented workaround)
- Minimal/surgical and tightly gated by vendor+product ID
- Regression risk for any other hardware is essentially zero
(`enable_usb2_transceiver_delay` only becomes true for one specific
ULPI PHY)
- Record: Low risk, high confidence.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: Blame**
- The `dwc3_hs_phy_setup()` / `dwc3_phy_setup()` loop structure using
`num_usb2_ports` was introduced by `921e109c62007` ("usb: dwc3: core:
Access XHCI address space temporarily to read port info") which is in
`v6.10-rc1`.
- Record: The per-port iteration model, and therefore the patch's `for
(index = 0; index < dwc->num_usb2_ports; index++)` construct, cleanly
matches v6.10.y and newer.
**Step 3.2: Fixes: tag follow-up**
- No Fixes: tag; the bug is a silicon erratum, not a prior kernel commit
- Record: N/A (expected).
**Step 3.3: Related history**
- Prior dwc3 ulpi history (`e5f4ca3fce90a`, `fca3f13810572`,
`ce722da66d3e9`, `e0082698b6898`, `98112041bcca1`, `88bc9d194ff69`)
shows the ULPI infrastructure has been in place since v4.x, with the
`dwc->ulpi_ready` / `dwc3_core_ulpi_init()` flow in place since v4.16.
- Record: Standalone patch – no unlanded prerequisites outside the dwc3
driver itself.
**Step 3.4: Author**
- Ingo Rohloff (Lauterbach) – embedded/debug tools vendor, not a dwc3
regular; patch went through 5 revisions to get maintainer acceptance.
- Record: External contributor, but maintainer Ack and Greg KH merge
provides the quality signal.
**Step 3.5: Dependencies**
- Relies on `dwc->num_usb2_ports` (added v6.10) and the per-port HS PHY
setup model.
- On the current 7.0 tree it uses `dwc3_readl(dwc->regs, ...)` – this is
the legacy API; commit `9accc68b1cf0a` renamed the first arg to `dwc`.
Both forms are functionally identical; any merge conflict with that
rename is trivial (one-arg swap).
- Record: Applies cleanly to 6.10.y+ stable trees; for 6.1.y/6.6.y
backport would need to be rewritten to use single-port indexing (not
just the XHCI-port probing).
## PHASE 4: MAILING LIST / EXTERNAL RESEARCH
**Step 4.1: b4 dig**
- `b4 dig -c a28de63356575` → matched thread at `https://lore.kernel.org
/all/20260305121452.54082-2-ingo.rohloff@lauterbach.com/`
- `b4 dig -a` shows v1 (Feb 24) → v2 (Feb 25) → v3 (Feb 27) → v4 (Mar 3)
→ v5 (Mar 5 — applied)
- v1/v2 used a DT property (`snps,enable_xcvrdly_quirk`) but reviewers
asked for autodetection; v3 switched to vendor/product-ID detection.
- Record: 5 revisions, significant review feedback addressed, applied
version is the latest.
**Step 4.2: b4 dig -w / recipients**
- Thread recipients include Thinh Nguyen (dwc3 maintainer), Greg KH,
linux-usb
- Record: Correct maintainers and list involved; Ack came from the
maintainer.
**Step 4.3: Bug report**
- External reports referenced in the commit message:
- Microchip erratum 80000645A ("Device Enumeration Failure with Link
IP Systems")
- AMD/Xilinx forum thread about USB stuck in full-speed on ZynqMP
- Record: Verified documented hardware issue affecting a real shipping
platform (Xilinx/AMD ZynqMP with USB3340 PHY).
**Step 4.4: Series context**
- Mbox shows this is the only patch in the v5 series (1/1); earlier
versions had a DT binding patch (2/2) that was dropped when approach
changed to autodetection
- Record: Standalone patch, no missing prerequisites.
**Step 4.5: Stable-list discussion**
- No explicit Cc: stable in the patch, and the maintainer's Ack ("Acked-
by: Thinh Nguyen ... Thanks, Thinh") did not discuss stable
- Record: No explicit stable nomination, but none is required for
candidate review.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: Key functions**
- New: `dwc3_ulpi_setup()`, `dwc3_ulpi_detect_config()`
- Modified call sites: `dwc3_core_init()`, `dwc3_ulpi_init()`
**Step 5.2: Callers**
- `dwc3_core_init()` is called from `dwc3_probe()` and from runtime
resume paths – core device bring-up path
- `dwc3_ulpi_init()` is called from `dwc3_core_ulpi_init()` which is
called from `dwc3_core_init()` only when the HW params indicate a ULPI
interface
- Record: New code runs only during dwc3 init; not in hot data paths,
IRQ context, or sleep paths.
**Step 5.3: Callees**
- `dwc3_readl`/`dwc3_writel` (MMIO) – bounded, no allocation, no lock
- Record: Minimal side effects; just register writes.
**Step 5.4: Reachability**
- Path: user boots board → dwc3 probe → `dwc3_core_init()` → (if ULPI)
`dwc3_ulpi_init()` → register PHY → read vendor/product ID → set flag;
then `dwc3_ulpi_setup()` applies XCVRDLY bit
- Reachable on every boot on affected ZynqMP/USB3340 boards; inert on
every other board
- Record: Trigger is "boot with USB3340 ULPI PHY", which is exactly the
affected population.
**Step 5.5: Similar patterns**
- The dwc3 driver already has many per-quirk bitfields
(`ulpi_ext_vbus_drv`, `dis_enblslpm_quirk`,
`dis_u2_freeclk_exists_quirk`, etc.)
- Record: New `enable_usb2_transceiver_delay` fits the existing quirk-
flag pattern.
## PHASE 6: CROSS-REFERENCING AND STABLE TREE ANALYSIS
**Step 6.1: Stable trees containing buggy code**
- The underlying "bug" is a PHY silicon defect that exists in the
hardware regardless of kernel version. The affected kernel construct
(`num_usb2_ports`, per-port HS setup) has been present since v6.10.
- Record: 6.10.y, 6.12.y (LTS), 6.14+, 6.16+, 6.17+ are applicable
targets.
**Step 6.2: Backport complications**
- v6.10.y+: patch applies essentially as-is (minor textual offset
likely)
- v6.6.y and older: the per-port `num_usb2_ports` model does not exist;
would need the single-port form (just GUSB2PHYCFG(0)). Still trivially
doable but requires an adjusted patch.
- The `dwc3_readl` API rename (`9accc68b1cf0a`) is only in 7.0-stream;
any stable tree older than that uses the same `(dwc->regs, reg)`
signature this patch writes, so no conflict there.
- Record: Clean apply to recent stable trees; minor rewrite needed for
older ones.
**Step 6.3: Related fixes already in stable**
- No prior fix for this specific USB3340 issue exists in stable; this is
the first/only fix
- Record: No duplication concern.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: Subsystem/criticality**
- Subsystem: `drivers/usb/dwc3/` – DWC3 USB controller driver (used on
Qualcomm, Xilinx/AMD ZynqMP, Rockchip, Intel, i.MX, TI, etc.)
- Criticality: IMPORTANT (widely deployed, but the fix only affects
boards with USB3340 ULPI PHY – primarily ZynqMP-based systems)
- Record: Important driver, narrow board-specific impact.
**Step 7.2: Activity**
- Very active subsystem – many recent commits in drivers/usb/dwc3 in the
past year
- Record: Actively maintained, maintainer is engaged.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: Who is affected**
- Users of boards combining a DWC3 controller with a Microchip USB3340
ULPI PHY (notably AMD/Xilinx ZynqMP UltraScale+ platforms)
- Record: Driver-specific / hardware-specific population; but a real,
shipping HW combination.
**Step 8.2: Trigger conditions**
- Every boot on affected hardware
- No special privilege needed; just plugging in any USB device
reproduces the symptom (full-speed instead of high-speed)
- Record: Easy to reproduce – it is the default behavior on affected HW
without the fix.
**Step 8.3: Failure mode**
- Functional degradation: USB stuck at 12 Mbit/s full-speed instead of
480 Mbit/s high-speed, plus outright "device enumeration failure" per
Microchip erratum
- Severity: HIGH (functional breakage of USB HS on affected boards; not
a crash, but USB effectively doesn't work properly).
**Step 8.4: Risk/benefit**
- Benefit: Restores proper USB HS operation on real shipping hardware
(ZynqMP + USB3340) — HIGH for affected users, NONE for others.
- Risk: Writes happen only when vendor+product match exactly →
effectively zero regression risk elsewhere. The new code path is well-
guarded (`if (!dwc->ulpi) return;` + `if
(dwc->enable_usb2_transceiver_delay)`).
- Scope: +49 lines, 3 files, one driver.
- Record: Very favorable benefit/risk ratio for the affected population.
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: Evidence compilation**
Evidence FOR:
- Fixes a documented silicon erratum (Microchip 80000645A) with a user-
visible symptom (USB stuck in full-speed)
- Narrowly gated by ULPI vendor+product ID – zero risk to other hardware
- Well-reviewed (5 versions), Acked by subsystem maintainer, merged by
Greg KH
- Small contained patch (+49 lines, 3 files in one driver)
- Matches the stable rules' explicit "QUIRKS and WORKAROUNDS" exception
- Addresses a real shipping platform (AMD/Xilinx ZynqMP) — not
theoretical
Evidence AGAINST:
- No Fixes: tag (but: hardware bug, no kernel commit to fix – expected)
- No Cc: stable (expected for candidate review)
- Not described with "fix" language in the subject (author framed it as
"Support")
- Adds new infrastructure (`dwc3_ulpi_setup`, `dwc3_ulpi_detect_config`,
new quirk flag) rather than a trivial one-line ID add; some consider
this "feature-shaped"
- Requires `num_usb2_ports` (v6.10+) for clean apply to older stable
trees
Unresolved: None material.
**Step 9.2: Stable rules checklist**
1. Obviously correct and tested? YES (small, gated by exact
vendor/product ID; controller register bit is the vendor-prescribed
workaround)
2. Real bug? YES (documented silicon erratum, real user reports)
3. Important issue? YES (USB HS broken on affected platforms)
4. Small/contained? YES (+49 lines, 3 files, one driver)
5. No new features/APIs? Borderline – it adds a quirk-detection
mechanism, but no userspace-visible API or new module parameter;
internal-only.
6. Applies to stable trees? YES for 6.10+; would need trivial adjustment
for older
**Step 9.3: Exception category**
- Hardware quirk/workaround for broken device → this is explicitly an
allowed exception per the stable rules.
**Step 9.4: Decision**
This is a hardware-quirk fix for a real, documented silicon erratum
affecting shipping AMD/Xilinx ZynqMP platforms. The change is strictly
additive, gated by a specific vendor/product ID match, carries
essentially zero regression risk for unaffected systems, was reviewed
through five revisions, and has the DWC3 maintainer's Ack. It directly
fits the "QUIRKS and WORKAROUNDS" exception in the stable kernel rules.
## Verification
- [Phase 1] Parsed tags and body of commit a28de63356575; confirmed
Acked-by: Thinh Nguyen and Signed-off-by: Greg KH; confirmed
references to Microchip erratum and Xilinx/AMD forum post.
- [Phase 2] Read the diff and current
`drivers/usb/dwc3/{core.c,core.h,ulpi.c}`; confirmed only-+ scope
(+49/0) and gating by `if (!dwc->ulpi)` and `if
(dwc->enable_usb2_transceiver_delay)`.
- [Phase 3] `git log --oneline master -- drivers/usb/dwc3/ulpi.c` showed
ULPI infrastructure dates back to v4.16 (`88bc9d194ff69`); `git
describe --contains 921e109c62007` = `v6.10-rc1~48^2~50` →
`num_usb2_ports` first appeared in 6.10.
- [Phase 3] `git describe --contains 9accc68b1cf0a` confirmed
`dwc3_readl` API rename is only in `next-20260205`/7.0-stream, so
older stable trees still use the signature this patch writes.
- [Phase 4] `b4 dig -c a28de63356575` → found lore thread `https://lore.
kernel.org/all/20260305121452.54082-2-ingo.rohloff@lauterbach.com/`.
- [Phase 4] `b4 dig -c a28de63356575 -a` → listed v1..v5 with lore URLs;
v5 is the applied revision.
- [Phase 4] Read `/tmp/thread.mbox` (saved via `b4 dig -m`): verified
Acked-by from Thinh Nguyen (dwc3 maintainer) and the changelog v1→v5
switching from DT property to vendor/product-ID autodetection.
- [Phase 5] Read `dwc3_ulpi_init()` in `drivers/usb/dwc3/ulpi.c` and its
caller `dwc3_core_ulpi_init()` at `drivers/usb/dwc3/core.c:651-665`;
confirmed reachability from `dwc3_core_init()` only on ULPI-capable
HW.
- [Phase 6] `git branch --contains a28de63356575` shows the commit is in
`bus-next` (linux-next branch for 7.1), not yet in `master` (7.0),
matching its status as a backport candidate.
- [Phase 8] Severity HIGH verified via the Microchip erratum title
"Device Enumeration Failure with Link IP Systems" cited in commit
message and the Xilinx/AMD forum title "USB stuck in full speed mode
with USB3340 ULPI PHY".
- UNVERIFIED: Exact port counts affected on ZynqMP deployments – not
needed for decision.
- UNVERIFIED: Whether the patch has been tested on multiple HW variants
– only author's testing environment implied.
The patch is a well-bounded hardware quirk for a documented silicon bug
on a real, shipping platform; it fits the stable rules' explicit
quirk/workaround exception, carries essentially zero regression risk to
unaffected systems, and is maintainer-Acked. For stable branches v6.10
and newer it should apply cleanly; older branches would need a small
adjustment to the port-iteration model.
**YES**
drivers/usb/dwc3/core.c | 20 ++++++++++++++++++++
drivers/usb/dwc3/core.h | 4 ++++
drivers/usb/dwc3/ulpi.c | 25 +++++++++++++++++++++++++
3 files changed, 49 insertions(+)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 161a4d58b2cec..6047ae280c602 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -782,6 +782,24 @@ static int dwc3_hs_phy_setup(struct dwc3 *dwc, int index)
return 0;
}
+static void dwc3_ulpi_setup(struct dwc3 *dwc)
+{
+ int index;
+ u32 reg;
+
+ /* Don't do anything if there is no ULPI PHY */
+ if (!dwc->ulpi)
+ return;
+
+ if (dwc->enable_usb2_transceiver_delay) {
+ for (index = 0; index < dwc->num_usb2_ports; index++) {
+ reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(index));
+ reg |= DWC3_GUSB2PHYCFG_XCVRDLY;
+ dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(index), reg);
+ }
+ }
+}
+
/**
* dwc3_phy_setup - Configure USB PHY Interface of DWC3 Core
* @dwc: Pointer to our controller context structure
@@ -1363,6 +1381,8 @@ int dwc3_core_init(struct dwc3 *dwc)
dwc->ulpi_ready = true;
}
+ dwc3_ulpi_setup(dwc);
+
if (!dwc->phys_ready) {
ret = dwc3_core_get_phy(dwc);
if (ret)
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index a35b3db1f9f3e..a39bf284c763f 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -302,6 +302,7 @@
#define DWC3_GUSB2PHYCFG_SUSPHY BIT(6)
#define DWC3_GUSB2PHYCFG_ULPI_UTMI BIT(4)
#define DWC3_GUSB2PHYCFG_ENBLSLPM BIT(8)
+#define DWC3_GUSB2PHYCFG_XCVRDLY BIT(9)
#define DWC3_GUSB2PHYCFG_PHYIF(n) (n << 3)
#define DWC3_GUSB2PHYCFG_PHYIF_MASK DWC3_GUSB2PHYCFG_PHYIF(1)
#define DWC3_GUSB2PHYCFG_USBTRDTIM(n) (n << 10)
@@ -1161,6 +1162,8 @@ struct dwc3_glue_ops {
* 3 - Reserved
* @dis_metastability_quirk: set to disable metastability quirk.
* @dis_split_quirk: set to disable split boundary.
+ * @enable_usb2_transceiver_delay: Set to insert a delay before the
+ * assertion of the TxValid signal during a HS Chirp.
* @sys_wakeup: set if the device may do system wakeup.
* @wakeup_configured: set if the device is configured for remote wakeup.
* @suspended: set to track suspend event due to U3/L2.
@@ -1403,6 +1406,7 @@ struct dwc3 {
unsigned dis_metastability_quirk:1;
unsigned dis_split_quirk:1;
+ unsigned enable_usb2_transceiver_delay:1;
unsigned async_callbacks:1;
unsigned sys_wakeup:1;
unsigned wakeup_configured:1;
diff --git a/drivers/usb/dwc3/ulpi.c b/drivers/usb/dwc3/ulpi.c
index 57daad15f502d..a256b7f5d78b4 100644
--- a/drivers/usb/dwc3/ulpi.c
+++ b/drivers/usb/dwc3/ulpi.c
@@ -10,10 +10,13 @@
#include <linux/delay.h>
#include <linux/time64.h>
#include <linux/ulpi/regs.h>
+#include <linux/ulpi/driver.h>
#include "core.h"
#include "io.h"
+#define USB_VENDOR_MICROCHIP 0x0424
+
#define DWC3_ULPI_ADDR(a) \
((a >= ULPI_EXT_VENDOR_SPECIFIC) ? \
DWC3_GUSB2PHYACC_ADDR(ULPI_ACCESS_EXTENDED) | \
@@ -83,6 +86,26 @@ static const struct ulpi_ops dwc3_ulpi_ops = {
.write = dwc3_ulpi_write,
};
+static void dwc3_ulpi_detect_config(struct dwc3 *dwc)
+{
+ struct ulpi *ulpi = dwc->ulpi;
+
+ switch (ulpi->id.vendor) {
+ case USB_VENDOR_MICROCHIP:
+ switch (ulpi->id.product) {
+ case 0x0009:
+ /* Microchip USB3340 ULPI PHY */
+ dwc->enable_usb2_transceiver_delay = true;
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
+}
+
int dwc3_ulpi_init(struct dwc3 *dwc)
{
/* Register the interface */
@@ -92,6 +115,8 @@ int dwc3_ulpi_init(struct dwc3 *dwc)
return PTR_ERR(dwc->ulpi);
}
+ dwc3_ulpi_detect_config(dwc);
+
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 7.0-5.10] usb: usbip: fix OOB read/write in usbip_pad_iso()
[not found] <20260428104133.2858589-1-sashal@kernel.org>
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.1] thunderbolt: Disable CLx on Titan Ridge-based devices with old firmware Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-6.12] usb: dwc3: Support USB3340x ULPI PHY high-speed negotiation Sasha Levin
@ 2026-04-28 10:40 ` Sasha Levin
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-5.10] usb: gadget: bdc: validate status-report endpoint indices Sasha Levin
` (2 subsequent siblings)
5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Kelvin Mbogo, Greg Kroah-Hartman, Sasha Levin, valentina.manea.m,
shuah, linux-usb, linux-kernel
From: Kelvin Mbogo <addcontent08@gmail.com>
[ Upstream commit 74a2287209a858470d15e2996ead2337bd293ff4 ]
usbip_pad_iso() repositions ISO frame data within the transfer buffer
via memmove(). Neither the source offset (actualoffset, derived by
subtracting wire-supplied actual_length values) nor the destination
offset (iso_frame_desc[i].offset, taken directly from the wire) is
bounds-checked.
If a crafted actual_length wraps actualoffset negative through the
subtraction (see patch 2/3 for the root cause), the memmove source
points before the allocation - slab OOB read, data returned to
userspace.
Independently, iso_frame_desc[i].offset is never validated against
transfer_buffer_length. Setting offset past the end of the buffer
gives a fully controlled OOB write into whatever sits next in the
slab - confirmed with offset=400 on a 392-byte buffer, 64-byte write.
Add bounds checks for both the source and destination ranges before
each memmove call. Use unsigned comparisons after the sign check on
actualoffset to avoid signed/unsigned conversion surprises.
Signed-off-by: Kelvin Mbogo <addcontent08@gmail.com>
Link: https://patch.msgid.link/20260325103640.8090-3-addcontent08@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:
Based on my extensive investigation, I have enough information to
provide a complete analysis.
## Phase 1: Commit Message Forensics
**Step 1.1 Subject:** `usb: usbip: fix OOB read/write in
usbip_pad_iso()` — subsystem `usb:usbip`, verb **fix**, claim: bounds-
check the source/destination offsets used inside `usbip_pad_iso()`'s
`memmove()`.
**Step 1.2 Tags:**
- `Signed-off-by: Kelvin Mbogo <addcontent08@gmail.com>` (author)
- `Link:
https://patch.msgid.link/20260325103640.8090-3-addcontent08@gmail.com`
(v2 3/3)
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (USB
maintainer applied it)
- No `Fixes:`, no explicit `Cc: stable` (expected for this review
pipeline)
**Step 1.3 Body:** Two distinct attacks documented:
1. Crafted `actual_length` makes `actualoffset` go negative (subtraction
underflow) → `memmove()` source points **before** the slab allocation
→ slab OOB read whose bytes are returned to userspace via
`USBDEVFS_REAPURB`.
2. Wire-supplied `iso_frame_desc[i].offset` is never validated against
`transfer_buffer_length`. Setting offset past buffer end → fully
controlled OOB write — **confirmed by the author with offset=400 on a
392-byte buffer producing a 64-byte OOB write**. Record: both an
info-leak and a controlled heap write, triggered by a malicious
USB/IP peer.
**Step 1.4 Hidden fix?** Not hidden at all — "fix OOB read/write" is a
textbook security bug-fix phrasing.
## Phase 2: Diff Analysis
**Step 2.1 Inventory:** Single file `drivers/usb/usbip/usbip_common.c`,
+36 lines, 0 removed. One function: `usbip_pad_iso()`.
**Step 2.2 Flow change:** Before — `memmove(transfer_buffer +
iso_frame_desc[i].offset, transfer_buffer + actualoffset,
iso_frame_desc[i].actual_length)` runs unconditionally on wire-supplied
values. After — two guard blocks reject negative/out-of-range source and
any destination offset that exceeds the buffer, logging and returning
early.
**Step 2.3 Bug mechanism:** (d) memory-safety bounds check + (f)
mitigates signed/unsigned conversion via explicit `actualoffset < 0`
check before the `(unsigned int)` casts. It is categories (a) (input
validation) and (d) (bounds check before `memmove`).
**Step 2.4 Quality:** Fix is surgical, arithmetic is written without
overflow risk (`length > buf - off` pattern, not `off + length > buf`).
Return is early (`void` function), no lock/state changes. Regression
risk: very low — a malformed frame simply skips padding restoration,
which would have corrupted the kernel anyway.
## Phase 3: Git History
**Step 3.1 Blame:** `git blame` shows the vulnerable loop was introduced
in **2011** by `28276a28d8b3cd ("staging: usbip: bugfix for isochronous
packets and optimization")` (Arjan Mels). That commit itself was `Cc:
stable` in 2011, landed in 3.0. Bug has existed in every LTS since then.
**Step 3.2 Fixes: tag:** None declared in the message body, but the
offending code = 28276a28d8b3cd (2011, v3.0).
**Step 3.3 Related changes:** This commit is **patch 3/3** of Kelvin
Mbogo's v2 series. Upstream, the three patches are:
- `1897852293fac` — patch 1/3: integer overflow in `np * sizeof(*iso)`
- `591c1d972d8f1` — patch 2/3: validate per-frame `actual_length`
- `74a2287209a85` — patch 3/3: THIS commit (OOB in `usbip_pad_iso()`)
A related, independent fix by Nathan Rebello — `2ab833a16a825` ("usbip:
validate number_of_packets in `usbip_pack_ret_submit()`") — closed a
fourth related hole; it carried `Cc: stable` and is already backported
as `d374421fc6889` on `pending-7.0`, plus into 6.12/6.6/6.1 pending
branches.
**Step 3.4 Author:** Kelvin Mbogo is a new contributor; fix was applied
by USB maintainer Greg KH. Series acknowledged by `Shuah Khan` (usbip
maintainer) for the parallel Nathan patch.
**Step 3.5 Dependencies:** The commit message references "see patch 2/3
for the root cause" of negative `actualoffset`. Critically, patch 3/3
contains its own `actualoffset < 0` check, so it stands alone; the OOB
write via `iso_frame_desc[i].offset` is **wholly independent** of
patches 1 and 2. Ideally patches 1 and 2 are backported together, but
3/3 still removes a user-triggerable heap OOB write even standalone.
## Phase 4: Mailing-list Research
**Step 4.1 b4 dig / b4 am:** Ran `b4 am -o /tmp/usbip_v3/
20260325103640.8090-3-addcontent08@gmail.com` — lore thread contains v2
patches 1/3, 2/3, 3/3; 4 thread messages + 6 code-review replies. The
committed version is v2, applies cleanly on current tree per b4.
**Step 4.2 Reviewers/CC:** `linux-usb@vger.kernel.org`,
`gregkh@linuxfoundation.org`, `skhan@linuxfoundation.org`. Applied
directly by Greg KH.
**Step 4.3 Bug origin:** Not a syzbot report — found by manual source
audit by the author and independently by Sebastián Alba Vives (spinics
stable-list [SECURITY] post Mar 29 2026: "[SECURITY] usbip:
iso_frame_desc OOB memmove via crafted offset/length"). Two independent
auditors flagging the same class of bug is a strong real-world signal.
**Step 4.4 Series context:** The three-patch series + Nathan Rebello's
complementary patch form a coordinated hardening of the USB/IP receive
path against malicious remote servers.
**Step 4.5 Stable list:** Sebastián's [SECURITY] post on 2026-03-29
proposed validating offset/length for the same issue, targeting stable
explicitly. The class has clear stable-maintainer awareness.
## Phase 5: Code Semantic Analysis
**Step 5.1 Functions modified:** `usbip_pad_iso()` only.
**Step 5.2 Callers:** `grep` shows exactly one caller:
`vhci_recv_ret_submit()` in `drivers/usb/usbip/vhci_rx.c` line 92 — the
vhci (client) receive path. That function runs in the vhci_rx kthread
processing `USBIP_RET_SUBMIT` responses from the remote server over TCP.
**Step 5.3 Callees:** `memmove()`, `dev_err()` — minimal.
**Step 5.4 Reachability:** Path is `vhci_rx_loop → vhci_rx_pdu →
vhci_recv_ret_submit → usbip_recv_xbuff → usbip_recv_iso →
usbip_pad_iso`. Every byte fed into `iso_frame_desc[i].offset` /
`actual_length` comes directly from the TCP stream via
`usbip_pack_iso()` (ref: `usbip_common.c:619-632`). **Fully reachable
from a network peer — no local privilege required on the vhci side.**
**Step 5.5 Similar patterns:** Patches 1 and 2 of same series handle
related missing bounds checks; Nathan's commit (already in
pending-6.1/6.6/6.12/7.0) handles the `number_of_packets` OOB. No other
sibling drivers need this fix.
## Phase 6: Stable Tree Analysis
**Step 6.1 Buggy code in stable?** Verified by reading
`stable/linux-5.4.y`, `stable/linux-5.10.y`, `stable/linux-6.1.y`:
`usbip_pad_iso()` body is character-for-character identical to mainline
pre-fix. Bug exists in **every active LTS** (5.4.y, 5.10.y, 5.15.y,
6.1.y, 6.6.y, 6.12.y, 7.0.y).
**Step 6.2 Backport difficulty:** The surrounding context (comments,
loop) is unchanged for 15 years. Patch will apply with at most line-
offset fuzz. Zero rework expected.
**Step 6.3 Related fix already in stable?** Nathan's `2ab833a16a825` is
in pending-6.1/6.6/6.12/7.0 already, explicitly marked `Cc: stable`.
This Kelvin patch is the complement that closes the remaining
offset/length-derived OOB.
## Phase 7: Subsystem Context
**Step 7.1 Subsystem:** `drivers/usb/usbip` — networked USB
virtualization. PERIPHERAL in user-count, but *security-critical*
because untrusted network bytes reach kernel memory operations.
**Step 7.2 Activity:** USB/IP is actively maintained; recent commits in
2025-2026 include multiple hardening fixes for the same receive path
(Nathan Rebello's commit, Kelvin's series).
## Phase 8: Impact & Risk
**Step 8.1 Affected users:** Anyone running a USB/IP client (vhci-hcd)
and attaching to a remote `usbipd`. Linux distributions ship this
(`CONFIG_USBIP_CORE`, `CONFIG_USBIP_VHCI_HCD`); cloud test labs, VDI
setups, IoT dev boards, and Android-on-x86 all use it.
**Step 8.2 Trigger:** A malicious or compromised USB/IP server sends a
crafted `USBIP_RET_SUBMIT` response with a valid number_of_packets but
poisoned `iso_frame_desc[i].offset` (> transfer_buffer_length) or
manufactured `actual_length` values that wrap `actualoffset` negative.
**No authentication; no local privilege needed on the client.**
**Step 8.3 Severity:** CRITICAL — slab-level OOB **write** of attacker-
controlled size at attacker-controlled offset in kernel heap memory;
plus OOB **read** that leaks kernel heap content back to userspace. This
is the classic "exploit primitive" class of bug.
**Step 8.4 Risk/benefit:**
- Benefit: very high — kernel heap corruption from network is among the
highest-severity bug classes.
- Risk: very low — adds only input validation with early `return;`; the
only possible regression is that previously-broken malformed frames
now fail silently instead of corrupting kernel memory, which is the
desired behavior.
## Phase 9: Synthesis
**Evidence FOR backport:** Remote OOB read (info leak) + remote OOB
write (controlled heap write) with confirmed reproducer; 15-year-old bug
in every stable tree; minimal surgical diff; USB maintainer SOB; part of
a coordinated hardening series whose companion Nathan patch is already
marked for stable and accepted into pending branches; two independent
security researchers flagged the same class.
**Evidence AGAINST:** Commit message mentions "see patch 2/3 for the
root cause" suggesting a small dependency on patch 2/3 for one of two
attack vectors; however patch 3/3 contains its own `actualoffset < 0`
check so it mitigates that vector standalone, and the
`iso_frame_desc[i].offset` OOB write is fully independent. Best practice
would be to backport the full 3-patch series together, but the
standalone 3/3 is still clearly beneficial.
**Stable rules checklist:** (1) Obviously correct — YES, read of 10
lines; (2) Fixes real bug — YES, OOB read+write; (3) Important — YES,
remote-triggerable memory corruption; (4) Small — YES, 36 lines one
function; (5) No new features — YES, input validation only; (6) Applies
— YES, identical context in every stable tree.
**Exception category:** Security fix, not listed in "exceptions" but
strongly supported by stable rules.
## Verification
- [Phase 1] Parsed tags in candidate commit message: author SOB, Greg KH
SOB, patch.msgid.link Link.
- [Phase 2] Read full diff; verified guard structure uses underflow-safe
form `length > buf - off`.
- [Phase 3] `git blame -L 750,760` → loop introduced by `28276a28d8b3cd`
in 2011 (v3.0).
- [Phase 3] `git show 28276a28d8b3cd` → original commit is `Cc: stable`
and landed in every LTS.
- [Phase 3] `git log --grep` + `git log pending-{6.1,6.6,6.12,7.0}` →
confirmed Nathan's `2ab833a16a825` / `d374421fc6889` is companion fix
already on all stable pending branches.
- [Phase 3] `git branch --contains 74a2287209a85` → commit is on
`origin/master` and `linux-next/master`, merged via `usb-7.1-rc1`.
- [Phase 4] `b4 am -o /tmp/usbip_v3
20260325103640.8090-3-addcontent08@gmail.com` → retrieved thread,
confirmed this is v2 3/3, applies cleanly.
- [Phase 4] Read `/tmp/usbip_patch.mbox` /
`20260325103640.8090-3-addcontent08@gmail.com.mbx` → reviewer feedback
by Greg KH, patch accepted.
- [Phase 4] WebSearch confirmed independent [SECURITY] report on spinics
stable list by Sebastián Alba Vives on 2026-03-29.
- [Phase 5] `Grep "usbip_pad_iso"` → exactly one caller
`vhci_recv_ret_submit` in `drivers/usb/usbip/vhci_rx.c:92`; confirmed
the trigger path begins at TCP input.
- [Phase 5] Read `usbip_recv_iso()` + `usbip_pack_iso()` → verified
`iso_frame_desc[i].offset/actual_length` are copied verbatim from the
wire.
- [Phase 6] `git show stable/linux-5.4.y:... | sed -n '647,680p'`,
`linux-5.10.y`, `linux-6.1.y` → confirmed identical pre-fix code in
every LTS.
- [Phase 8] Failure mode: OOB read into userspace + OOB write of
attacker-controlled bytes → CRITICAL, remote, unauthenticated.
- UNVERIFIED: Whether a public CVE number has been assigned to this
specific OOB in `usbip_pad_iso()` (not required for decision).
The fix is small, clearly correct, reproduced by the author, addresses
remote kernel-heap OOB read/write in code that has been present in every
stable tree since 2011, was accepted by the USB maintainer, and is part
of a series whose companion fix was already nominated and accepted for
stable. This is exactly what stable trees exist to carry.
**YES**
drivers/usb/usbip/usbip_common.c | 36 ++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index fd620e9600394..8ebaaeaf848e5 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -770,6 +770,42 @@ void usbip_pad_iso(struct usbip_device *ud, struct urb *urb)
*/
for (i = np-1; i > 0; i--) {
actualoffset -= urb->iso_frame_desc[i].actual_length;
+
+ /*
+ * Validate source range: actualoffset can go negative
+ * via crafted actual_length values from the wire.
+ */
+ if (actualoffset < 0 ||
+ (unsigned int)actualoffset >
+ (unsigned int)urb->transfer_buffer_length ||
+ urb->iso_frame_desc[i].actual_length >
+ (unsigned int)urb->transfer_buffer_length -
+ (unsigned int)actualoffset) {
+ dev_err(&urb->dev->dev,
+ "pad_iso: bad src off=%d len=%u bufsz=%d\n",
+ actualoffset,
+ urb->iso_frame_desc[i].actual_length,
+ urb->transfer_buffer_length);
+ return;
+ }
+
+ /*
+ * Validate destination range: iso_frame_desc[i].offset
+ * is wire-supplied and must not exceed the buffer.
+ */
+ if (urb->iso_frame_desc[i].offset >
+ (unsigned int)urb->transfer_buffer_length ||
+ urb->iso_frame_desc[i].actual_length >
+ (unsigned int)urb->transfer_buffer_length -
+ urb->iso_frame_desc[i].offset) {
+ dev_err(&urb->dev->dev,
+ "pad_iso: bad dst off=%u len=%u bufsz=%d\n",
+ urb->iso_frame_desc[i].offset,
+ urb->iso_frame_desc[i].actual_length,
+ urb->transfer_buffer_length);
+ return;
+ }
+
memmove(urb->transfer_buffer + urb->iso_frame_desc[i].offset,
urb->transfer_buffer + actualoffset,
urb->iso_frame_desc[i].actual_length);
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 7.0-5.10] usb: gadget: bdc: validate status-report endpoint indices
[not found] <20260428104133.2858589-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-5.10] usb: usbip: fix OOB read/write in usbip_pad_iso() Sasha Levin
@ 2026-04-28 10:40 ` Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-5.10] usb: usbip: fix integer overflow in usbip_recv_iso() Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-5.10] usb: usbip: validate iso frame actual_length " Sasha Levin
5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-04-28 10:40 UTC (permalink / raw)
To: patches, stable
Cc: Pengpeng Hou, Florian Fainelli, Justin Chen, Greg Kroah-Hartman,
Sasha Levin, alcooperx, linux-usb, linux-kernel
From: Pengpeng Hou <pengpeng@iscas.ac.cn>
[ Upstream commit a402532ab855620e02a16950aea86fc621c6f87c ]
bdc_sr_xsf() decodes a 5-bit endpoint number from the hardware status
report and uses it to index bdc->bdc_ep_array[] directly. The array is
only allocated to bdc->num_eps for the current controller instance, so a
status report can carry an endpoint number that still fits the 5-bit
field but does not fit the runtime-sized endpoint table.
Reject status reports whose endpoint number is outside bdc->num_eps
before indexing the endpoint array.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Tested-by: Justin Chen <justin.chen@broadcom.com>
Link: https://patch.msgid.link/20260323121730.75245-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:
Now I have enough context. Let me write up the complete analysis.
## Analysis
### PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject line**
- Subsystem: `usb: gadget: bdc:`
- Action verb: "validate" (typical hardening language; equivalent to
"add missing bounds check")
- Record: "usb gadget BDC driver adds bounds check on endpoint number
decoded from hardware status report before indexing the endpoint
array"
**Step 1.2: Tags**
- `Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>` (author)
- `Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>`
(Broadcom maintainer)
- `Tested-by: Justin Chen <justin.chen@broadcom.com>` (Broadcom
engineer)
- `Link:
https://patch.msgid.link/20260323121730.75245-1-pengpeng@iscas.ac.cn`
- `Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>` (USB
maintainer)
- No `Fixes:` tag (expected per instructions)
- No `Cc: stable` (expected per instructions)
- No `Reported-by:` (no external bug report)
- Record: reviewed by hardware vendor's kernel team and tested by a
Broadcom engineer; strong quality signal
**Step 1.3: Commit body**
- Bug described: `bdc_sr_xsf()` decodes a 5-bit endpoint number from the
hardware status report and indexes `bdc->bdc_ep_array[]` without
validating it. The array is only `bdc->num_eps` entries long.
- Symptom/failure mode: if hardware delivers an `ep_num` in [num_eps,
32), an out-of-bounds read occurs followed by a dereference of
whatever garbage pointer was read.
- Record: author clearly identifies the OOB mechanism; this is a
defensive bounds check
**Step 1.4: Hidden bug fix detection**
- "validate" + "Reject status reports whose endpoint number is outside
bdc->num_eps before indexing the endpoint array" = hardening that
prevents OOB array read; this IS a bug fix
- Record: bounds-check fix, effectively a buffer-overread fix
### PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- 1 file changed: `drivers/usb/gadget/udc/bdc/bdc_ep.c`
- +4/-0 lines
- Function modified: `bdc_sr_xsf()`
- Record: single-file surgical fix, minimal scope
**Step 2.2: Code flow change**
- Before: decodes `ep_num` from `sreport->offset[3]`, then directly uses
it as array index into `bdc_ep_array[]`. The subsequent `!ep` check
only catches the case where the out-of-bounds read happens to return
NULL.
- After: rejects and logs out-of-range `ep_num` before touching the
array.
- Record: adds bounds validation on the IRQ-context transfer-complete
status path
**Step 2.3: Bug mechanism**
- Category (f): memory safety / bounds check
- `bdc->num_eps = num_ieps + num_oeps + 2`, each from 4-bit `NUM_NCS`
register field (see `drivers/usb/gadget/udc/bdc/bdc_core.c:392-400`),
so `num_eps` can be as low as 2 and up to 32.
- `ep_num = (le32_to_cpu(sreport->offset[3])>>4) & 0x1f;` yields 0..31.
- For any controller variant where `num_eps < 32`, a status report with
`ep_num >= num_eps` triggers an out-of-bounds read on
`bdc_ep_array[ep_num]`, then `ep->flags` dereferences whatever pointer
was read.
- Record: classic array-index-OOB read on a dynamically-sized array,
potential NULL-check bypass + dereference of uninitialized/unrelated
kernel memory
**Step 2.4: Fix quality**
- Obviously correct: `if (ep_num >= bdc->num_eps) return;` is a textbook
guard.
- Minimal: 4 lines, no unrelated changes.
- Regression risk: ~zero. The only new behavior is rejecting status
reports with out-of-range endpoint numbers — which the existing code
could never service correctly anyway.
- Record: fix quality very high; virtually no regression risk
### PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: Blame**
- `git blame` on the target lines shows the buggy code was introduced by
`efed421a94e62` ("usb: gadget: Add UDC driver for Broadcom USB3.0
device controller IP BDC") by Ashwini Pahuja, 2014-11-13.
- `git describe --contains efed421a94e62` → `v3.19-rc1~80^2~32^2~37`.
Code present since v3.19 (2015).
- Record: bug has been latent in the driver since v3.19 — present in
every currently supported stable tree
**Step 3.2: Fixes: tag**
- None present (expected).
- Record: no explicit Fixes target; buggy code is the original driver
submission
**Step 3.3: Related file history**
- `git log --oneline -20 -- drivers/usb/gadget/udc/bdc/bdc_ep.c` shows
only minor cleanups since 2014 — no semantic churn.
- Record: no prerequisites; patch is self-contained
**Step 3.4: Author history**
- Author Pengpeng Hou is a systematic hardening contributor doing
"validate X indices" / "bound Y buffer" patches across multiple
subsystems (wifi, NFC, Bluetooth, USB, tracing, etc.).
- Record: author has a consistent track record of bounds-check hardening
fixes
**Step 3.5: Dependencies**
- None; patch only adds a local `if` block. No new functions or
structures referenced.
- Record: standalone fix
### PHASE 4: MAILING LIST RESEARCH
**Step 4.1: b4 dig**
- `b4 dig -c a402532ab8556` found the original thread:
https://patch.msgid.link/20260323121730.75245-1-pengpeng@iscas.ac.cn
- `b4 dig -c a402532ab8556 -a` shows only v1 exists; applied version is
the submitted version.
- Mbox thread read: Florian Fainelli (Broadcom) Reviewed-by; Justin Chen
(Broadcom) Tested-by. No NAKs, no concerns, no stable discussion.
**Step 4.2: Recipients**
- `b4 dig -c ... -w`: Justin Chen, Al Cooper, Broadcom kernel feedback
list, Greg KH, linux-usb, LKML. Correct mailing lists and Broadcom
maintainers were CC'd.
**Step 4.3: Bug report**
- No `Reported-by:` tag, no syzbot/bugzilla link. Patch is author-
initiated hardening.
**Step 4.4: Series**
- Single-patch series; no dependencies.
**Step 4.5: Stable list**
- No explicit stable nomination in the thread.
### PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: Functions modified**
- `bdc_sr_xsf()` only.
**Step 5.2: Callers**
- `bdc->sr_handler[0] = bdc_sr_xsf;` in `bdc_core.c:301`
- Called from `bdc_udc_interrupt()` in `bdc_udc.c:331` via the
sr_handler dispatch
- Context: hard IRQ handler, executes whenever hardware posts a
transfer-complete status report into the SRR ring (DMA-backed memory
read via `rmb()`)
- Record: hot path in the IRQ handler, runs on every transfer completion
**Step 5.3: Callees**
- Reads DMA-backed `sreport`, indexes `bdc_ep_array[]`, dispatches by
sr_status.
**Step 5.4: Call chain reachability**
- Any functioning USB gadget transfer on BDC-based hardware will
generate XSF status reports. The path is reachable every time a device
does USB I/O.
**Step 5.5: Similar patterns**
- Multiple "validate endpoint index" siblings exist in USB gadget UDC
drivers, all have gone through stable:
- `ee0d382feb44` usb: gadget: aspeed_udc: validate endpoint index — in
6.1.y, 6.6.y, 6.12.y
- `ce9daa2efc08` usb: gadget: fsl_qe_udc: validate endpoint index — in
pending-6.6
- `f880aac8a57e` (cherry `e4c25cedbbeee`) usb: gadget: renesas_usb3:
validate endpoint index — in pending-6.6
- `7f14c7227f34` USB: gadget: validate endpoint index for xilinx udc —
has Cc: stable
- Record: consistent pattern; this is exactly the same class of fix
### PHASE 6: STABLE TREE ANALYSIS
**Step 6.1: Code in stable?**
- Checked `stable/linux-6.1.y`, `stable/linux-6.6.y`,
`stable/linux-6.12.y` — the vulnerable snippet in `bdc_sr_xsf()` is
identical to mainline pre-patch. Present since v3.19.
- Record: all active stable trees (6.1+) contain the buggy code
**Step 6.2: Backport complications**
- The surrounding function is unchanged in all stable branches; patch
applies verbatim.
- Record: clean apply expected
**Step 6.3: Related fixes already in stable?**
- None. The other UDC driver "validate endpoint" fixes target different
files.
### PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1: Subsystem**
- `drivers/usb/gadget/udc/bdc/` — USB gadget device controller driver
for Broadcom STB SoCs
- Criticality: PERIPHERAL — specific Broadcom SoC hardware
- Record: niche driver but active; used on Broadcom STB/set-top box
platforms
**Step 7.2: Activity**
- Low-churn driver; mostly cleanup commits in the past years, no major
refactors.
### PHASE 8: IMPACT / RISK
**Step 8.1: Affected users**
- Users of BDC-based Broadcom hardware (ARM STB platforms with USB
gadget).
- Record: driver-specific; small-to-moderate population
**Step 8.2: Trigger conditions**
- `num_eps` is derived from hardware registers
(`BDC_FSCNIC`/`BDC_FSCNOC`, `NUM_NCS() = val >> 28`, so 4-bit
quantities). For any BDC variant with fewer than 30 configurable
endpoints, `num_eps < 32` and the 5-bit `ep_num` space can overflow
the array. Triggering requires the hardware/firmware to post a status
report with an unexpected `ep_num`, or DMA corruption, or a stale
entry in the SRR.
- Record: not user-triggerable directly; triggered by hardware/firmware
behavior
**Step 8.3: Failure mode severity**
- OOB read of a `struct bdc_ep *` pointer beyond the kzalloc'd array →
dereference at `ep->flags` of arbitrary adjacent heap data. Potential
outcomes:
- If read yields 0: caught by `!ep` check (benign).
- If read yields non-NULL garbage: kernel dereferences it → oops /
crash / info leak / possibly exploitable.
- Severity: MEDIUM-HIGH (kernel OOB read + potential crash in IRQ
context)
**Step 8.4: Risk-benefit**
- Benefit: eliminates a real OOB array read in an IRQ handler of a long-
lived driver; matches a widely-backported pattern for UDC drivers.
- Risk: 4-line additive guard on an error path; cannot affect valid code
flow.
- Record: very favorable risk/benefit
### PHASE 9: SYNTHESIS
**Evidence for backport:**
- Real OOB array access (array dynamically sized to `num_eps < 32`,
index from 5-bit field 0..31)
- Small, obviously correct 4-line fix
- Reviewed by the Broadcom maintainer (Florian Fainelli)
- Tested by a Broadcom engineer (Justin Chen)
- Applied by the USB subsystem maintainer (Greg KH)
- Clean apply to all active stable trees
- Strong and consistent precedent: identical "validate endpoint index"
patches in `aspeed_udc`, `fsl_qe_udc`, `renesas_usb3`, `xilinx udc`
have all been taken into stable, many without `Fixes:` tags
- Latent since v3.19; present in every supported stable tree
**Evidence against:**
- No concrete crash report / `Reported-by:` / syzbot link
- Hardware/firmware-triggered rather than userspace-triggered
- Driver is peripheral (Broadcom STB hardware)
**Stable rules checklist:**
1. Obviously correct and tested? YES (reviewed and tested by the
hardware vendor)
2. Fixes a real bug? YES (OOB array read potential)
3. Important issue (crash/security)? YES (kernel OOB in IRQ context)
4. Small and contained? YES (+4 lines)
5. No new features/APIs? YES
6. Applies cleanly? YES
**Exception category:** hardware hardening fix — matches the well-
established "validate endpoint index" pattern for USB UDC drivers.
## Verification
- [Phase 1] Parsed all tags via `git show a402532ab8556`: Reviewed-by
Florian Fainelli (Broadcom), Tested-by Justin Chen (Broadcom), Signed-
off-by Greg KH. No Fixes/Cc-stable/Reported-by.
- [Phase 2] Diff analysis: 4 lines added at `bdc_ep.c:1650`, bounds
check `if (ep_num >= bdc->num_eps)` before `ep =
bdc->bdc_ep_array[ep_num]`.
- [Phase 2] Confirmed `num_eps` derivation in
`drivers/usb/gadget/udc/bdc/bdc_core.c:392-400`: `num_ieps + num_oeps
+ 2`, where each comes from `NUM_NCS()` (4-bit hardware register
field). `num_eps` range ≤ 32.
- [Phase 3] `git blame -L 1646,1656
drivers/usb/gadget/udc/bdc/bdc_ep.c`: buggy code from `efed421a94e62`
(2014).
- [Phase 3] `git describe --contains efed421a94e62` →
`v3.19-rc1~80^2~32^2~37` (v3.19 kernel).
- [Phase 4] `b4 dig -c a402532ab8556`: original submission found.
- [Phase 4] `b4 dig -c a402532ab8556 -a`: only v1 exists.
- [Phase 4] `b4 dig -c a402532ab8556 -w`: correct Broadcom maintainers
CC'd.
- [Phase 4] Read `/tmp/bdc/thread.mbox`: Florian Fainelli Reviewed-by,
Justin Chen Tested-by, no concerns raised.
- [Phase 5] `git grep sr_handler`: caller is `bdc_udc_interrupt()` in
`drivers/usb/gadget/udc/bdc/bdc_udc.c:331`. Runs in hard IRQ context
on every transfer completion.
- [Phase 5] Sibling "validate endpoint index" patches identified:
`ee0d382feb44` aspeed, `ce9daa2efc08` fsl_qe_udc, `f880aac8a57e`
renesas_usb3, `7f14c7227f34` xilinx.
- [Phase 6] `git show
stable/linux-6.1.y:drivers/usb/gadget/udc/bdc/bdc_ep.c`, same for
6.6.y and 6.12.y: identical pre-patch code present — patch applies
cleanly to all active stable trees.
- [Phase 6] `git log --grep="aspeed_udc: validate endpoint"
stable/linux-{6.1,6.6,6.12}.y`: confirmed identical-pattern aspeed_udc
patch was backported to all three stable trees despite having no
Fixes: tag and no Cc: stable.
- [Phase 8] Failure mode: OOB read of `struct bdc_ep *` beyond kzalloc'd
array in IRQ context → potential oops when the garbage pointer is
dereferenced at `ep->flags`; severity MEDIUM-HIGH.
- UNVERIFIED: I did not confirm specific BDC hardware variants ship with
`num_eps < 32`, though the register encoding clearly allows it and the
commit message plus hardware-vendor Tested-by imply it is realistic.
The precedent-based reasoning does not depend on this detail.
The fix is a minimal, obviously-correct bounds check that prevents a
real out-of-bounds array read in an interrupt handler of a driver
present since v3.19. It has been reviewed by the subsystem maintainer,
tested by the hardware vendor, and precisely matches a pattern
("validate endpoint index for <udc>") that stable has consistently
accepted, including patches without `Fixes:` tags.
**YES**
drivers/usb/gadget/udc/bdc/bdc_ep.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c
index c0ab3347059a0..a7a22e5ec47ba 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_ep.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
@@ -1647,6 +1647,10 @@ void bdc_sr_xsf(struct bdc *bdc, struct bdc_sr *sreport)
u8 ep_num;
ep_num = (le32_to_cpu(sreport->offset[3])>>4) & 0x1f;
+ if (ep_num >= bdc->num_eps) {
+ dev_err(bdc->dev, "xsf for invalid ep %u\n", ep_num);
+ return;
+ }
ep = bdc->bdc_ep_array[ep_num];
if (!ep || !(ep->flags & BDC_EP_ENABLED)) {
dev_err(bdc->dev, "xsf for ep not enabled\n");
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 7.0-5.10] usb: usbip: fix integer overflow in usbip_recv_iso()
[not found] <20260428104133.2858589-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-04-28 10:40 ` [PATCH AUTOSEL 7.0-5.10] usb: gadget: bdc: validate status-report endpoint indices Sasha Levin
@ 2026-04-28 10:41 ` Sasha Levin
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-5.10] usb: usbip: validate iso frame actual_length " Sasha Levin
5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-04-28 10:41 UTC (permalink / raw)
To: patches, stable
Cc: Kelvin Mbogo, Greg Kroah-Hartman, Sasha Levin, valentina.manea.m,
shuah, linux-usb, linux-kernel
From: Kelvin Mbogo <addcontent08@gmail.com>
[ Upstream commit 1897852293faca4c2be51e0a19f739622f771623 ]
usbip_recv_iso() computes the iso descriptor buffer size as:
int size = np * sizeof(*iso);
where np comes straight from the wire (urb->number_of_packets, set by
usbip_pack_ret_submit() before we get here). With np = 0x10000001 and
sizeof(*iso) == 16 the product is 0x100000010 which truncates to 16 on
a 32-bit int. kzalloc(16) succeeds but the following receive loop
writes np * 16 bytes into it - game over.
USBIP_MAX_ISO_PACKETS (1024) already exists in usbip_common.h for the
submit path but was never enforced on the receive side.
Clamp np to [1, USBIP_MAX_ISO_PACKETS] and switch to kcalloc() so
the allocator itself can catch overflows in the future. Fold the
existing np == 0 early return into the new bounds check.
usbip_pack_ret_submit() already copied the bogus np into
urb->number_of_packets before we run, so just returning -EPROTO is
not enough - processcompl() in the HCD will still iterate that many
iso_frame_desc entries when it completes the failed URB. Zero out
urb->number_of_packets before bailing to prevent that secondary crash
(confirmed on 6.12.0, processcompl+0x63 with CR2 in unmapped slab).
Signed-off-by: Kelvin Mbogo <addcontent08@gmail.com>
Link: https://patch.msgid.link/20260325103640.8090-1-addcontent08@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:
Based on my investigation, I now have enough information to complete the
analysis.
## Phase 1: Commit Message Forensics
**Step 1.1 - Subject line parsing:**
Record: Subsystem `usb: usbip:`, action verb `fix`, summary: "fix
integer overflow in usbip_recv_iso()"
**Step 1.2 - Tags parsed:**
Record:
- `Signed-off-by: Kelvin Mbogo` (author)
- `Link:
https://patch.msgid.link/20260325103640.8090-1-addcontent08@gmail.com`
- `Signed-off-by: Greg Kroah-Hartman` (subsystem maintainer applied it)
- No Fixes: tag, no explicit Cc: stable (but mbox shows v2 "Drop
security@kernel.org CC" — the author originally reported this as a
security issue)
**Step 1.3 - Body analysis:**
Record: Integer overflow in `size = np * sizeof(*iso)` when `np` (wire-
supplied `urb->number_of_packets`) is large enough. Example: `np =
0x10000001 * 16 = 0x100000010` truncates to 16 on 32-bit int. Small
`kzalloc(16)` is followed by a receive loop that writes `np * 16` bytes
= massive heap overflow. Also mentions secondary crash reproduced on
6.12.0 at `processcompl+0x63`.
**Step 1.4 - Hidden bug fix detection:**
Record: Not hidden — explicitly labeled "fix integer overflow". Real
security bug (heap overflow).
## Phase 2: Diff Analysis
**Step 2.1 - Inventory:**
Record: 1 file (`drivers/usb/usbip/usbip_common.c`), +15/-5 lines,
single function `usbip_recv_iso()`. Classification: surgical single-file
fix.
**Step 2.2 - Code flow change:**
Record: Before: `size = np * sizeof(*iso)` computed before any
validation, with only `np == 0` check. After: clamp `np` to `[1,
USBIP_MAX_ISO_PACKETS]` range, zero `urb->number_of_packets` on error
(prevents processcompl from iterating OOB in `iso_frame_desc[]`), switch
`kzalloc` to `kcalloc` for overflow-safe allocation.
**Step 2.3 - Bug mechanism:**
Record: Integer overflow in multiplication → undersized heap allocation
→ OOB heap write in receive loop (category: buffer overflow / memory
safety). Also fixes cascading NULL deref in `processcompl()` via
`urb->number_of_packets` reset.
**Step 2.4 - Fix quality:**
Record: Fix is obviously correct. Uses already-existing
`USBIP_MAX_ISO_PACKETS` (1024) from `usbip_common.h` that was already
enforced on the submit path in `stub_rx.c:381`. Mirrors existing
validation pattern. No regression risk: bounds check is strictly tighter
than kzalloc behavior.
## Phase 3: Git History Investigation
**Step 3.1 - Blame:**
Record: The buggy code (`int size = np * sizeof(*iso)`) has been in
`usbip_recv_iso()` since the function was first introduced in commit
`05a1f28e879e3` ("Staging: USB/IP: add common functions needed",
2008-07-09). The bug has existed for ~18 years.
**Step 3.2 - Follow Fixes: tag:**
Record: No Fixes: tag in the commit (the bug predates git history
cleanup). Confirmed original buggy introduction in 2008.
**Step 3.3 - File history:**
Record: The follow-up series has three related fixes (`1897852293fac`,
`591c1d972d8f1`, `74a2287209a85`) plus a later independent fix
`2ab833a16a825` ("usbip: validate number_of_packets in
usbip_pack_ret_submit()") which carries **explicit `Cc: stable
<stable@kernel.org>` and `Acked-by: Shuah Khan` (usbip maintainer)**.
Commit 2ab833a16a825 explicitly references Kelvin Mbogo's series as
complementary.
**Step 3.4 - Author's other commits:**
Record: Author Kelvin Mbogo submitted a 3-patch security series. Patch
went through v1→v2 with review from Greg KH (USB maintainer). This
specific commit is self-contained and standalone.
**Step 3.5 - Dependencies:**
Record: Uses `USBIP_MAX_ISO_PACKETS` macro which exists in all stable
trees (verified in 5.10.y through 6.19.y). No dependencies. Standalone.
## Phase 4: Mailing List Research
**Step 4.1 - Original discussion (b4 dig):**
Record:
- `b4 dig -c 1897852293faca` → found at `https://lore.kernel.org/all/202
60325104841.8282-1-addcontent08@gmail.com/`
- `b4 dig -a` → patch went through v1→v2; v2 is what was applied
- v2 changelog mentions: "Drop security@kernel.org CC" — proving the
author initially reported this through the security channel
**Step 4.2 - Reviewers:**
Record: Greg KH (USB maintainer) reviewed and applied; Shuah Khan (usbip
maintainer) acked the follow-up patch that explicitly mentions this
series and is marked for stable.
**Step 4.3 - Bug report:**
Record: Multiple independent security researchers have reported related
usbip vulnerabilities in this area (Kelvin Mbogo, Nathan Rebello,
Sebastián Alba Vives). Nathan Rebello's patch confirms "KASAN confirmed
this with kernel 7.0.0-rc5: BUG: KASAN: slab-out-of-bounds in
usbip_recv_iso+0x46a/0x640, Write of size 4 at addr ffff888106351d40".
The commit message confirms reproduction on 6.12.0.
**Step 4.4 - Related patches:**
Record: This is patch 1/3 of a series. Patches 2 and 3 have been applied
as `591c1d972d8f1` and `74a2287209a85`. The companion commit
`2ab833a16a825` by Nathan Rebello has explicit `Cc: stable`.
**Step 4.5 - Stable list:**
Record: Sebastián Alba Vives posted this category of issue as
`[SECURITY]` on the stable mailing list
(spinics.net/lists/stable/msg928028.html), describing the vulnerability
as causing "OOB memmove that corrupts kernel heap memory. No
authentication required."
## Phase 5: Code Semantic Analysis
**Step 5.1-5.4 - Callers:**
Record: `usbip_recv_iso()` is called from:
- `vhci_rx.c:86` (`vhci_recv_ret_submit` → invoked from the vhci_rx
kthread)
- `stub_rx.c:605`
- `vudc_rx.c:173`
Call chain from userspace: User creates a VHCI device via sysfs
(`attach` command), passes a TCP socket, vhci_rx kthread reads PDUs from
the socket → `vhci_recv_ret_submit()` → `usbip_pack_ret_submit()` copies
`number_of_packets` from wire → `usbip_recv_iso()` computes size with
overflow → OOB heap write. **The bug is reachable over the network with
no authentication.**
**Step 5.5 - Similar patterns:**
Record: `stub_rx.c:379-386` already validates `number_of_packets`
against `USBIP_MAX_ISO_PACKETS` on the CMD_SUBMIT path. This commit
applies the symmetric validation that was missing on the RET_SUBMIT
path.
## Phase 6: Cross-Referencing Stable Trees
**Step 6.1 - Code exists in stable:**
Record: Verified identical buggy code in every stable tree:
`linux-5.10.y`, `linux-5.15.y`, `linux-6.1.y`, `linux-6.6.y`,
`linux-6.12.y`, `linux-6.18.y`, `linux-6.19.y`. Bug has been present
since staging era (2008).
**Step 6.2 - Backport complications:**
Record: `git apply --check --3way` confirms patch applies cleanly to
current tree. Since all stable branches have identical code, the patch
will apply cleanly with no conflicts.
**Step 6.3 - Related fixes in stable:**
Record: No related fix already in stable. `USBIP_MAX_ISO_PACKETS` exists
in all stable trees (1024) so the fix uses an already-present constant.
## Phase 7: Subsystem Context
**Step 7.1 - Subsystem:**
Record: `drivers/usb/usbip/` — USB/IP network-attached USB. Criticality:
IMPORTANT (network-reachable code path, security-sensitive). Used by
users with USB-over-IP functionality; enabled in many distros.
**Step 7.2 - Activity:**
Record: Actively maintained; multiple security fixes in 2026 (this
series plus independent Nathan Rebello and Sebastián Alba Vives
contributions).
## Phase 8: Impact and Risk Assessment
**Step 8.1 - Who is affected:**
Record: Any user who runs vhci-hcd (USB/IP client) and attaches to an
untrusted USB/IP server. Also stub-side and vudc-side code paths.
Config-dependent on `CONFIG_USBIP_CORE`.
**Step 8.2 - Trigger conditions:**
Record: A malicious USB/IP server sends a RET_SUBMIT PDU with crafted
`number_of_packets = 0x10000001` or similar. **No authentication
required — reachable entirely from the network peer.**
**Step 8.3 - Failure mode severity:**
Record: **CRITICAL.** Heap out-of-bounds write with controlled size and
partially controlled content (iso descriptor bytes received from
network). Confirmed by KASAN report. The author reproduced a secondary
NULL deref crash in `processcompl()` at CR2 in unmapped slab. Security
vulnerability exploitable over network.
**Step 8.4 - Risk/benefit:**
Record:
- Benefit: **Very high** — fixes a network-reachable heap overflow;
security vulnerability.
- Risk: **Very low** — 20-line change using an already-existing
constant; bounds checks are strictly tighter than existing behavior;
no API changes.
- Ratio: Strongly favorable for backport.
## Phase 9: Final Synthesis
**Evidence FOR backporting:**
- Security vulnerability: integer overflow → heap OOB write, reachable
from malicious network peer
- Small, surgical fix (1 file, +15/-5 lines) using existing
`USBIP_MAX_ISO_PACKETS` constant
- Original submission was sent via `security@kernel.org` (per v2
changelog)
- Reviewed and applied by Greg KH (USB maintainer)
- Companion patch (2ab833a16a825) has explicit `Cc: stable` and `Acked-
by: Shuah Khan` (usbip maintainer) and references this series
- Independent security researchers (Nathan Rebello, Sebastián Alba
Vives) confirmed related vulnerabilities via KASAN and manual audit
- KASAN confirmed heap OOB write in this exact function
- Author also fixes a cascading NULL deref in `processcompl()` (verified
at kernel 6.12.0)
- Buggy code is identical in every stable tree (5.10.y – 7.0.y)
- Patch applies cleanly to stable (verified with `git apply --check`)
**Evidence AGAINST backporting:**
- None.
**Stable rules checklist:**
1. Obviously correct and tested? **YES** — clamp to existing limit,
kcalloc replaces kzalloc
2. Real bug affecting users? **YES** — heap OOB write, KASAN-confirmed
3. Important issue? **YES** — CRITICAL security vulnerability, network-
reachable
4. Small and contained? **YES** — 20 lines in 1 file, 1 function
5. No new features? **YES** — pure defensive validation
6. Applies to stable? **YES** — verified clean apply
## Verification
- [Phase 1] `git show 1897852293fac`: confirmed full commit message, no
Fixes: tag, no explicit Cc: stable in final version
- [Phase 2] Read diff in `drivers/usb/usbip/usbip_common.c`: confirmed
+15/-5 surgical change in `usbip_recv_iso()`
- [Phase 3] `git log
-L:usbip_recv_iso:drivers/usb/usbip/usbip_common.c`: buggy code traced
to original commit `05a1f28e879e3` (2008-07-09, "Staging: USB/IP: add
common functions needed")
- [Phase 3] `git show 2ab833a16a825`: related follow-up has explicit
`Cc: stable <stable@kernel.org>` and `Acked-by: Shuah Khan
<skhan@linuxfoundation.org>`
- [Phase 4] `b4 dig -c 1897852293faca`: found lore thread at `https://lo
re.kernel.org/all/20260325104841.8282-1-addcontent08@gmail.com/`
- [Phase 4] `b4 dig -a`: confirmed v1→v2 progression; v2 applied is the
latest
- [Phase 4] mbox thread inspection: v2 changelog says "Drop
security@kernel.org CC" — confirming security channel origin; Greg KH
reviewed Nathan Rebello's complementary patch
- [Phase 4] Web search confirmed multiple security researchers (Kelvin
Mbogo, Nathan Rebello, Sebastián Alba Vives) reported related
vulnerabilities; Nathan's patch cites KASAN slab-out-of-bounds report
- [Phase 5] `rg usbip_recv_iso`: confirmed 3 call sites (vhci_rx.c,
stub_rx.c, vudc_rx.c), all invoked from kthread loops reading network
PDUs
- [Phase 5] Read `vhci_rx.c:vhci_rx_pdu`: confirmed trigger path from
TCP socket → `vhci_recv_ret_submit` → `usbip_recv_iso`, no
authentication required
- [Phase 5] `rg USBIP_MAX_ISO_PACKETS`: confirmed constant exists in
`usbip_common.h:130` and is used symmetrically in `stub_rx.c:381`
- [Phase 6] Verified buggy code is identical in stable-
push/linux-{5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y, 6.18.y, 6.19.y} and
`USBIP_MAX_ISO_PACKETS` exists in all of them
- [Phase 6] `git apply --check`: patch applies cleanly to current HEAD
(stable/linux-7.0.y)
- [Phase 8] Failure mode verified as heap OOB write, KASAN-confirmed by
Nathan Rebello on kernel 7.0.0-rc5; secondary NULL deref in
`processcompl()` confirmed on 6.12.0
This commit is a textbook YES for stable: it fixes a network-reachable,
unauthenticated heap buffer overflow present in every supported stable
tree. The fix is minimal, uses an already-existing constant, was vetted
by the USB maintainer, and has been reproduced with KASAN by independent
researchers. The companion patch in the same area explicitly carries
`Cc: stable`.
**YES**
drivers/usb/usbip/usbip_common.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index 29d776fbeef74..fd620e9600394 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -662,7 +662,7 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
void *buff;
struct usbip_iso_packet_descriptor *iso;
int np = urb->number_of_packets;
- int size = np * sizeof(*iso);
+ int size;
int i;
int ret;
u32 total_length = 0;
@@ -670,11 +670,21 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
if (!usb_pipeisoc(urb->pipe))
return 0;
- /* my Bluetooth dongle gets ISO URBs which are np = 0 */
- if (np == 0)
- return 0;
+ if (np <= 0 || np > USBIP_MAX_ISO_PACKETS) {
+ dev_err(&urb->dev->dev,
+ "recv iso: invalid number_of_packets %d\n", np);
+ /*
+ * usbip_pack_ret_submit() already set urb->number_of_packets
+ * from the wire. Zero it so processcompl() does not iterate
+ * OOB descriptors on the way out.
+ */
+ urb->number_of_packets = 0;
+ return -EPROTO;
+ }
+
+ size = np * sizeof(*iso);
- buff = kzalloc(size, GFP_KERNEL);
+ buff = kcalloc(np, sizeof(*iso), GFP_KERNEL);
if (!buff)
return -ENOMEM;
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 7.0-5.10] usb: usbip: validate iso frame actual_length in usbip_recv_iso()
[not found] <20260428104133.2858589-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-04-28 10:41 ` [PATCH AUTOSEL 7.0-5.10] usb: usbip: fix integer overflow in usbip_recv_iso() Sasha Levin
@ 2026-04-28 10:41 ` Sasha Levin
5 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2026-04-28 10:41 UTC (permalink / raw)
To: patches, stable
Cc: Kelvin Mbogo, Greg Kroah-Hartman, Sasha Levin, valentina.manea.m,
shuah, linux-usb, linux-kernel
From: Kelvin Mbogo <addcontent08@gmail.com>
[ Upstream commit 591c1d972d8f19862ecd7279c7ef4df48b0a9b33 ]
usbip_recv_iso() sums each frame's actual_length into an int
accumulator without checking the individual values first:
total_length += urb->iso_frame_desc[i].actual_length;
A malicious server can send actual_length = 0xFFFFFFFC for one frame
and a small value for the other, making the signed sum wrap around to
match urb->actual_length. The sanity check passes, and usbip_pad_iso()
later computes a negative actualoffset, feeding it to memmove() as a
source pointer - reads before the allocation, leaked to userspace via
USBDEVFS_REAPURB.
Reject any frame whose actual_length exceeds transfer_buffer_length
(one frame can't carry more data than the whole buffer), and widen the
accumulator to u32 so that many moderately-large frames can't wrap it
either.
Signed-off-by: Kelvin Mbogo <addcontent08@gmail.com>
Link: https://patch.msgid.link/20260325103640.8090-2-addcontent08@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:
I have enough information to make an informed decision. Let me now
compile the comprehensive analysis.
## Phase 1: Commit Message Forensics
**Step 1.1 - Subject Line**
- Subsystem: `usb: usbip` (USB/IP driver)
- Action verb: `validate` (strong bug-fix indicator, even without "fix")
- Record: `[usb/usbip] [validate/harden] ISO frame actual_length in
usbip_recv_iso() to prevent integer wraparound and downstream OOB
read`
**Step 1.2 - Tags Present**
- `Signed-off-by: Kelvin Mbogo` (author)
- `Link:
https://patch.msgid.link/20260325103640.8090-2-addcontent08@gmail.com`
(patch 2 of a series)
- `Signed-off-by: Greg Kroah-Hartman` (USB maintainer – strong trust
signal)
- No `Fixes:` tag (expected for pre-git-era code in staging origin)
- No `Cc: stable` (expected; that's why it's under review)
- Record: Greg KH SOB confirms maintainer applied it; Link shows this is
patch "-2" of the series.
**Step 1.3 - Body Analysis**
- Bug mechanism: `total_length` is `int` (signed) accumulator; per-frame
`actual_length` is from the wire (attacker-controlled `unsigned int`)
- Exploit: server sends `actual_length = 0xFFFFFFFC` + small value;
signed sum wraps to match `urb->actual_length`, passing the sanity
check
- Downstream impact: `usbip_pad_iso()` computes negative `actualoffset`
→ `memmove()` reads before allocation → data leaked to userspace via
`USBDEVFS_REAPURB`
- Fix approach: (1) reject any frame whose `actual_length >
transfer_buffer_length`; (2) widen accumulator from `int` to `u32`
- Record: Concrete, reproducible kernel info-leak from malicious USBIP
server; commit body explains both root cause and fix clearly.
**Step 1.4 - Hidden Fix Detection**
- "validate" = adds missing input validation = bug fix
- Record: Not hidden - this is an explicit security hardening fix.
## Phase 2: Diff Analysis
**Step 2.1 - Inventory**
- Single file: `drivers/usb/usbip/usbip_common.c`
- +12 / -3 (15 changed lines total)
- Single function modified: `usbip_recv_iso()`
- Classification: small, single-file surgical fix
- Record: Minimal scope; textbook stable-appropriate patch size.
**Step 2.2 - Code-Flow Change**
- Before: `int total_length = 0; ... total_length +=
urb->iso_frame_desc[i].actual_length;` then `if (total_length !=
urb->actual_length)`
- After: `u32 total_length = 0;` + per-iteration `if
(iso_frame_desc[i].actual_length > transfer_buffer_length) return
-EPROTO;` + cast comparison to u32
- Affected path: RET_SUBMIT handling in vhci-hcd receive path (network-
sourced data)
- Record: Hardens the receive path for untrusted network input.
**Step 2.3 - Bug Mechanism Category**
- (d) Memory safety + integer overflow: `u32` widening prevents signed
accumulator wrap; bounds check prevents any single-frame value from
being > buffer capacity
- Secondary: (g) Logic correctness — format specifier `%d` → `%u` and
explicit `(u32)` cast make the comparison semantically correct
- Record: Classic "validate untrusted input from the wire" pattern —
same class as the 2016 commit `b348d7dddb6c4` in the same file.
**Step 2.4 - Fix Quality**
- Obviously correct: bounds check is simple (`actual_length >
transfer_buffer_length → reject`), u32 widening cannot introduce new
overflow behavior in a non-negative accumulation
- No regression risk: rejects only genuinely malformed input; legitimate
clients never produce frames larger than the whole buffer
- Record: High-quality, minimal, safe fix.
## Phase 3: Git History Investigation
**Step 3.1 - Blame**
- `usbip_recv_iso()` was introduced by `05a1f28e879e3` ("Staging:
USB/IP: add common functions needed", July 2008)
- Record: Vulnerable code present since 2.6.28 — affects every single
active stable tree.
**Step 3.2 - Fixes: Tag**
- No Fixes: tag present; the buggy pattern dates to the original 2008
import (pre-git staging era for USBIP)
- Record: N/A — but effectively "Fixes: 05a1f28e879e3" which predates
all stable branches.
**Step 3.3 - File History**
Recent commits touching `drivers/usb/usbip/usbip_common.c`:
- `2ab833a16a825` usbip: validate number_of_packets in
usbip_pack_ret_submit() **[has `Cc: stable`]**
- `74a2287209a85` usb: usbip: fix OOB read/write in usbip_pad_iso()
(patch 3/3 of series)
- `591c1d972d8f1` **← TARGET** (patch 2/3)
- `1897852293fac` usb: usbip: fix integer overflow in usbip_recv_iso()
(patch 1/3)
- Record: Part of a 3-patch hardening series against malicious USBIP
server. Companion patch from Nathan Rebello (`2ab833a16a825`)
explicitly has `Cc: stable`, confirming maintainers view this cluster
of bugs as stable-worthy.
**Step 3.4 - Author Context**
- Kelvin Mbogo: new contributor sending hardening patches; patches
vetted by Greg KH with v1→v2 rework
- Greg KH (USB maintainer) applied all three patches
- Record: Proper maintainer review chain, v2 addressed review feedback.
**Step 3.5 - Prerequisite Check**
- The diff hunks in `591c1d972d8f1` (the target) only touch:
1. `int total_length = 0;` → `u32 total_length = 0;`
2. Add per-frame actual_length bounds check
3. Cast/format the comparison
- I verified that stable/linux-6.19.y (and all older stable branches)
still contain `int total_length = 0;` and the unmodified loop/check —
so the patch hunks CAN apply standalone without patch 1/3
- However, this patch is part of a security cluster; ideally patches
1/3, 2/3, 3/3 + Nathan's `2ab833a16a825` all get backported together
- Record: Patch applies cleanly to stable without dependencies, though
full cluster is the complete fix.
## Phase 4: Mailing List Research
**Step 4.1 - Original Thread (b4 dig)**
- `b4 dig -c 591c1d972d8f1` found match at `https://lore.kernel.org/all/
20260325103640.8090-2-addcontent08@gmail.com/`
- `b4 dig -a` confirmed series evolution: v1 → v2 (applied version is
v2)
- v2 changes per Kelvin: "Drop Reported-by (author is signer)"
- Record: v2 is the applied version, v1 received feedback from Greg KH
and was refined.
**Step 4.2 - Reviewers (b4 dig -w)**
- Originally CC'd: `linux-usb@vger.kernel.org`,
`gregkh@linuxfoundation.org`, `skhan@linuxfoundation.org` (USBIP
maintainer)
- Both the USB subsystem maintainer (Greg KH) and the USBIP subsystem
maintainer (Shuah Khan) were in loop
- Record: Proper maintainer review coverage.
**Step 4.3 - Bug Report**
- No explicit Reported-by/syzbot on v2 (author is signer)
- Exploit scenario clearly described in commit message with concrete
payload (`0xFFFFFFFC`) and path to userspace via `USBDEVFS_REAPURB`
- Record: Author-discovered security issue; mechanism is well-documented
in commit body.
**Step 4.4 - Related Patches**
- Thread shows all three patches of the series (patches 1/3, 2/3, 3/3 by
Kelvin)
- Companion patch from Nathan Rebello posted in same thread got `Acked-
by: Shuah Khan` and `Cc: stable@vger.kernel.org`, submitted separately
as `2ab833a16a825`
- Greg KH explicitly asked Nathan to "submit it separately, on top of
that series, to make it easier to review and apply"
- Record: Maintainers treat this as a coordinated security cluster;
Nathan's companion patch explicitly nominated for stable.
**Step 4.5 - Stable List**
- Not individually discussed on stable list (fix flew under radar of
formal stable nomination process for 1/3, 2/3, 3/3 — only the
separately-submitted Nathan patch has Cc: stable)
- Record: This is exactly the kind of fix that SHOULD be caught by
autosel review.
## Phase 5: Code Semantic Analysis
**Step 5.1 - Functions Modified**
- `usbip_recv_iso()` (single function changed)
**Step 5.2 - Callers**
- `drivers/usb/usbip/vhci_rx.c:86` → called from
`vhci_recv_ret_submit()` (vhci-hcd RX path, client side)
- `drivers/usb/usbip/stub_rx.c:605` → stub_rx RET_SUBMIT handling
(server side)
- `drivers/usb/usbip/vudc_rx.c:173` → VUDC receive path
- Record: Called from every USBIP RET_SUBMIT receive path. Critical path
for any USBIP user.
**Step 5.3 - Callees**
- `usbip_pack_iso()` — deserialize ISO frame descriptor
- `usbip_iso_packet_correct_endian()` — byte-order conversion
- `usbip_recv()` — TCP socket recv
- Record: Pure data-processing function for wire-format data.
**Step 5.4 - Reachability**
- Call chain: user runs `usbip attach` (CAP_SYS_ADMIN) → vhci-hcd
connects to USBIP server → kernel RX thread `vhci_rx_loop` →
`vhci_recv_ret_submit` → `usbip_recv_iso` → on return, `usbip_pad_iso`
→ memmove into user-readable buffer → userspace `USBDEVFS_REAPURB`
reads kernel memory
- Record: Directly reachable from the network when connected to a
malicious USBIP server — all the leaked data then reaches userspace.
**Step 5.5 - Similar Patterns**
- `usbip_recv_xbuff()` already validates `size >
urb->transfer_buffer_length` (from commit `b348d7dddb6c4`, 2016) —
this patch applies the same defensive pattern to ISO frames
- Record: Patch extends an established validation pattern already
accepted in the same file.
## Phase 6: Cross-Referencing Stable Trees
**Step 6.1 - Vulnerable Code in Stable**
- Verified via `git show
stable/linux-6.19.y:drivers/usb/usbip/usbip_common.c` that the
unpatched vulnerable code (`int total_length`, no per-frame bounds
check) is present
- Code unchanged since 2008 staging import → present in 5.10.y, 5.15.y,
6.1.y, 6.6.y, 6.12.y, 6.17.y (if active), 6.18.y, 6.19.y
- Record: All active stable trees contain the bug.
**Step 6.2 - Backport Complications**
- None of prerequisite patch 1/3 (`1897852293fac`) or companion 3/3
(`74a2287209a85`) is in any stable tree yet
- The specific hunks of this patch do not depend on 1/3 — they modify
`int total_length` which exists in all stable trees
- Expected: clean apply
- Record: Should apply cleanly to all stable branches; the full security
benefit requires also backporting 1/3 and 3/3.
**Step 6.3 - Related Fixes in Stable**
- None yet — this entire USBIP security cluster is fresh (March-April
2026)
- Record: No conflicting/duplicate fixes in stable.
## Phase 7: Subsystem Context
**Step 7.1 - Criticality**
- `drivers/usb/usbip/` = USB/IP driver (used in VM environments, remote
USB access, Android development, CI with USB test devices)
- Criticality: IMPORTANT (affects users who use USB/IP; not CORE but not
obscure)
- Record: IMPORTANT criticality — specific user population but real
attack surface.
**Step 7.2 - Activity**
- USBIP has periodic maintenance with multiple historical security fixes
(race conditions, shift OOB, buffer validation); active subsystem
- Record: Maintained subsystem with history of similar stable-worthy
hardening patches.
## Phase 8: Impact & Risk
**Step 8.1 - Who's Affected**
- Any user who runs `usbip attach` to connect to a remote USBIP server
(then connection to a compromised/malicious server exposes the client)
- Record: USBIP client users — real user population, not a theoretical
risk.
**Step 8.2 - Trigger**
- Attacker runs malicious USBIP server; user attaches to it; server
returns crafted RET_SUBMIT
- Attach requires privileged operation, but once attached, reading data
via `USBDEVFS_REAPURB` is accessible to any process with access to the
virtual device
- Record: Trigger is "connect to malicious USBIP server" — a very
realistic scenario (VM escapes, supply-chain USBIP servers,
compromised networks).
**Step 8.3 - Failure Severity**
- OOB slab read → kernel memory leaked to userspace = **information
disclosure** (potential leak of sensitive data: kernel pointers,
credentials, keys)
- Could be chained with other bugs for KASLR bypass or further
exploitation
- Severity: **HIGH** (info leak, security-relevant)
- Record: HIGH — kernel info leak to userspace via crafted network
input.
**Step 8.4 - Risk/Benefit**
- Benefit: HIGH — closes a security-relevant info leak in untrusted-
network parsing path
- Risk: VERY LOW — adds a bounds check and a type widening; cannot
produce false rejections on legitimate data (no legitimate frame can
be larger than the whole buffer); u32 widening is semantically
equivalent for non-overflowing cases
- Record: Excellent benefit/risk ratio.
## Phase 9: Final Synthesis
**Step 9.1 - Evidence Summary**
FOR backporting:
- Security fix: kernel info leak from attacker-controlled network input
to userspace
- Small (+12/-3), single-file, single-function, surgical
- Obviously correct: simple bounds check + type widening
- Vulnerable code present since 2008 — every active stable affected
- Signed off by USB maintainer Greg KH; reviewed through v1→v2 cycle
- Follows established pattern (`b348d7dddb6c4`, 2016) accepted in same
file for `usbip_recv_xbuff`
- Companion patch in the same security cluster has explicit `Cc: stable`
- Applies cleanly to stable trees even without prerequisite patch 1/3
AGAINST backporting:
- Patch 2/3 of a series; ideally 1/3 and 3/3 should also be backported
for complete defense-in-depth
- No explicit `Cc: stable` (but absence is expected — that's the autosel
context)
Unresolved:
- No CVE assigned yet (not a blocker — many pre-CVE security fixes are
backported)
**Step 9.2 - Stable Rules Checklist**
1. Obviously correct? ✅ Yes
2. Fixes real bug? ✅ Yes (kernel info leak)
3. Important? ✅ HIGH — security issue
4. Small & contained? ✅ +12/-3, single function
5. No new features? ✅ Pure input validation
6. Applies to stable? ✅ Verified: hunks match stable code
**Step 9.3 - Exception Category**
- Hardening / security validation of untrusted network input — fits
under "fixes a real bug" (not strictly an exception category, but
textbook stable material)
**Step 9.4 - Decision**
YES. The benefit (closing a kernel info-leak attackable by a malicious
USBIP server) significantly outweighs the risk (near-zero — the change
is input validation + type widening).
## Verification
- **[Phase 1]** Parsed subject and tags: Verified Link to lore, Greg KH
SOB (USB maintainer), absence of Fixes/Cc:stable expected for review
candidate.
- **[Phase 1]** Body analysis: commit explains `0xFFFFFFFC + small =
wraparound` mechanism, path to `USBDEVFS_REAPURB` info leak.
- **[Phase 2]** Ran `git show 591c1d972d8f1`: confirmed +12/-3 in single
file, single function `usbip_recv_iso()`.
- **[Phase 2]** Read `include/linux/usb.h`: verified
`iso_frame_desc[i].actual_length` is `unsigned int` (line 1418),
`urb->actual_length` is `u32` (line 1655),
`urb->transfer_buffer_length` is `u32` (line 1654) — confirms signed-
wrap bug is real.
- **[Phase 3]** `git log --follow` traced `usbip_common.c` back to
`05a1f28e879e3` (July 2008, staging import) — bug predates all stable
branches.
- **[Phase 3]** `git log origin/master --
drivers/usb/usbip/usbip_common.c`: found the 3-patch series
(1897852293fac, 591c1d972d8f1, 74a2287209a85) plus companion
`2ab833a16a825` which has `Cc: stable`.
- **[Phase 3]** Read
`stable/linux-6.19.y:drivers/usb/usbip/usbip_common.c`: confirmed
unpatched code (`int total_length`, no bounds check) is present.
- **[Phase 4]** `b4 dig -c 591c1d972d8f1`: found `https://lore.kernel.or
g/all/20260325103640.8090-2-addcontent08@gmail.com/`.
- **[Phase 4]** `b4 dig -c 591c1d972d8f1 -a`: confirmed series went v1 →
v2 (applied version is v2 after Greg's feedback).
- **[Phase 4]** `b4 dig -c 591c1d972d8f1 -w`: confirmed Greg KH and
Shuah Khan (USBIP maintainer) were CC'd.
- **[Phase 4]** Read thread mbox: confirmed Greg KH directed Nathan to
submit his companion patch separately, and that companion got `Acked-
by: Shuah Khan` + `Cc: stable`.
- **[Phase 5]** Grep on usbip directory: verified `usbip_recv_iso` is
called from `vhci_rx.c:86`, `stub_rx.c:605`, `vudc_rx.c:173`;
`usbip_pad_iso` called from `vhci_rx.c:92`.
- **[Phase 5]** Read `vhci_rx.c`: confirmed the exploit path
`usbip_recv_iso → usbip_pad_iso → memmove into transfer_buffer` that
later reaches userspace.
- **[Phase 5]** Verified precedent commit `b348d7dddb6c4` (2016) applied
exactly the same "validate untrusted length" pattern to
`usbip_recv_xbuff` in the same file.
- **[Phase 6]** Checked `stable-rc-push/linux-*.y` branches: no USBIP
security series patches applied yet.
- **[Phase 6]** Inspected hunks vs stable code: the three hunks only
touch `int total_length = 0`, the iso-decode loop, and the post-loop
comparison — all present in all stable trees. Expected clean apply
independent of patch 1/3.
- **[Phase 7]** Context: `drivers/usb/usbip/` — IMPORTANT criticality
(VM/remote USB users).
- **[Phase 8]** Severity: HIGH — kernel slab OOB read → info leak to
userspace via USBDEVFS_REAPURB, triggered by crafted USBIP server
replies.
- UNVERIFIED: Exact minor versions of 5.10.y/5.15.y/etc. weren't walked
line-by-line, but mainline unpatched state in 6.19.y + unchanged code
since 2008 staging import reliably implies bug exists in all active
stable branches.
The fix is a small, surgical, obviously-correct validation of attacker-
controlled network input that prevents a kernel information leak. It
applies cleanly to all active stable trees, is signed off by the USB
subsystem maintainer, and follows an established pattern already
accepted in the same file. The companion fix in the same security
cluster is already explicitly nominated for stable. This is textbook
stable material.
**YES**
drivers/usb/usbip/usbip_common.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/usbip/usbip_common.c b/drivers/usb/usbip/usbip_common.c
index a2b2da1255dda..29d776fbeef74 100644
--- a/drivers/usb/usbip/usbip_common.c
+++ b/drivers/usb/usbip/usbip_common.c
@@ -665,7 +665,7 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
int size = np * sizeof(*iso);
int i;
int ret;
- int total_length = 0;
+ u32 total_length = 0;
if (!usb_pipeisoc(urb->pipe))
return 0;
@@ -696,14 +696,23 @@ int usbip_recv_iso(struct usbip_device *ud, struct urb *urb)
for (i = 0; i < np; i++) {
usbip_iso_packet_correct_endian(&iso[i], 0);
usbip_pack_iso(&iso[i], &urb->iso_frame_desc[i], 0);
+ if (urb->iso_frame_desc[i].actual_length >
+ (unsigned int)urb->transfer_buffer_length) {
+ dev_err(&urb->dev->dev,
+ "recv iso: frame actual_length %u exceeds buffer %d\n",
+ urb->iso_frame_desc[i].actual_length,
+ urb->transfer_buffer_length);
+ kfree(buff);
+ return -EPROTO;
+ }
total_length += urb->iso_frame_desc[i].actual_length;
}
kfree(buff);
- if (total_length != urb->actual_length) {
+ if (total_length != (u32)urb->actual_length) {
dev_err(&urb->dev->dev,
- "total length of iso packets %d not equal to actual length of buffer %d\n",
+ "total length of iso packets %u not equal to actual length of buffer %d\n",
total_length, urb->actual_length);
if (ud->side == USBIP_STUB || ud->side == USBIP_VUDC)
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread