Linux USB
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	Sasha Levin <sashal@kernel.org>,
	andreas.noever@gmail.com, westeri@kernel.org,
	YehezkelShB@gmail.com, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.18-5.10] thunderbolt: Keep XDomain reference during the lifetime of a service
Date: Mon, 31 Aug 2026 09:23:05 -0400	[thread overview]
Message-ID: <20260831133314.4125787-157-sashal@kernel.org> (raw)
In-Reply-To: <20260831133314.4125787-1-sashal@kernel.org>

From: Mika Westerberg <mika.westerberg@linux.intel.com>

[ Upstream commit 8b4060998637f06975fceee9b73845d8672d411e ]

This is needed because we release the service ID in tb_service_release()
and the ID array is owned by the parent XDomain.

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:

## Phase 1: Commit Message Forensics

### Step 1.1: Parse the Subject Line
**Record:** `[thunderbolt]` `[keep]` — Keep an XDomain reference for the
full lifetime of a Thunderbolt service device.

### Step 1.2: Parse All Commit Message Tags
**Record:**
- **Fixes:** — absent (expected for manual review)
- **Reported-by:** — absent
- **Tested-by:** — absent
- **Reviewed-by:** — absent
- **Acked-by:** — absent
- **Link:** — absent
- **Cc: stable@vger.kernel.org** — absent (not a negative signal)
- **Signed-off-by:** Mika Westerberg \<mika.westerberg@linux.intel.com\>
  (subsystem maintainer)

No syzbot, no multi-reporter tags. Author is the Thunderbolt maintainer.

### Step 1.3: Analyze Commit Body
**Record:**
- **Bug:** `tb_service_release()` calls `ida_free(&xd->service_ids,
  ...)`, but `service_ids` is owned by the parent XDomain. The XDomain
  can be freed before the service’s final `release` callback runs.
- **Symptom:** Use-after-free when freeing the service ID during service
  teardown (potential crash / memory corruption).
- **Version info:** Not stated in the commit message.
- **Root cause (author):** Missing explicit XDomain reference for the
  service’s lifetime.

### Step 1.4: Detect Hidden Bug Fixes
**Record:** Not disguised — this is an explicit lifetime/reference-
counting bug fix, not cleanup or optimization.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory the Changes
**Record:**
- **File:** `drivers/thunderbolt/xdomain.c` (+2 lines net)
- **Functions modified:** `tb_service_release()`, `enumerate_services()`
- **Scope:** Single-file, surgical fix (2 meaningful lines)

### Step 2.2: Code Flow Change (per hunk)

**Hunk 1 — `tb_service_release()`:**
- **Before:** Frees service ID from parent XDomain’s IDA, frees service
  memory; no XDomain refcount drop.
- **After:** Same, then calls `tb_xdomain_put(xd)` to release the
  reference taken at enumeration.
- **Path:** Service device final release callback (after last
  `put_device()` on the service).

**Hunk 2 — `enumerate_services()`:**
- **Before:** `svc->dev.parent = &xd->dev` (bare pointer, no refcount).
- **After:** `svc->dev.parent = get_device(&xd->dev)` (holds XDomain
  alive).
- **Path:** XDomain service enumeration during property exchange /
  reconnect.

### Step 2.3: Bug Mechanism
**Record:** **Reference counting / use-after-free fix.**

Mechanism verified against the driver core:
1. `enumerate_services()` registers child service devices parented under
   the XDomain.
2. `tb_service_release()` accesses `xd->service_ids` via `ida_free()`.
3. `tb_xdomain_release()` destroys that IDA with
   `ida_destroy(&xd->service_ids)`.
4. On `device_unregister(service)`, `device_del()` immediately calls
   `put_device(parent)` (see `drivers/base/core.c:3983`), dropping the
   parent reference acquired in `device_add()` — even if the service
   device struct still exists because something holds an extra
   reference.
5. `tb_xdomain_remove()` unregisters all services, then unregisters the
   XDomain; the XDomain can reach refcount zero and run
   `tb_xdomain_release()` while a service device is still pending final
   release.
6. When `tb_service_release()` finally runs, `xd` and `xd->service_ids`
   may already be freed → UAF.

The fix holds an independent XDomain reference from enumeration until
`tb_service_release()`.

### Step 2.4: Fix Quality Assessment
**Record:**
- **Quality:** Obviously correct; standard `get_device()` /
  `put_device()` pairing via `tb_xdomain_put()`.
- **Scope:** Minimal; no API changes.
- **Regression risk:** Very low. Refcount is balanced: one
  `get_device()` at parent assignment, one `tb_xdomain_put()` at service
  release. `device_add()`/`device_del()` continue to manage their own
  parent reference separately.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame the Changed Lines
**Record:** `git blame` attributes current `tb_service_release()` /
`enumerate_services()` code to commit `19eef1d98eeda` in this tree
(stable history is squashed; that commit message is unrelated). The
service/XDomain code is present and has the buggy pattern. Approximate
introduction: with the XDomain service enumeration infrastructure
(present in this 6.18.y tree).

### Step 3.2: Follow the Fixes: Tag
**Record:** No `Fixes:` tag present — step not applicable.

### Step 3.3: File History for Related Changes
**Record:** Recent thunderbolt commits in this tree include XDomain
security hardening (`b5daa920f44cb`, `46da5c3ea011e`, `fcbd0cdab9283`,
etc.). This fix is **standalone** (2 lines, no structural
prerequisites). Related stable series patches (debugfs unregister,
delayed-work UAF) are separate; this commit does not depend on them.

### Step 3.4: Author's Other Commits
**Record:** Mika Westerberg is the Thunderbolt subsystem maintainer. No
other commits by this author found in this tree’s `drivers/thunderbolt/`
log (history is compressed).

### Step 3.5: Prerequisite Commits
**Record:** No dependencies. `tb_xdomain_get()`/`tb_xdomain_put()`,
`tb_service_parent()`, `enumerate_services()`, and
`ida_alloc`/`ida_free` on `xd->service_ids` all exist in this tree.
Patch applies to current `xdomain.c` with only the two line changes
(candidate diff uses `kzalloc_obj`; local tree uses `kzalloc` —
unrelated context, no conflict).

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Patch Discussion
**Record:** `b4 dig` could not be run — commit hash is not in this
checkout. lore.kernel.org returned 403 (bot protection). **Ratatoskr
stable-queue archives** show this exact patch nominated for multiple
stable trees:
- `[PATCH 5.10.y 1/3] thunderbolt: Keep XDomain reference during the
  lifetime of a service`
- `[PATCH 5.15.y 3/6] ...`
- `[PATCH 6.6.y 4/7] ...`

Part of a broader Thunderbolt XDomain stability series (`Stable-dep-of:
2c5d2d3c3f70` on related patches).

### Step 4.2: Reviewers
**Record:** UNVERIFIED — could not fetch lore thread or run `b4 dig -w`.
Author is subsystem maintainer.

### Step 4.3: Bug Report
**Record:** No external bug report referenced. Bug identified by code
analysis / disconnect teardown path.

### Step 4.4: Related Patches / Series
**Record:** Related stable patches in the same series (debugfs
unregister, remove without holding `tb->lock`, delayed-work UAF) are
complementary but **this commit is independently correct and
applicable**. Greg’s Linux 6.18.44 announcement (2026-08-09 per
Ratatoskr) suggests the broader series is heading into 6.18.y.

### Step 4.5: Stable Mailing List History
**Record:** Stable nominations confirmed via Ratatoskr for 5.10.y,
5.15.y, 6.6.y at minimum. Direct lore stable-list search UNVERIFIED
(403).

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `tb_service_release()`, `enumerate_services()`,
`tb_xdomain_remove()`, `tb_xdomain_release()`, `tb_service_parent()`.

### Step 5.2: Trace Callers
**Record:**
- `enumerate_services(xd)` — called from XDomain property update path
  (line 1497).
- `tb_service_release()` — device core release callback for
  `tb_service_type`.
- `tb_xdomain_remove()` — called on XDomain disconnect; unregisters all
  child services then the XDomain.

Callers of `tb_xdomain_remove()` include ICM and core Thunderbolt
disconnect paths — common during cable unplug / peer host disconnect.

### Step 5.3: Trace Callees
**Record:** `ida_free()`, `ida_destroy()`, `get_device()`,
`tb_xdomain_put()` (wraps `put_device()`), `device_register()`,
`device_unregister()`.

### Step 5.4: Call Chain / Reachability
**Record:**
```
Thunderbolt disconnect / XDomain removal
  → tb_xdomain_remove()
    → device_for_each_child_reverse(..., unregister_service)
      → device_unregister(service)  [parent ref dropped in device_del]
    → device_unregister(xd)
      → tb_xdomain_release()  [ida_destroy(&xd->service_ids)]
  → (later) tb_service_release()  [ida_free on possibly freed xd]  ← BUG
```
**Userspace-reachable:** Yes — triggered by Thunderbolt hot-unplug /
peer disconnect while a service device has lingering references (driver
binding, `get_device()` holders, etc.). Not theoretical.

### Step 5.5: Similar Patterns
**Record:** XDomain itself correctly uses `get_device(parent)` at
allocation (`xdomain.c:2016`). Services were the missing symmetric case.
`tb_service_get()`/`tb_service_put()` exist for service devices but did
not protect the parent XDomain.

---

## Phase 6: Cross-Referencing Against the Local Tree

### Step 6.1: Does the Buggy Code Exist?
**Record:** **YES.** Local tree is **6.18.43** (`git describe HEAD` →
`v6.18.43-1-gc7f0dac02d232`, `make kernelversion` → `6.18.43`).

Current buggy code verified:

```1006:1015:drivers/thunderbolt/xdomain.c
static void tb_service_release(struct device *dev)
{
        struct tb_service *svc = container_of(dev, struct tb_service,
dev);
        struct tb_xdomain *xd = tb_service_parent(svc);

        tb_service_debugfs_remove(svc);
        ida_free(&xd->service_ids, svc->id);
        kfree(svc->key);
        kfree(svc);
}
```

```1120:1124:drivers/thunderbolt/xdomain.c
                svc->id = id;
                svc->dev.bus = &tb_bus_type;
                svc->dev.type = &tb_service_type;
                svc->dev.parent = &xd->dev;
                dev_set_name(&svc->dev, "%s.%d", dev_name(&xd->dev),
svc->id);
```

No `get_device(&xd->dev)` on parent assignment; no `tb_xdomain_put(xd)`
in release. Fix is **not** already present (`git log --grep` and `git
log -S "svc->dev.parent = get_device"` returned nothing).

### Step 6.2: Backport Complications
**Record:** **Clean apply expected** — 2 lines in one file, matching
current code structure. No refactor conflicts in the target hunks.

### Step 6.3: Related Fixes Already Present?
**Record:** No — grep and git searches found no prior application of
this fix or equivalent `tb_xdomain_put` in `tb_service_release`.

---

## Phase 7: Subsystem and Maintainer Context

### Step 7.1: Subsystem and Criticality
**Record:** `drivers/thunderbolt/` — **IMPORTANT** (Thunderbolt/USB4
XDomain networking and device interconnection; not core kernel, but
affects real hardware on laptops/workstations).

### Step 7.2: Subsystem Activity
**Record:** Actively maintained in this tree — multiple recent XDomain
security/stability fixes (packet validation, bounds checking, property
parsing).

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users with `CONFIG_THUNDERBOLT` and active XDomain
connections (Thunderbolt networking, cross-host services). Driver-
specific but affects a widely deployed laptop/workstation feature.

### Step 8.2: Trigger Conditions
**Record:** XDomain removal/disconnect while a service child device
still has refcount > 1 after `device_unregister()`. Realistic during
hot-unplug, peer shutdown, or driver teardown races. Unprivileged users
can trigger disconnect by unplugging cable.

### Step 8.3: Failure Mode Severity
**Record:** **Use-after-free** on `xd->service_ids` during `ida_free()`
→ kernel oops / memory corruption. **Severity: HIGH to CRITICAL.**

### Step 8.4: Risk-Benefit Ratio
**Record:**
- **Benefit:** HIGH — prevents UAF crash on a real disconnect path.
- **Risk:** VERY LOW — 2-line refcount fix, maintainer-authored, already
  queued for multiple stable trees.
- **Ratio:** Strongly favors backport.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Compiled

**FOR backport:**
- Real UAF on XDomain disconnect teardown path (verified via code +
  driver-core semantics).
- Buggy code confirmed present in 6.18.43.
- Minimal, obviously correct fix (2 lines).
- Thunderbolt maintainer authorship.
- Already nominated for stable on 5.10.y, 5.15.y, 6.6.y (Ratatoskr).
- Standalone — no prerequisite commits required.
- Part of broader XDomain stability effort heading into 6.18.y.

**AGAINST backport:**
- Config-specific (`CONFIG_THUNDERBOLT`) — affects subset of users, not
  all kernels.
- No syzbot/user crash report attached (but mechanism is clear from
  code).

**UNRESOLVED:**
- Full lore review thread (403 blocked).
- Exact upstream commit SHA (not in this checkout).

Neither unresolved item weakens the technical case.

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — standard refcount pattern;
   maintainer-authored; stable-queue nominations imply review.
2. Fixes a real bug affecting users? **PASS** — UAF on Thunderbolt
   XDomain disconnect.
3. Important issue? **PASS** — UAF / potential crash (HIGH-CRITICAL).
4. Small and contained? **PASS** — 2 lines, one file.
5. No new features or APIs? **PASS** — refcount fix only.
6. Can apply to local tree? **PASS** — code exists; clean 2-line apply.

### Step 9.3: Exception Categories
**Record:** Not applicable (not a device ID, quirk, DT, build, or docs
fix — standard bug fix).

### Step 9.4: Decision Rationale

This commit fixes a genuine use-after-free: `tb_service_release()`
accesses the parent XDomain’s `service_ids` IDA after the XDomain may
already have been destroyed during disconnect teardown, because
`device_del()` drops the parent reference at unregister time rather than
at final service release. The fix is minimal, self-contained, and the
buggy code is present in this 6.18.43 tree. Stable maintainers have
already nominated it for multiple older stable branches.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body; no Fixes/Reported-by/Link
  tags; author is Mika Westerberg.
- **[Phase 2]** Read full diff; confirmed 2-line refcount fix in
  `tb_service_release()` and `enumerate_services()`.
- **[Phase 3]** `git blame` on xdomain.c service code; `git log -S`
  searches; no fix already in tree.
- **[Phase 4]** lore.kernel.org: 403 Forbidden. Ratatoskr: stable
  nominations found for 5.10.y/5.15.y/6.6.y. `b4 dig`: not run (no
  commit hash in tree).
- **[Phase 5]** Traced call chain through `tb_xdomain_remove()` →
  `device_unregister()` → `tb_xdomain_release()` vs deferred
  `tb_service_release()`.
- **[Phase 5]** Verified `device_del()` calls `put_device(parent)` at
  `drivers/base/core.c:3983`.
- **[Phase 5]** Verified `tb_xdomain_release()` calls
  `ida_destroy(&xd->service_ids)` at `xdomain.c:1883`.
- **[Phase 6]** `git describe HEAD` → v6.18.43; `make kernelversion` →
  6.18.43.
- **[Phase 6]** Read current `xdomain.c` — buggy pattern confirmed, fix
  absent.
- **[Phase 6]** Verified `tb_xdomain_get`/`tb_xdomain_put` exist in
  `include/linux/thunderbolt.h:325-336`.
- **[Phase 7]** Thunderbolt subsystem active with recent XDomain fixes
  in this tree.
- **[Phase 8]** Assessed UAF severity as HIGH-CRITICAL on disconnect
  path.

**YES**The analysis is done for **6.18.43**.

**Verdict: YES** — backport this commit.

It fixes a real use-after-free: when an XDomain disconnects,
`tb_service_release()` can call `ida_free()` on the parent XDomain’s
`service_ids` after that XDomain has already been torn down. The fix is
small (two lines: `get_device()` on parent assignment,
`tb_xdomain_put()` in release), applies cleanly here, and the buggy code
is present in this tree.

 drivers/thunderbolt/xdomain.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c
index 5fbdd412a712f..fed0fae2a6299 100644
--- a/drivers/thunderbolt/xdomain.c
+++ b/drivers/thunderbolt/xdomain.c
@@ -1029,6 +1029,7 @@ static void tb_service_release(struct device *dev)
 	ida_free(&xd->service_ids, svc->id);
 	kfree(svc->key);
 	kfree(svc);
+	tb_xdomain_put(xd);
 }
 
 const struct device_type tb_service_type = {
@@ -1137,7 +1138,7 @@ static void enumerate_services(struct tb_xdomain *xd)
 		svc->id = id;
 		svc->dev.bus = &tb_bus_type;
 		svc->dev.type = &tb_service_type;
-		svc->dev.parent = &xd->dev;
+		svc->dev.parent = get_device(&xd->dev);
 		dev_set_name(&svc->dev, "%s.%d", dev_name(&xd->dev), svc->id);
 
 		tb_service_debugfs_init(svc);
-- 
2.53.0


  parent reply	other threads:[~2026-08-31 13:38 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18] usb: xhci: remove legacy 'num_trbs_free' tracking Sasha Levin
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.10] xhci: Prevent queuing new commands if xhci is inaccessible Sasha Levin
2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-6.6] thunderbolt: Don't access path config space on Lane 1 adapters in tb_switch_reset_host() Sasha Levin
2026-08-31 13:23 ` Sasha Levin [this message]
2026-08-31 13:23 ` [PATCH AUTOSEL 6.18-6.1] usb: gadget: aspeed_udc: avoid past-the-end iterator in dequeue Sasha Levin
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-6.1] usb: gadget: udc: skip pullup() if already connected Sasha Levin
2026-08-31 13:25 ` [PATCH AUTOSEL 6.18-5.10] usb: core: hcd: fix possible deadlock in rh control transfers Sasha Levin
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.12] usb: xhci: Improve Soft Retries after short transfers Sasha Levin
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.6] thunderbolt: Verify Router Ready bit is set after router enumeration Sasha Levin
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.12] thunderbolt: Avoid reserved fields in path config space for USB4 routers Sasha Levin
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18-6.6] thunderbolt: Improve multi-display DisplayPort tunnel allocation Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-6.1] thunderbolt: Don't create multiple DMA tunnels on firmware connection manager Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-5.10] usb: gadget: goku_udc: avoid NULL deref of dev->driver in INT_USBRESET log Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18] thunderbolt: Verify PCIe adapter in detect state before tunnel setup Sasha Levin
2026-08-31 13:28 ` [PATCH AUTOSEL 6.18-5.10] usbip: vhci_hcd: fix NULL deref in status_show_vhci Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] thunderbolt: Set tb->root_switch to NULL when domain is stopped Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-6.12] thunderbolt: Don't disable lane adapter if XDomain lane bonding isn't possible Sasha Levin
2026-08-31 13:29 ` [PATCH AUTOSEL 6.18-5.10] usb: host: add ARCH_AIROHA in XHCI MTK dependency Sasha Levin
2026-08-31 13:30 ` [PATCH AUTOSEL 6.18-6.1] USB: cdc-acm: start bulk-IN polling when ALWAYS_POLL_CTRL is set Sasha Levin
2026-08-31 13:30 ` [PATCH AUTOSEL 6.18-5.10] net: usb: qmi_wwan: add MeiG SRM813Q Sasha Levin
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-5.10] thunderbolt: Keep the domain reference while processing hotplug Sasha Levin
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-6.6] thunderbolt: Increase timeout for Configuration Ready bit Sasha Levin
2026-08-31 13:31 ` [PATCH AUTOSEL 6.18-5.10] thunderbolt: Release request if tb_cfg_request() fails in __tb_xdomain_response() Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260831133314.4125787-157-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=YehezkelShB@gmail.com \
    --cc=andreas.noever@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=westeri@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox