* [PATCH AUTOSEL 6.18-5.10] nvme: zns: cap zone report nr_zones by DMA buffer size
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] nvme: clamp FDP nruhsd to allocated RUH status descriptor count Sasha Levin
` (10 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: Xixin Liu, Christoph Hellwig, Keith Busch, Sasha Levin, axboe,
sagi, linux-nvme, linux-kernel
From: Xixin Liu <liuxixin@kylinos.cn>
[ Upstream commit 09c9062d4f62199a634a45e2bb9e6e8e572cc78c ]
With Partial Report (PR=1), the Number of Zones (NZ) field in the report
header must equal the number of zone descriptors fully transferred in the
DMA buffer (ZNS Command Set Specification Rev 1.2, section 3.4.2).
nvme_ns_report_zones() does not cap the parse loop by max_in_buf derived
from buflen. Cap nz with min3() over the device-reported count, nr_zones -
zone_idx, and max_in_buf.
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
candidate is `09c9062d4f621` (“nvme: zns: cap zone report nr_zones by
DMA buffer size”). The ZNS report path has been here since v5.9; this is
a small host-side bounds cap against a device-controlled zone count.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[nvme: zns:] [cap] [limit zone-report parse count by DMA
buffer size]**
- Subsystem: NVMe host Zoned Namespace (ZNS) driver
- Verb “cap” is a bounds/safety fix, not a feature
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Fixes:** none (expected for manual review)
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** Christoph Hellwig `<hch@lst.de>` (NVMe/block
maintainer)
- **Acked-by:** none
- **Link:** none
- **Cc: stable:** none (expected; not a negative)
- **Signed-off-by:** Xixin Liu `<liuxixin@kylinos.cn>` (author); Keith
Busch `<kbusch@kernel.org>` (NVMe maintainer)
- Ignore any Sasha Levin SOB / “Upstream commit” markers — none present
on the real commit
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `nvme_ns_report_zones()` trusts `report->nr_zones` from the
device and caps only by the caller’s `nr_zones`, not by how many
`nvme_zone_descriptor`s fit in the allocated DMA/vmalloc buffer
(`buflen`).
- **Spec:** With Partial Report (PR=1), NZ must equal descriptors fully
transferred (ZNS Command Set Spec Rev 1.2 §3.4.2).
- **Symptom/failure mode:** parse loop `report->entries[i]` can walk
past `buflen` → kernel out-of-bounds read (crash / KASAN / info leak).
`sector += zsze * nz` can also advance by an inflated `nz`.
- **Version info in message:** none
- **Root cause:** missing cap derived from `buflen`
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not disguised.** Explicit bounds-cap of a device-provided
count. Real memory-safety fix, not cleanup.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/nvme/host/zns.c` — **+6 / −2** (8 lines in the commit)
- Function: `nvme_ns_report_zones()` only
- Scope: **single-file surgical fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** after `nvme_zns_alloc_report_buffer()` and
`nvme_submit_sync_cmd()`, `nz = min(le64_to_cpu(report->nr_zones),
nr_zones)` then `for (i = 0; i < nz && zone_idx < nr_zones; i++)`
reads `report->entries[i]`.
- **After:** `max_in_buf = (buflen - sizeof(nvme_zone_report)) /
sizeof(nvme_zone_descriptor)`; `nz = min3(device NZ, nr_zones -
zone_idx, max_in_buf)`.
- Path: normal ZNS zone-report (probe/revalidate, `BLKREPORTZONE` ioctl,
filesystems using `blkdev_report_zones()`).
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Memory safety / OOB] [device-controlled length not capped by
allocation]**
- `nvme_zone_report` is 64 bytes + flexible `entries[]`; each descriptor
is 64 bytes (verified in `include/linux/nvme.h`).
- Buffer is often **smaller** than `nr_zones` descriptors:
`nvme_zns_alloc_report_buffer()` min()s against
`queue_max_hw_sectors`, `queue_max_segments << PAGE_SHIFT`, and halves
`bufsize` on vmalloc failure.
- Callers pass `BLK_ALL_ZONES` (`(unsigned int)-1` / `UINT_MAX`) — e.g.
`disk->fops->report_zones(disk, 0, UINT_MAX, ...)` in `block/blk-
zoned.c` during zone revalidate. Then `min(device_nz, UINT_MAX) ==
device_nz`.
- If firmware reports total zones (or a huge NZ) instead of the partial-
transfer count, the loop indexes past the allocation.
- Secondary: `nr_zones - zone_idx` keeps `nz` as remaining requested
zones for `sector += zsze * nz`.
- Same buffer-derived cap already exists in the NVMe **target**:
`get_nr_zones_from_buf()` in `drivers/nvme/target/zns.c`.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Obviously correct:** `min3` of device count, remaining request, and
descriptors that fit in `buflen`.
- **Minimal:** two small hunks, no API/struct change.
- **Regression risk: very low.** Only **lowers** `nz`. Compliant PR=1
devices already set NZ ≤ `max_in_buf`, so behavior is unchanged on
good hardware.
- `min3()` exists in `include/linux/minmax.h` in this tree.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Buggy pattern introduced in **`240e6ee272c07`** (“nvme: support for
zoned namespaces”, Keith Busch, 2020-06-29).
- First contained in **`v5.9-rc1`**.
- Present for many years; not a recent-only bug.
- `nz = min(device NZ, nr_zones)` form after **`936fab503ff4a`** (“nvme:
fix error handling in nvme_ns_report_zones”), which still did not cap
by buffer size.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Introduced by `240e6ee272c07` (v5.9). That
commit **is** an ancestor of this tree (`git merge-base --is-ancestor
240e6ee272c07 HEAD` succeeded). Candidate **`09c9062d4f621` is also
already an ancestor of HEAD** (landed in **v7.3-rc1**). No later
overlapping fix of the same `nz` cap.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Related: `936fab503ff4a` (error handling), `8b4fb0f968ffe` (split
report_zones), `fdb9aed869f34` (`disk_report_zone()` args).
- **Standalone:** this hunk does not need those refactors; it only
changes how `nz` is computed.
- Sibling **`3456b52552896`** (v2 2/2) only adds zone index to an error
log — **not a prerequisite**.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record:
- Author Xixin Liu: several NVMe patches (`09c9062d4f621`,
`3456b52552896`, plus tcp/rdma/auth). Not the subsystem maintainer.
- Applied by **Keith Busch**; reviewed by **Christoph Hellwig**. High
maintainer weight.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **No functional prerequisites.** Uses existing `min3`, `buflen`,
`nvme_zone_report` / `nvme_zone_descriptor`. Can apply standalone on any
tree with this parse loop (since v5.9).
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 09c9062d4f621` → https://patch.msgid.link/8ed1b6a33454.v2.1
782280600.git.liuxixin@kylinos.cn
- Full thread saved via `b4 dig -m /tmp/nvme-zns-thread.mbox`
- **v1** (2026-06-24): combined bounds cap + log message change
- **v2** (2026-06-25): split per review; **1/2 is this fix**, 2/2 is
logging
- Christoph on v1: log change “unrelated, please split”; **“The rest
looks good as well.”**
- Christoph on v2 1/2: **“Looks good: Reviewed-by: Christoph Hellwig”**
- Keith Busch: **“Thanks, applied to nvme-7.3.”**
- **No NAK, no stable objection, no risk flags.** No explicit Cc: stable
in the thread.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc linux-nvme, Keith Busch, Jens Axboe,
Christoph Hellwig, Sagi Grimberg, linux-kernel. Appropriate NVMe/block
maintainers.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No syzbot, bugzilla, or user crash report.** Spec-driven host-
side hardening. Still a real OOB if NZ exceeds the buffer
(buggy/malicious NVMe, including fabrics/USB).
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: v2 is 2/2. **1/2 is self-contained.** 2/2 is optional logging,
not required for the bounds fix.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: Web search of lore.kernel.org/stable for this subject found **no
relevant discussion** (unrelated historical hits only). Not previously
rejected for stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: **`nvme_ns_report_zones()`** (uses
`nvme_zns_alloc_report_buffer()`, `nvme_zone_parse_entry()`,
`nvme_submit_sync_cmd()`).
**Step 5.2: TRACE CALLERS**
Record:
- `nvme_report_zones()` in `drivers/nvme/host/core.c` — `gendisk`
`.report_zones` (`nvme_bdev_ops`)
- `nvme_ns_head_report_zones()` in `drivers/nvme/host/multipath.c` —
multipath head ops
Both under `CONFIG_BLK_DEV_ZONED` (`zns.o` is `nvme-
core-$(CONFIG_BLK_DEV_ZONED)`).
Upper layer: `blkdev_do_report_zones()` → ioctl,
`blkdev_report_zones()`, and **revalidate with `UINT_MAX`** (`block/blk-
zoned.c` ~2354). Filesystems: zonefs, f2fs, btrfs, xfs zoned.
**Step 5.3: TRACE CALLEES**
Record: alloc buffer (vmalloc, queue limits), Zone Management Receive
with `NVME_REPORT_ZONE_PARTIAL`, parse entries, `disk_report_zone()`,
`kvfree`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace `BLKREPORTZONE` / zoned FS mount / disk revalidate →
`disk->fops->report_zones` → `nvme_ns_report_zones`. **Reachable from
userspace and from device probe.** Unprivileged trigger depends on
device node permissions; probe path is admin/boot. Malicious NVMe-over-
Fabrics still in scope for some deployments.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: NVMe target already caps with `get_nr_zones_from_buf()`. SCSI
`sd_zbc.c` uses report-length/`nr_zones` differently. Host ZNS was the
missing cap.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Tree: **Linux 7.3.0-rc4** (`git describe HEAD` =
`v7.3-rc4-27-gf0100363d8c37`).
- File and function exist; current `zns.c` **already contains this cap**
(commit is in HEAD).
- Bug introduced **v5.9** (`240e6ee272c07`); not after this branch
point.
- This checkout is **mainline 7.3-rc**, not an older LTS cut, but the
**same `nvme_ns_report_zones` loop** is what older stables still have
without the cap.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** on this tree (already applied). Context around
`nz = min(...)` is stable since the 2020 error-handling fix. No
conflicting later rewrite of that assignment. Expected **clean or
trivial** cherry-pick onto trees that still have
`nvme_ns_report_zones()`.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This is the fix. No alternate `max_in_buf` cap. Sibling logging
commit is independent.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **`drivers/nvme/host/` ZNS** — **IMPORTANT** (storage; not core
mm/VFS, but NVMe zoned devices used by zonefs/f2fs/btrfs/xfs). Config-
gated by `CONFIG_BLK_DEV_ZONED`.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: NVMe host is actively maintained; `zns.c` itself is **mature**
(ZNS since 2020). Long-lived bug, not churn from a new subsystem.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver/config-specific:** NVMe ZNS namespaces
(`CONFIG_BLK_DEV_ZONED`). Not all NVMe users; all ZNS users on
report/revalidate/ioctl/FS mount.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Common path: zone report / revalidate (`UINT_MAX` zones).
- Harmless on spec-compliant PR=1 devices (NZ already ≤ buffer).
- Harmful if firmware reports NZ larger than transferred descriptors
(ignore PR, report total count, or malicious controller).
- Not a timing race. Device-controlled integer.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Out-of-bounds kernel read** of `report->entries[i]` past
`buflen` → oops/KASAN, possible infoleak. **Severity: HIGH** (memory
safety from device-provided length). Inflated `nz` can also skip zones
in `sector +=` if the loop continues; the OOB is the primary issue.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** HIGH for ZNS — prevents OOB on buggy/malicious
controllers; correct remaining-count; matches target-side logic.
- **Risk:** VERY LOW — 6 insertions, only tightens a min(); no
lock/API/hot-path change.
- **Ratio:** strongly favors backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real OOB: parse count from device NZ not limited by DMA/vmalloc buffer
- Buffer is routinely smaller than requested `nr_zones` (queue limits,
vmalloc backoff)
- Revalidate uses `UINT_MAX`, so `min(NZ, nr_zones)` does not cap
- Small (+6/−2), obviously correct `min3`
- Reviewed by Christoph Hellwig; applied by Keith Busch
- Same cap already in NVMe target
- Bug in tree since v5.9; no extra deps
- No new API/feature
AGAINST:
- No public crash/syzbot report (spec-compliant devices already set NZ)
- ZNS is a subset of NVMe users
- No Fixes:/Cc: stable (ignored per rules)
UNRESOLVED:
- Could not fetch lore.kernel.org HTML (403); thread recovered via `b4
dig -m`
- No stable-list discussion found
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — maintainer Reviewed-by;
applied to nvme-7.3
2. Fixes a real bug that affects users? **PASS** — OOB if NZ exceeds
buffer; reachable zone-report path
3. Important issue? **PASS** — HIGH memory-safety (OOB read), not
cosmetic
4. Small and contained? **PASS** — 8 lines, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — code present; patch already
applies here
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **Not** a device-ID/quirk/DT/build/docs exception. Ordinary
**memory-safety bug fix**.
**Step 9.4: MAKE YOUR DECISION**
This should be backported: it is a small, maintainer-acked cap that
stops the ZNS host from walking off the report buffer when the device’s
NZ is larger than what `buflen` can hold. On this 7.3-rc4 tree the ZNS
report path exists and the same bug has been in `nvme_ns_report_zones()`
since v5.9.
What it solves for stable users: kernel OOB on NVMe ZNS zone reports
(ioctl, FS mount, disk revalidate), especially with `BLK_ALL_ZONES` and
firmware that does not honor Partial Report.
Risk vs benefit: benefit is memory safety on a real I/O path; risk is a
tighter min that is a no-op on compliant devices.
Concerns: none material. Sibling logging patch is not required. No
missing prerequisites on this tree.
Verification:
- [Phase 1] Parsed subject/tags: Reviewed-by Christoph Hellwig; SOB
Keith Busch; no Fixes/Reported-by/Cc:stable
- [Phase 1] Body: PR=1 / NZ vs DMA buffer; missing `max_in_buf` cap
- [Phase 2] Diff: `drivers/nvme/host/zns.c` +6/−2 in
`nvme_ns_report_zones()`; `min3(device NZ, nr_zones - zone_idx,
max_in_buf)`
- [Phase 2] Struct sizes: `nvme_zone_report` 64B, descriptor 64B
(`include/linux/nvme.h`)
- [Phase 2] Alloc can shrink buffer: `nvme_zns_alloc_report_buffer()`
hw_sectors/segments/vmalloc halving
- [Phase 3] `git blame`: loop from `240e6ee272c07` (v5.9-rc1)
- [Phase 3] `git describe --contains 240e6ee272c07` → v5.9-rc1;
`09c9062d4f621` → v7.3-rc1
- [Phase 3] `git merge-base --is-ancestor` both SHAs vs HEAD: success
- [Phase 3] No Fixes: tag; sibling `3456b52552896` is logging-only
- [Phase 4] `b4 dig -c`: msgid
`8ed1b6a33454.v2.1782280600.git.liuxixin@kylinos.cn`
- [Phase 4] `b4 dig -a`: v1 then v2 (split log change)
- [Phase 4] `b4 dig -w`: linux-nvme, kbusch, axboe, hch, sagi
- [Phase 4] mbox: Christoph “Looks good” + R-b; Keith “applied to
nvme-7.3”; no NAK
- [Phase 4] lore HTTP 403; used b4 mbox instead
- [Phase 4] stable list search: no relevant thread
- [Phase 5] Callers: `core.c` `nvme_report_zones`, `multipath.c`
`nvme_ns_head_report_zones`; `blk-zoned.c` `UINT_MAX` revalidate
- [Phase 5] `BLK_ALL_ZONES` = `(unsigned int)-1` in
`include/linux/blkdev.h`
- [Phase 5] Target already has `get_nr_zones_from_buf()`
- [Phase 6] `git describe HEAD`: v7.3-rc4-27-g…; `make kernelversion`:
7.3.0-rc4
- [Phase 6] Current `zns.c` already has `max_in_buf` / `min3`
- [Phase 8] Failure: OOB read of `report->entries[]`; severity HIGH
- UNVERIFIED: live crash on specific firmware (no public repro); does
not override code-level OOB
The patch is small, correct, and fixes a real device-controlled bounds
bug in NVMe ZNS zone reporting.
**YES**
drivers/nvme/host/zns.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/zns.c b/drivers/nvme/host/zns.c
index cce4c5b55aa9c..f067185920451 100644
--- a/drivers/nvme/host/zns.c
+++ b/drivers/nvme/host/zns.c
@@ -178,7 +178,7 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
struct nvme_zone_report *report;
struct nvme_command c = { };
int ret, zone_idx = 0;
- unsigned int nz, i;
+ unsigned int max_in_buf, nz, i;
size_t buflen;
if (ns->head->ids.csi != NVME_CSI_ZNS)
@@ -188,6 +188,9 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
if (!report)
return -ENOMEM;
+ max_in_buf = (buflen - sizeof(struct nvme_zone_report)) /
+ sizeof(struct nvme_zone_descriptor);
+
c.zmr.opcode = nvme_cmd_zone_mgmt_recv;
c.zmr.nsid = cpu_to_le32(ns->head->ns_id);
c.zmr.numd = cpu_to_le32(nvme_bytes_to_numd(buflen));
@@ -207,7 +210,8 @@ int nvme_ns_report_zones(struct nvme_ns *ns, sector_t sector,
goto out_free;
}
- nz = min((unsigned int)le64_to_cpu(report->nr_zones), nr_zones);
+ nz = min3((unsigned int)le64_to_cpu(report->nr_zones),
+ nr_zones - zone_idx, max_in_buf);
if (!nz)
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH AUTOSEL 6.18] nvme: clamp FDP nruhsd to allocated RUH status descriptor count
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-5.10] nvme: zns: cap zone report nr_zones by DMA buffer size Sasha Levin
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-5.10] nvme: ratelimit the completion-path messages driven by device data Sasha Levin
` (9 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: Hari Mishal, Christoph Hellwig, Greg Kroah-Hartman, Keith Busch,
Sasha Levin, axboe, sagi, linux-nvme, linux-kernel
From: Hari Mishal <harimishal1@gmail.com>
[ Upstream commit 29261f8bb41662f2a660c479e5cf592942b53f78 ]
nvme_query_fdp_info() allocates the RUH status buffer for at most S8_MAX
- 1 descriptors, and then copies ruhs->ruhsd[] into head->plids[] using
the controller reported ruhs->nruhsd directly as the loop bound.
However, that count wasn't taken into account for the actual buffer's
size, so there was a chance for a controller reporting a larger nruhsd
to cause the copy to overflow the buffer. Clamp nr_plids to the same
bound used for the allocation.
Assisted-by: gkh_clanker_t1000
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This is a one-line out-of-bounds-read clamp in NVMe FDP setup. The local
tree is **v7.3-rc4** (`7.3.0-rc4`, HEAD `f0100363d8c37`).
`nvme_query_fdp_info()` exists here; the overflow is real; the change is
surgical.
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[nvme]** **[clamp]** clamp FDP `nruhsd` to the allocated RUH
status descriptor count. “Clamp” is a bounds-fix verb.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Reviewed-by:** Christoph Hellwig `<hch@lst.de>`
- **Assisted-by:** gkh_clanker_t1000
- **Signed-off-by:** Hari Mishal (author), Greg Kroah-Hartman
(submitter), Keith Busch (NVMe maintainer)
- **No Fixes:** tag (expected)
- **No Reported-by / Tested-by / Link / Cc: stable**
- Ignore pipeline SOBs; Greg’s SOB is from the actual submission, not a
stable-prep marker
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `nvme_query_fdp_info()` allocates a RUH-status buffer for at
most `S8_MAX - 1` descriptors, then copies `ruhs->ruhsd[]` using
controller-reported `ruhs->nruhsd` as the loop bound.
- **Symptom:** a controller that reports `nruhsd` larger than the
allocated descriptor count overflows that buffer on the copy into
`head->plids[]`.
- **Root cause:** allocation size and copy bound were not the same.
- **Fix:** clamp `nr_plids` to the same bound used for allocation.
- **Version info in message:** none.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. This is an explicit buffer-overflow / OOB-read fix.
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/nvme/host/core.c` — 1 insertion, 1 deletion
- **Function:** `nvme_query_fdp_info()`
- **Scope:** single-file, one-line surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: Before: `head->nr_plids = le16_to_cpu(ruhs->nruhsd);` then
`kcalloc(head->nr_plids, …)` and `for (i = 0; i < head->nr_plids; i++)
head->plids[i] = le16_to_cpu(ruhs->ruhsd[i].pid);`. After: `nr_plids` is
`min(reported, S8_MAX - 1)`, so the copy cannot walk past the allocated
`ruhs` buffer. Path is namespace-head init when the controller
advertises FDP.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **(g) logic/correctness + (d) memory safety — out-of-bounds
read.** `nruhsd` is a `__le16` from the device (0..65535). Allocation
used `struct_size(ruhs, ruhsd, S8_MAX - 1)` (126 descriptors, 4048
bytes). Verified layout in `include/linux/nvme.h`: 16-byte header +
32-byte `nvme_fdp_ruh_status_desc`. If `nruhsd == 65535`, the loop reads
~2MB past the buffer. `plids` is sized from `nr_plids`, so the overflow
is an OOB **read** of `ruhs`, not a write into `plids`.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct: clamp matches the allocation bound. Minimal.
Regression risk is low: extra reported placement IDs are dropped instead
of overflowing. Later mainline commit `53cdaeab2e30e` raised that cap to
`U8_MAX` / `NVME_MAX_PLIDS`; that is a follow-up improvement, not a sign
this clamp is wrong.
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Allocation `struct_size(..., S8_MAX - 1)` introduced in
`30b5f20bb2dda` (“nvme: register fdp parameters with the block layer”,
Keith Busch, 2025-05-06).
- Unclamped `nr_plids` assignment plus the `ruhsd[]` copy loop:
`38e8397dde633` (“nvme: use fdp streams if write stream is provided”).
- Both first appear in **v6.16-rc1** (`git describe --contains --match
'v*'`).
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Introducing commits identified by blame, not by
tag.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- `29261f8bb4166` — this clamp (candidate)
- `53cdaeab2e30e` — raise cap to `U8_MAX` and introduce `NVME_MAX_PLIDS`
(depends on the clamp being present)
- `56e1c6bbe4bb0` — race on the plids array (independent locking fix)
- Series partner `3c568b35a0d30` — NS descriptor bounds (different
function, not a prerequisite)
Standalone. Labeled `[PATCH 2/2]` but 1/2 is an independent bounds check
in `nvme_identify_ns_descs()`.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Hari Mishal also authored sibling overflow fixes (`nvme: bound
ns descriptor header…`, `nvmet: passthru: fix OOB reads…`). Submitted
via Greg; merged by NVMe maintainer Keith Busch. Christoph Hellwig
reviewed.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None for the clamp itself. It only needs `nvme_query_fdp_info()`
with `S8_MAX - 1` allocation and the copy loop — present since v6.16. No
new helpers or APIs.
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 29261f8bb4166` → https://patch.msgid.link/2026070932-fried-
frying-9c36@gregkh
- **v1 only** (`b4 dig -a`): `[PATCH 1/2] nvme: bound ns descriptor…` +
`[PATCH 2/2]` this clamp
- Full thread saved via `b4 dig -m`; lore HTTP fetch returned 403, so
discussion is from the mbox, not lore HTML
- Christoph: “Looks good: Reviewed-by: Christoph Hellwig”
- Keith Busch: “Thanks, series applied to nvme-7.3.”
- No NAK, no stable discussion in-thread, no reviewer risk flags
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): linux-nvme, linux-kernel, Keith Busch, Jens Axboe,
Christoph Hellwig, Sagi Grimberg. Appropriate maintainers were on Cc.
Christoph reviewed; Keith applied.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla/`Reported-by`. Bug is from code review /
static analysis (Greg/clanker-assisted). Still a concrete OOB from
device-provided `nruhsd`.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: 2/2 is independent of 1/2. Follow-up `53cdaeab2e30e` changes the
numeric cap but does not replace the need to clamp. Race fix
`56e1c6bbe4bb0` is separate.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search returned 403. **UNVERIFIED**
whether stable@ already discussed this. No evidence in the nvme thread
of a stable NAK.
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `nvme_query_fdp_info()` only.
**Step 5.2: TRACE CALLERS**
Record: Only caller is `nvme_alloc_ns_head()` in
`drivers/nvme/host/core.c`:
```4043:4047:drivers/nvme/host/core.c
if (ctrl->ctratt & NVME_CTRL_ATTR_FDPS) {
ret = nvme_query_fdp_info(ns, info);
if (ret < 0)
goto out_cleanup_srcu;
}
```
Triggered during namespace-head creation when Identify Controller
`ctratt` has `NVME_CTRL_ATTR_FDPS`. That is probe/scan, not a hot I/O
path, but it runs for every FDP-capable namespace.
**Step 5.3: TRACE CALLEES**
Record: `nvme_get_features()`, `nvme_query_fdp_granularity()`,
`kzalloc`/`kcalloc`, `nvme_submit_sync_cmd()` (device fills `ruhs`),
then the copy loop.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Device probe / namespace scan → `nvme_init_ns_head` →
`nvme_alloc_ns_head` → `nvme_query_fdp_info`. Reachable from attaching
an NVMe device, including NVMe over Fabrics. A malicious or buggy target
can set FDPS and return a huge `nruhsd`. Not a direct unprivileged
syscall, but device/Fabrics-controlled and kernel-reachable.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same author series also bounded NS identify descriptors. Follow-
up kept the clamp and only raised the cap. No second unclamped `nruhsd`
copy found.
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes, in this lineage.** This checkout is **v7.3-rc4** (`make
kernelversion` → `7.3.0-rc4`). FDP query exists. Bug introduced
**v6.16-rc1**, before 7.3. Candidate `29261f8bb4166` is already an
ancestor of HEAD (`git merge-base --is-ancestor` succeeded; first
contained in **v7.3-rc1**). Current HEAD already clamps:
```2371:2371:drivers/nvme/host/core.c
head->nr_plids = min(le16_to_cpu(ruhs->nruhsd), NVME_MAX_PLIDS);
```
That is this fix plus `53cdaeab2e30e` (`NVME_MAX_PLIDS` = `U8_MAX`). The
unclamped assignment is gone at HEAD because the fix landed in 7.3-rc1;
the overflow pattern is still the code this tree maintains.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Against **current HEAD**, the original hunk would not apply
(`S8_MAX - 1` vs `NVME_MAX_PLIDS`). Against the pre-fix function (parent
of `29261f8bb4166` in this history), it is a one-line change on
`head->nr_plids = le16_to_cpu(ruhs->nruhsd)` with matching `S8_MAX - 1`
allocation. **Expected backport difficulty: clean apply** on trees that
still have that allocation; this 7.3 checkout already has the later
form.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This clamp and the `NVME_MAX_PLIDS` follow-up are both in HEAD.
That is this tree already containing the upstream fix, not a different
substitute.
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **NVMe host core** (`drivers/nvme/host/`). **IMPORTANT** —
common storage stack, not staging. Overflow is in host core during
namespace setup.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Active (recent FDP, multipath, scan, SRCU fixes). FDP streams
themselves date to v6.16; this is not brand-new experimental code in
7.3.
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific / config-specific:** NVMe controllers with FDP
(`NVME_CTRL_ATTR_FDPS` + `FDPCFG_FDPE`). Includes local NVMe and Fabrics
targets that advertise FDP.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Namespace scan/probe on an FDP-enabled controller that returns
`nruhsd >` allocated descriptor count. Not every boot. A buggy SSD can
hit it; an untrusted NVMe/TCP target can hit it on purpose. Unprivileged
local users cannot trigger it without a device.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Kernel out-of-bounds read** of `ruhs->ruhsd[]` → KASAN/oops or
infoleak of adjacent heap. **Severity: HIGH.**
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record: **Benefit: high** (stops device-triggered OOB). **Risk: very
low** (one `min()`, same bound as allocation). **Ratio: strongly
favorable.**
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real OOB read from device-controlled `nruhsd` vs a 126-descriptor
buffer
- One-line, obviously correct clamp
- Reviewed by Christoph Hellwig; applied by Keith Busch
- No new API/feature
- Self-contained; series 1/2 is not required
- FDP path exists in this tree; bug dates to v6.16
**AGAINST:**
- No public user/syzbot report (code-analysis find)
- Only FDP-capable devices
- HEAD already has this clamp (and a later cap raise), so this exact
hunk is not what HEAD’s source line looks like
- Lore stable@ search could not be fetched (403)
**UNRESOLVED:**
- Whether stable@ already queued it (lore/stable HTTP 403)
- No runtime reproducer in the message (mechanism is still verified in
code)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — clamp equals allocation
bound; reviewed and merged to nvme-7.3
2. Fixes a real bug that affects users? **PASS** — OOB read on FDP
namespace setup
3. Important issue (crash/security/corruption/deadlock)? **PASS** —
HIGH, kernel OOB from device data
4. Small and contained? **PASS** — 1 line, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** for this codebase’s pre-fix
form (`S8_MAX - 1` allocation + unclamped `nruhsd`). HEAD already has
the evolved clamp; that does not make the fix inapplicable to this
lineage.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: None (not a device-ID/quirk/DT/build/docs exception). It is a
normal memory-safety fix.
**Step 9.4: MAKE YOUR DECISION**
This tree’s NVMe host core has the FDP RUH-status query, and using the
controller’s `nruhsd` as the copy bound without capping it to the
allocated descriptor count is a real overflow. The fix is the smallest
correct change. This 7.3 checkout already contains that logic (then
raised the cap to `U8_MAX`); that is the upstream landing, not a reason
to treat the overflow as unworthy of stable. Benefit vs risk is strongly
in favor of backporting.
### What the commit solves, and why it matters
A Flexible Data Placement controller returns how many Reclaim Unit
Handle status descriptors it has (`nruhsd`). The driver only allocated
room for 126, then indexed `ruhs->ruhsd[i]` up to the raw reported
count. That is a kernel heap OOB read during namespace setup, including
from a remote NVMe/TCP target.
### Stable rules
Small, obviously correct, no new API, real memory-safety bug.
### Risk vs benefit
One `min()` vs device-triggered OOB: take the fix.
### Concerns
Patch 2/2 does not need patch 1/2. Trees that already switched
allocation to `NVME_MAX_PLIDS` should keep clamp and allocation in sync
(already true here via `53cdaeab2e30e`). Trees still on `S8_MAX - 1`
should get this commit as-is.
Verification:
- [Phase 1] Parsed tags: Reviewed-by Christoph Hellwig; SOBs Hari
Mishal, Greg KH, Keith Busch; no Fixes/Reported-by/Cc stable
- [Phase 2] Diff: 1 line in `nvme_query_fdp_info()`;
`le16_to_cpu(ruhs->nruhsd)` → `min(..., S8_MAX - 1)`
- [Phase 2] Struct sizes verified in `include/linux/nvme.h`: 16-byte
header + 32-byte descriptors; 126 descriptors → 4048-byte buffer;
`nruhsd` up to 65535
- [Phase 3] Blame: allocation `30b5f20bb2dda`, copy loop
`38e8397dde633`; both `git describe --contains --match 'v*'` →
v6.16-rc1
- [Phase 3] `git merge-base --is-ancestor 29261f8bb4166 HEAD` → commit
is in this tree; `describe --contains` → v7.3-rc1
- [Phase 3] Parent of candidate still has `size = struct_size(ruhs,
ruhsd, S8_MAX - 1)` and unclamped `nr_plids`
- [Phase 4] `b4 dig -c` → msgid 2026070932-fried-frying-9c36@gregkh;
`-a` v1 only; `-w` nvme maintainers; `-m` full thread
- [Phase 4] mbox: Christoph R-b; Keith “applied to nvme-7.3”; no NAK
- [Phase 4] lore.kernel.org and patch.msgid.link HTTP 403; LKML mirrors
agree with mbox
- [Phase 5] grep callers: only `nvme_alloc_ns_head()` when
`NVME_CTRL_ATTR_FDPS`
- [Phase 6] `git describe HEAD` / `make kernelversion`: v7.3-rc4 /
7.3.0-rc4; current clamp is `min(..., NVME_MAX_PLIDS)` from
`53cdaeab2e30e`
- [Phase 8] Failure mode: OOB read of `ruhs->ruhsd[]`, severity HIGH
- UNVERIFIED: stable@ mailing-list history (HTTP 403); no runtime
reproducer in the commit message
**YES**
drivers/nvme/host/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 5ed62e070f715..ad725e346c94e 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2313,7 +2313,7 @@ static int nvme_query_fdp_info(struct nvme_ns *ns, struct nvme_ns_info *info)
goto free;
}
- head->nr_plids = le16_to_cpu(ruhs->nruhsd);
+ head->nr_plids = min(le16_to_cpu(ruhs->nruhsd), S8_MAX - 1);
if (!head->nr_plids)
goto free;
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH AUTOSEL 6.18-5.10] nvme: ratelimit the completion-path messages driven by device data
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-5.10] nvme: zns: cap zone report nr_zones by DMA buffer size Sasha Levin
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18] nvme: clamp FDP nruhsd to allocated RUH status descriptor count Sasha Levin
@ 2026-10-19 13:45 ` Sasha Levin
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.15] nvmet: zns: reject full zone report when buffer is too small Sasha Levin
` (8 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:45 UTC (permalink / raw)
To: patches, stable
Cc: Chao Shi, Keith Busch, Sasha Levin, axboe, sagi, linux-nvme,
linux-kernel
From: Chao Shi <coshi036@gmail.com>
[ Upstream commit c05c86681170e381e24498257db3a879deb1ae89 ]
nvme_find_rq() and nvme_handle_cqe() print an unratelimited message for
every completion queue entry whose command id does not resolve to an
in-flight request. Both are reached from the completion interrupt path
(nvme_irq() -> nvme_poll_cq() -> nvme_handle_cqe()) and the decision to
print is made entirely from device-supplied data, so a controller that
posts a stream of bogus command ids drives unbounded printk from hard
interrupt context.
This is not hypothetical. A single boot under an emulated controller
that posts invalid completions produced 846 "could not locate request
for tag 0x0", 846 "invalid id 0 completed on queue 2" and 123 "genctr
mismatch" lines. Once the tag set has been torn down every subsequent
completion resolves to nothing, so the print rate is bounded only by how
fast the device can post entries.
Ratelimit the three messages. The information they carry is diagnostic
and repeats, so the suppression count printed by the ratelimit helpers
is enough to tell that the condition persists. This matches how the
other device-driven error prints in the driver are already handled, for
example the status messages in nvme_log_error() and
nvme_log_err_passthru().
nvme_find_rq() lives in nvme.h and is shared by pci, tcp, rdma, apple and
target-loop, so all transports are covered.
Found by FuzzNvme.
Signed-off-by: Chao Shi <coshi036@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have enough local-tree and review evidence to decide. The remaining
work is to write the full nine-phase analysis.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
Step 1.1: PARSE THE SUBJECT LINE
Record: **[nvme]** **[ratelimit]** Completion-path messages that fire
from device-supplied command IDs are unbounded; the commit rate-limits
those three prints.
Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
Record:
- **Signed-off-by:** Chao Shi \<coshi036@gmail.com\> (author)
- **Signed-off-by:** Keith Busch \<kbusch@kernel.org\> (NVMe maintainer;
applied the patch)
- **No Fixes:** tag (expected for this review queue)
- **No Cc: stable** (expected; not a negative)
- **No Reported-by / Tested-by / Reviewed-by / Acked-by / Link:**
- Notable: “Found by FuzzNvme” in the body (fuzzer-found, not syzbot)
- Ignore any pipeline “Upstream commit” / “Signed-off-by: Sasha Levin”
markers
Step 1.3: ANALYZE THE COMMIT BODY TEXT
Record:
- **Bug:** `nvme_find_rq()` and `nvme_handle_cqe()` print on every CQE
whose command id does not map to an in-flight request. The print
decision is made from device-supplied data.
- **Symptom:** Unbounded `printk` from the completion interrupt path
(`nvme_irq()` → `nvme_poll_cq()` → `nvme_handle_cqe()`). One emulated-
controller boot produced 846 “could not locate request for tag 0x0”,
846 “invalid id 0 completed on queue 2”, and 123 “genctr mismatch”
lines. After the tag set is torn down, every later completion prints;
rate is limited only by how fast the device posts entries.
- **Version info:** None in the message. The prints have been in-tree
since v5.15 / v5.9 (verified in Phase 3).
- **Root cause:** Device-controlled, unratelimited diagnostics on the
hard-IRQ completion path. Other device-driven NVMe error prints
(`nvme_log_error()`, `nvme_log_err_passthru()`) are already
ratelimited.
Step 1.4: DETECT HIDDEN BUG FIXES
Record: Not a disguised cleanup. It is an explicit fix for IRQ-context
log flood. The subject uses “ratelimit” rather than “fix”, but the body
states the failure mode and a concrete reproduction.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
Step 2.1: INVENTORY THE CHANGES
Record:
- `drivers/nvme/host/nvme.h`: `nvme_find_rq()` — `pr_err` →
`pr_err_ratelimited`; `dev_err` → `dev_err_ratelimited` (~5 lines net
churn)
- `drivers/nvme/host/pci.c`: `nvme_handle_cqe()` — `dev_warn` →
`dev_warn_ratelimited` (3 lines)
- Scope: **single-purpose surgical fix**, two files, ~9 insertions / 8
deletions. No logic, locking, or API changes.
Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
Record:
- Hunk 1 (`nvme_find_rq`, missing request): same `return NULL`; only the
error print is ratelimited.
- Hunk 2 (`nvme_find_rq`, genctr mismatch): same `return NULL`; only the
error print is ratelimited.
- Hunk 3 (`nvme_handle_cqe`, invalid id): same early return; only the
warning is ratelimited.
- Affected path: completion / IRQ / poll error path when the device
posts a bad command id. Success path is unchanged.
Step 2.3: IDENTIFY THE BUG MECHANISM
Record:
- **Category:** Logic / correctness of diagnostics; IRQ-context
resource/DoS (unbounded `printk` driven by device data).
- **Mechanism:** A malicious or failing controller can post a stream of
CQEs with bogus command ids. Each entry hits
`pr_err`/`dev_err`/`dev_warn` in `nvme_irq()`. After tag-set teardown
the miss path is permanent. `_ratelimited` variants cap volume while
still reporting that the condition persists (suppression count).
Step 2.4: ASSESS THE FIX QUALITY
Record:
- Obviously correct: swap to helpers already used in the same driver
(`nvme_log_error()`, PCI timeout path).
- Minimal: three call-site changes, no behavior change on the success
path.
- Regression risk: very low. Worst case is fewer repeated log lines for
a persistent fault; first occurrences and the ratelimit “suppressed”
count remain.
- Red flags: none (no API, locking, or data-structure changes).
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
Step 3.1: BLAME THE CHANGED LINES
Record:
- `nvme_find_rq()` and both prints: `e7006de6c23803` (“nvme: code
command_id with a genctr for use-after-free validation”, Sagi
Grimberg, 2021-06-16). Contained in **v5.15-rc1**. Present in this
tree (`git merge-base --is-ancestor` succeeded).
- PCI “invalid id” print: `50b7c24390a53c` (“nvme-pci: fix NULL req in
completion handler”, Xianting Tian, 2020-09-22). Contained in
**v5.9-rc7**. Present in this tree.
- The buggy prints have been in NVMe for years, including all 6.12.y
history.
Step 3.2: FOLLOW THE FIXES: TAG
Record: No `Fixes:` tag. N/A. The introducing commits above are the
functional origin.
Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
Record:
- `git log --oneline -15 HEAD -- drivers/nvme/host/nvme.h` shows no
later rewrite of `nvme_find_rq()`.
- `git log --oneline --grep="ratelimit the completion-path" HEAD --
drivers/nvme/` is empty: this fix is **not** in the local tree.
- Standalone one-patch change. Not “patch X/Y”. Keith applied it to
nvme-7.3 with only a line-wrap tweak.
Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Record:
- Chao Shi: only `4298c52252cf6` (“nvme: reject passthrough of driver-
managed Set Features”) in this tree under `drivers/nvme/`. Not a
series dependency.
- Keith Busch is an NVMe maintainer; his SOB is a strong quality signal.
Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Record: No new helpers or structs. `pr_err_ratelimited` /
`dev_err_ratelimited` / `dev_warn_ratelimited` exist in this tree
(`include/linux/printk.h`, `include/linux/dev_printk.h`) and are already
used in `drivers/nvme/host/core.c` and `pci.c`. Applies standalone.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Record:
- `b4 dig -c <msgid>` failed: the landed commit is not in this checkout,
so there is no local commit-ish to match.
- Thread recovered from Patchew and Openwall (lore.kernel.org returned
403):
- Submission: https://patchew.org/linux/20260810220258.1960208-1-
coshi036@gmail.com/
- Message-ID: `20260810220258.1960208-1-coshi036@gmail.com`
(2026-08-10)
- Single v1 patch (not a multi-revision series).
- Keith Busch: agreed that dumping every garbage CQE adds no diagnostic
value; **applied to nvme-7.3** with a minor overly-long-line wrap. No
NAK, no stability concern.
- Author replied that later submissions would avoid long lines. That is
style only; Keith already applied the wrap.
Step 4.2: CHECK WHO REVIEWED THE PATCH
Record: To/Cc on the v1: Keith Busch, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, linux-nvme, linux-kernel. Maintainer applied it. No
separate Reviewed-by on the provided commit text.
Step 4.3: SEARCH FOR THE BUG REPORT
Record: No bugzilla/syzbot link. Reproduction is in the commit: FuzzNvme
+ emulated controller, 1815 diagnostic lines in one boot. Severity from
the author’s data: hard-IRQ printk flood, unbounded after tag-set
teardown.
Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Record: Standalone. Sibling unratelimited prints remain in
apple/tcp/rdma/loop *after* `nvme_find_rq()` fails; those are extra
transport messages, not prerequisites. This patch still covers the
shared `nvme_find_rq()` prints on all of those transports.
Step 4.5: CHECK STABLE MAILING LIST HISTORY
Record: lore.kernel.org/stable search returned 403. Openwall/Patchew
show no stable NAK or “already rejected” discussion. UNVERIFIED: no
evidence this was previously declined for stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF
Record: `nvme_find_rq()` (inline in `nvme.h`); `nvme_handle_cqe()`
(`pci.c`).
Step 5.2: TRACE CALLERS
Record (verified with grep + reads):
- `nvme_find_rq()`:
- `nvme_handle_cqe()` — PCI
- `nvme_tcp_process_nvme_cqe()`, `nvme_tcp_handle_c2h_data()`,
`nvme_tcp_handle_r2t()` — TCP
- `nvme_rdma_process_nvme_rsp()` — RDMA
- `apple_nvme_handle_cqe()` — Apple
- `nvme_loop` completion path — target-loop
- `nvme_handle_cqe()` ← `nvme_poll_cq()` ← `nvme_irq()` (hard IRQ or
threaded IRQ), `nvme_poll_irqdisable()`, `nvme_poll()`,
`nvme_reap_pending_cqes()`.
- Impact: every NVMe transport’s completion path. PCI hard-IRQ is the
hottest and the one the commit calls out.
Step 5.3: TRACE CALLEES
Record: `blk_mq_tag_to_rq()`, genctr helpers, then `pr_*` / `dev_*`. No
allocation, no new locking.
Step 5.4: FOLLOW THE CALL CHAIN
Record: PCI: device posts CQE → MSI/MSI-X → `nvme_irq()` →
`nvme_poll_cq()` → `nvme_handle_cqe()` → `nvme_find_rq()`. Reachable
whenever an NVMe controller is attached, including VMs/emulators and
failing real devices. Userspace does not need a special syscall; a bad
device is enough. FuzzNvme reached it.
Step 5.5: SEARCH FOR SIMILAR PATTERNS
Record: Same driver already ratelimits device-driven errors
(`nvme_log_error`, `nvme_log_err_passthru`, PCI timeout
`dev_warn_ratelimited`). Apple still has an unratelimited “invalid id”
`dev_warn`; TCP/RDMA/loop have their own unratelimited “bad command_id”
prints. Those leftovers do not block this fix.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Record: **Yes.** Local tree is **6.12.111** (`git describe HEAD` →
`p-6.12-3-gc504cf0baa3d8`, `make kernelversion` → 6.12.111).
- `drivers/nvme/host/nvme.h:598-607` still uses `pr_err` / `dev_err`.
- `drivers/nvme/host/pci.c:1128-1130` still uses `dev_warn`.
- Introducing commits are ancestors of HEAD. Bug present since v5.15
(find_rq) / v5.9 (PCI invalid-id print).
Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Record: **Clean apply (line-number offset only).** The 6.12
`nvme_find_rq()` body matches the hunk. `nvme_handle_cqe()` differs
later (`trace_nvme_sq` lacks the newer `data_race()` comment in this
tree), but that is after the changed warn. No conflicting refactor.
Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
Record: No commit in this tree ratelimits these three messages. No
alternate fix found.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
Record: **NVMe host** (`drivers/nvme/host/`). **IMPORTANT** — primary
block storage on servers, workstations, and many embedded/Android
platforms. Not core mm/VFS, but a very common production driver.
Step 7.2: ASSESS SUBSYSTEM ACTIVITY
Record: Active in this 6.12 branch (recent PCI IRQ, doorbell, quirk, and
core lifetime fixes). The buggy prints themselves have been unchanged
since 2021.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
Step 8.1: DETERMINE WHO IS AFFECTED
Record: **Driver-specific**, all NVMe host transports via
`nvme_find_rq()`; PCI users are the main hard-IRQ case. Large user
population whenever a controller posts invalid completions (faulty
firmware, reset/teardown, emulator, fuzzer, passthrough VM).
Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Record:
- Trigger: CQE with a command id that does not resolve, or genctr
mismatch; also every completion after tag-set teardown.
- Commonness: rare on healthy hardware; realistic on failing devices,
resets, and VMs. FuzzNvme hit it in one boot.
- Unprivileged userspace: not via a normal syscall on a healthy disk. A
VM user with an emulated/passthrough NVMe device, or a malicious
controller, can drive it. Device-controlled printk from IRQ is a host
DoS vector in those setups.
Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Record:
- **Failure mode:** Unbounded `printk` from hard IRQ / completion
context. Can stall on `console_lock`, flood the log, and contribute to
soft lockups on slow consoles. Not a UAF/oops by itself.
- **Severity: HIGH** (IRQ-context flood / hang risk), not CRITICAL (no
panic/corruption on the success path).
Step 8.4: CALCULATE RISK-BENEFIT RATIO
Record:
- **Benefit:** High for NVMe users who hit a dying, resetting, or
emulated controller — stops IRQ printk storms that have been
demonstrated in practice.
- **Risk:** Very low (3 call-site swaps to existing ratelimit helpers).
- **Ratio:** Strongly favors backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
Step 9.1: COMPILE THE EVIDENCE
**FOR:**
- Real, reproduced bug (1815 lines in one boot; FuzzNvme).
- Hard-IRQ, device-controlled printk — known lockup/DoS class.
- After tag-set teardown the miss path is permanent; print rate = device
CQE rate.
- Tiny, obviously correct, matches existing NVMe style.
- Maintainer (Keith Busch) applied it with agreement, no NAK.
- Buggy code present in this 6.12.111 tree since v5.15/v5.9.
- Applies cleanly; no prerequisites.
- `nvme_find_rq()` is shared across PCI/TCP/RDMA/Apple/loop.
**AGAINST:**
- Not a crash, UAF, or data-corruption fix.
- Healthy devices rarely hit the path.
- Transport-local “bad command_id” prints in tcp/rdma/apple/loop stay
unratelimited (incomplete sibling coverage, not a dependency).
**UNRESOLVED:**
- lore.kernel.org 403; discussion recovered from Patchew/Openwall.
- `b4 dig -c` could not run against a local SHA (commit not in this
tree).
- No stable-list discussion found (search blocked).
Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? **PASS** — three `_ratelimited` swaps;
maintainer applied; FuzzNvme reproduction.
2. Fixes a real bug that affects users? **PASS** — documented IRQ printk
flood; teardown path makes it unbounded.
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
IRQ-context flood can stall the console / soft-lockup; device-driven
DoS. Severity HIGH.
4. Small and contained? **PASS** — ~9/+8 lines, two files, error prints
only.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — buggy code present; helpers
present; clean apply expected.
Step 9.3: CHECK FOR EXCEPTION CATEGORIES
Record: Not a device-ID / quirk / DT / build / docs exception. Qualifies
as a normal small stability fix.
Step 9.4: MAKE YOUR DECISION
This belongs on 6.12.y. The tree still has the unratelimited IRQ prints,
the fix is surgical, and the failure mode is a real host-stability
problem on failing or emulated NVMe — not a feature or cleanup.
Detailed explanation:
The problem is unbounded `printk` from NVMe completion handling when the
device supplies a command id that does not map to an in-flight request.
That path runs from `nvme_irq()` in hard interrupt context (or the
equivalent completion callbacks on other transports). After the tag set
is torn down, every CQE misses, so a broken controller can print as fast
as it can post completions. That is a real production concern during
reset/remove of a failing disk and in VMs; FuzzNvme already produced
thousands of lines in one boot. Rate-limiting those three diagnostics
matches `nvme_log_error()` and does not change I/O completion logic.
Risk is negligible; benefit is avoiding IRQ-context log storms on a
widely deployed storage stack.
Verification:
- [Phase 1] Parsed tags: author SOB Chao Shi; maintainer SOB Keith
Busch; no Fixes/Cc stable/Reported-by (expected).
- [Phase 1] Body: concrete counts (846/846/123) and IRQ path `nvme_irq`
→ `nvme_poll_cq` → `nvme_handle_cqe`.
- [Phase 2] Diff: `pr_err`/`dev_err`/`dev_warn` → `_ratelimited` in
`nvme_find_rq` and `nvme_handle_cqe` only.
- [Phase 3] `git blame drivers/nvme/host/nvme.h`: prints from
`e7006de6c23803` (v5.15-rc1).
- [Phase 3] `git blame drivers/nvme/host/pci.c`: invalid-id print from
`50b7c24390a53c` (v5.9-rc7).
- [Phase 3] `git merge-base --is-ancestor e7006de6c23803 HEAD` and
`50b7c24390a53c HEAD`: both ancestors.
- [Phase 3] `git log --grep="ratelimit the completion-path" HEAD --
drivers/nvme/`: empty; fix not in this tree.
- [Phase 4] `b4 dig` needs a local commit-ish; candidate is not in this
checkout — UNVERIFIED via b4.
- [Phase 4] Patchew + Openwall: v1 applied by Keith Busch to nvme-7.3;
no NAK.
- [Phase 4] lore.kernel.org fetch: 403 — discussion taken from
Patchew/Openwall instead.
- [Phase 5] Callers of `nvme_find_rq`: pci, tcp, rdma, apple,
target/loop (grep).
- [Phase 5] `nvme_poll_cq` called from `nvme_irq`,
`nvme_poll_irqdisable`, `nvme_poll`, `nvme_reap_pending_cqes`.
- [Phase 5] `queue_request_irq()` registers `nvme_irq` as the hard-IRQ
handler (or thread fn).
- [Phase 5] Ratelimit macros present in `include/linux/printk.h` and
`include/linux/dev_printk.h`; already used in `nvme_log_error()`.
- [Phase 6] `git describe HEAD` / `make kernelversion`: 6.12.111
(`p-6.12-3-gc504cf0baa3d8`).
- [Phase 6] Read `nvme.h:598-607` and `pci.c:1126-1131`: unratelimited
prints still present.
- [Phase 8] Failure mode: device-driven printk flood from hard IRQ;
severity HIGH (lockup/DoS), not oops/UAF.
- UNVERIFIED: whether this SHA already exists on some other remote
branch (not required; local tree does not have the fix).
- UNVERIFIED: stable mailing-list history (lore/stable 403).
**YES**
drivers/nvme/host/nvme.h | 6 +++---
drivers/nvme/host/pci.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index ec03a07617af1..b28c1a82f79e6 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -672,12 +672,12 @@ static inline struct request *nvme_find_rq(struct blk_mq_tags *tags,
rq = blk_mq_tag_to_rq(tags, tag);
if (unlikely(!rq)) {
- pr_err("could not locate request for tag %#x\n",
- tag);
+ pr_err_ratelimited("could not locate request for tag %#x\n",
+ tag);
return NULL;
}
if (unlikely(nvme_genctr_mask(nvme_req(rq)->genctr) != genctr)) {
- dev_err(nvme_req(rq)->ctrl->device,
+ dev_err_ratelimited(nvme_req(rq)->ctrl->device,
"request %#x genctr mismatch (got %#x expected %#x)\n",
tag, genctr, nvme_genctr_mask(nvme_req(rq)->genctr));
return NULL;
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 4efb55f1e3833..6e4261b0a0821 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1375,9 +1375,9 @@ static inline void nvme_handle_cqe(struct nvme_queue *nvmeq,
req = nvme_find_rq(nvme_queue_tagset(nvmeq), command_id);
if (unlikely(!req)) {
- dev_warn(nvmeq->dev->ctrl.device,
- "invalid id %d completed on queue %d\n",
- command_id, le16_to_cpu(cqe->sq_id));
+ dev_warn_ratelimited(nvmeq->dev->ctrl.device,
+ "invalid id %d completed on queue %d\n",
+ command_id, le16_to_cpu(cqe->sq_id));
return;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH AUTOSEL 6.18-5.15] nvmet: zns: reject full zone report when buffer is too small
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-5.10] nvme: ratelimit the completion-path messages driven by device data Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 14:15 ` sashiko-bot
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] nvme: bound ns descriptor header and body to identify buffer Sasha Levin
` (7 subsequent siblings)
11 siblings, 1 reply; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Xixin Liu, Christoph Hellwig, Keith Busch, Sasha Levin, sagi, kch,
linux-nvme, linux-kernel
From: Xixin Liu <liuxixin@kylinos.cn>
[ Upstream commit 86985da12699360a2b20748c5a492ddd92db8c47 ]
Zone Management Receive uses the Partial Report (PR) bit in dword 13. On a
partial report (PR bit set), the host accepts an incomplete listing and
Number of Zones must not exceed the zone descriptors copied to the host
buffer. On a full report (PR bit clear), Number of Zones is the total
number of matching zones and every descriptor must fit in the buffer (ZNS
Command Set Specification Rev 1.2, section 3.4.2).
nvmet_bdev_zone_zmgmt_recv_work() already caps Number of Zones for partial
reports, but on a full report it may still succeed when the buffer only
holds part of the matching descriptors. Reject the command in that case.
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111** (`pending-6.12`). The buggy Zone
Management Receive path is present here, and this is a small spec-
compliance fix that should be backported to this tree.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
Step 1.1: PARSE THE SUBJECT LINE
- Subsystem prefix: `nvmet: zns:`
- Action verb: **reject** (prevents an illegal success path)
- Claimed intent: fail a full (PR=0) zone report when the host buffer
cannot hold every matching descriptor
Record: [nvmet/zns] [reject] [fail full zone report when the data buffer
is too small]
Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
- Signed-off-by: Xixin Liu <liuxixin@kylinos.cn> (author)
- Reviewed-by: Christoph Hellwig <hch@lst.de> (NVMe/block maintainer)
- Signed-off-by: Keith Busch <kbusch@kernel.org> (NVMe maintainer;
applied the patch)
- No Fixes: tag (expected for this review pipeline)
- No Reported-by / Tested-by / Link / Cc: stable
- Ignore any pipeline-added Sasha Levin SOB / Upstream markers
Record: Author SOB + Hellwig Reviewed-by + Busch SOB. No syzbot, no user
report, no Fixes tag.
Step 1.3: ANALYZE THE COMMIT BODY TEXT
Zone Management Receive uses the Partial Report (PR) bit:
- PR=1 (partial): host accepts an incomplete listing; Number of Zones
must not exceed descriptors actually copied.
- PR=0 (full): Number of Zones is the total match count; every matching
descriptor must fit (ZNS Command Set Spec Rev 1.2 §3.4.2). If the
buffer is too small, the command must be aborted with Invalid Field in
Command.
`nvmet_bdev_zone_zmgmt_recv_work()` already caps Number of Zones for
PR=1, but for PR=0 it can still complete successfully when only some
matching descriptors were copied.
Record:
- Bug: full report succeeds with incomplete descriptors while Number of
Zones is the full match count
- Symptom: initiator sees SUCCESS plus a header that claims more zones
than were transferred
- Version info: none in the message; code dates to 2021
- Root cause: PR=0 path never checks `nr_zones > out_nr_zones`
Step 1.4: DETECT HIDDEN BUG FIXES
Not hidden. This is an explicit protocol-correctness fix: illegal
SUCCESS → INVALID_FIELD | DNR.
Record: [Not a disguised cleanup; it is a command-handling bug fix]
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
Step 2.1: INVENTORY THE CHANGES
- File: `drivers/nvme/target/zns.c`
- Scope: ~10 insertions / 3 deletions in one function
- Function: `nvmet_bdev_zone_zmgmt_recv_work()`
- Classification: single-file surgical fix
Record: [zns.c, ~+10/-3] [nvmet_bdev_zone_zmgmt_recv_work] [surgical]
Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
Before (this tree, lines 301–309):
```301:309:drivers/nvme/target/zns.c
/*
- When partial bit is set nr_zones must indicate the number of zone
- descriptors actually transferred.
*/
if (req->cmd->zmr.pr)
rz_data.nr_zones = min(rz_data.nr_zones,
rz_data.out_nr_zones);
nr_zones = cpu_to_le64(rz_data.nr_zones);
status = nvmet_copy_to_sgl(req, 0, &nr_zones, sizeof(nr_zones));
```
After:
- PR=1: same cap as today
- PR=0 and `nr_zones > out_nr_zones`: set `error_loc` to `numd`, return
`NVME_SC_INVALID_FIELD | NVME_STATUS_DNR`, skip writing the header
- Otherwise: unchanged success path
The callback still copies only descriptors that fit (`if (rz->nr_zones <
rz->out_nr_zones)`), then increments `nr_zones` for every match. So
after `blkdev_report_zones()`, `nr_zones` is the match count and
`out_nr_zones` is buffer capacity. That comparison is exactly the
missing check.
Record: [PR=0 undersized buffer: SUCCESS + inflated nr_zones →
INVALID_FIELD | DNR]
Step 2.3: IDENTIFY THE BUG MECHANISM
Category: **logic / protocol correctness** (not UAF, leak, or race).
Mechanism: PR=0 must fail when the buffer cannot hold every matching
descriptor. The target currently reports SUCCESS and writes Number of
Zones = full match count while copying only `out_nr_zones` descriptors.
A host that iterates `report->nr_zones` entries then reads past its
buffer.
The status and `error_loc` match the existing too-small-header check in
`nvmet_bdev_validate_zone_mgmt_recv()` (lines 164–166).
Record: [Logic/spec] [PR=0 truncated full report incorrectly succeeds]
Step 2.4: ASSESS THE FIX QUALITY
Obviously correct against the spec and against the existing PR=1 cap.
Minimal. No API/data-structure change.
Regression risk is low:
- Linux kernel host always sets `c.zmr.pr = NVME_REPORT_ZONE_PARTIAL` in
`nvme_ns_report_zones()` — that path is unchanged
- PR=0 with an adequate buffer is unchanged
- Only PR=0 with a too-small buffer flips SUCCESS → error, which is the
specified behavior
- Failed commands already `goto out` in this function; `error_loc` /
`NVME_STATUS_DNR` are used throughout this file
Record: [High quality, surgical] [Low regression risk; only the illegal
success path changes]
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
Step 3.1: BLAME THE CHANGED LINES
`git blame -L 267,313 drivers/nvme/target/zns.c`: every line of this
function is from
`aaf2e048af2704` — `nvmet: add ZBD over ZNS backend support` (Chaitanya
Kulkarni, 2021-06-09)
`git merge-base --is-ancestor aaf2e048af2704 v6.12` succeeded. The bug
has been in this series from the start of 6.12 (and since ~v5.14).
Record: [Introduced by aaf2e048af2704, 2021] [Present in this 6.12 tree]
Step 3.2: FOLLOW THE FIXES: TAG
No Fixes: tag. The introducing commit is the original ZNS target
feature, not a later regression.
Record: [No Fixes tag] [Buggy code is the original 2021 implementation,
ancestor of HEAD]
Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
Recent `drivers/nvme/target/zns.c` history in this tree: identify NULL
deref, DNR rename, conv_zones_bitmap, etc. None already implement this
PR=0 rejection. Current source still has the old comment and the `if
(req->cmd->zmr.pr)` cap only.
Standalone 1/1 (cover letter on lore confirms a single patch).
Record: [No prior equivalent fix in this file] [Standalone; no series
prerequisites]
Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Xixin Liu has other NVMe work in this tree (tcp/rdma), but this ZNS recv
check is not already here. Review weight comes from Hellwig + Busch, not
from the author being the subsystem maintainer.
Record: [Contributor, not nvmet maintainer] [Reviewed/applied by NVMe
maintainers]
Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Diff uses `req->error_loc`, `offsetof(struct nvme_zone_mgmt_recv_cmd,
numd)`, `NVME_SC_INVALID_FIELD | NVME_STATUS_DNR`. All exist in this
tree (`NVME_STATUS_DNR` via `dd0b0a4a2c5d7`, already in 6.12). Context
in `zns.c` matches the pre-image exactly.
Record: [No missing helpers] [Applies standalone to this tree]
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
lore.kernel.org WebFetch returned 403. Thread recovered from the gnuweeb
mirror:
https://lore-kernel.gnuweeb.org/linux-
nvme/cover.1783579899.git.liuxixin@kylinos.cn/T/
- v1: 2026-07-09, nested `else { if (...) }`
- Hellwig: line too long; use `else if`
- v2: 2026-07-13, matches the candidate diff
- Hellwig: “Looks good” + Reviewed-by
- Busch: “Thanks, applied to nvme-7.3.”
- No NAK, no stable discussion, no security/crash report
- Author: “Tested on nvmet-tcp loopback with io-passthru.”
`b4 dig -c` was not used: the candidate SHA is not in this checkout.
Thread contents were verified from the archive instead.
Record: [v1→v2 style-only] [Hellwig RB, Busch applied] [No NAK / no Cc:
stable in review]
Step 4.2: CHECK WHO REVIEWED THE PATCH
Original To/Cc: linux-nvme, hch, sagi (Sagi Grimberg), kch (Chaitanya
Kulkarni). Right maintainers were on the thread.
Record: [Hellwig reviewed; Busch applied; Kulkarni/Grimberg CCed]
Step 4.3: SEARCH FOR THE BUG REPORT
No Reported-by, bugzilla, or syzbot. Same author also posted a host-side
defensive patch (`nvme: zns: cap zone report nr_zones by DMA buffer
size`) because hosts that trust Number of Zones can walk past the DMA
buffer. That is independent; this tree’s host `zns.c` has no
`max_in_buf` yet, and Linux host uses PR=1 anyway.
Record: [No end-user crash report] [Author also wrote a host-side bound
against inflated nr_zones]
Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Cover is 0/1 + 1/1. Self-contained. Host cap patch is a separate
hardening change, not a prerequisite for the target rejection.
Record: [Standalone target fix] [Host companion not required here]
Step 4.5: CHECK STABLE MAILING LIST HISTORY
lore.kernel.org/stable search also 403. No evidence in the nvme thread
that stable was requested or rejected.
Record: [No stable-list discussion found] [UNVERIFIED: could not query
lore.kernel.org/stable directly]
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF
Record: [`nvmet_bdev_zone_zmgmt_recv_work`]
Step 5.2: TRACE CALLERS
- `nvmet_bdev_execute_zone_mgmt_recv()` queues that work
- Set as `req->execute` from `nvmet_bdev_zns_parse_io_cmd()` on
`nvme_cmd_zone_mgmt_recv`
- `nvmet_parse_io_cmd()` → `nvmet_bdev_zns_parse_io_cmd()` when
`req->ns->csi == NVME_CSI_ZNS` and `CONFIG_BLK_DEV_ZONED`
Record: [I/O path for ZNS namespaces on nvmet] [Triggered by Zone
Management Receive from an initiator]
Step 5.3: TRACE CALLEES
`nvmet_bdev_validate_zone_mgmt_recv`, `blkdev_report_zones` /
`nvmet_bdev_report_zone_cb`, `nvmet_copy_to_sgl`, `nvmet_req_complete`.
No new locks or allocations.
Record: [Existing report/copy/complete helpers only]
Step 5.4: FOLLOW THE CALL CHAIN
Reachable from NVMe-oF (TCP/RDMA/FC/loop) against a ZNS target
namespace. Userspace tools (`nvme zns report-zones`, libnvme, SPDK) can
send PR=0. Linux in-kernel host always uses PR=1, so it does not hit the
bug, but other initiators can.
Record: [Userspace/NVMe-oF reachable] [Linux kernel host does not use
PR=0]
Step 5.5: SEARCH FOR SIMILAR PATTERNS
Only this one PR=0/PR=1 split in nvmet. Host always sets
`NVME_REPORT_ZONE_PARTIAL`. No sibling driver with the same missing
full-report check.
Record: [Single instance in nvmet ZNS]
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Yes. `make kernelversion` = 6.12.111; `git describe HEAD` =
`p-6.12-3-gc504cf0baa3d8`. Current `nvmet_bdev_zone_zmgmt_recv_work()`
still only caps PR=1. Introducer `aaf2e048af2704` is an ancestor of
v6.12 and of HEAD.
Record: [Bug exists at drivers/nvme/target/zns.c:305-306] [Since
aaf2e048af2704 / ~5.14]
Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Pre-image matches this file exactly (`OLD FOUND` in a dry-run of the
hunk). `NVME_STATUS_DNR` and `numd` exist. No refactor in the way.
Record: [Expected clean apply]
Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
`git log --grep` on this file found no “reject full zone report”. Host
`max_in_buf` is also absent. No equivalent fix.
Record: [Not already present]
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
`drivers/nvme/target/zns.c` — NVMe target ZNS backend, built as
`nvmet-$(CONFIG_BLK_DEV_ZONED)`.
Record: [nvmet ZNS] [IMPORTANT for NVMe-oF zoned exports; not core
mm/VFS]
Step 7.2: ASSESS SUBSYSTEM ACTIVITY
nvmet is actively fixed in this 6.12 tree (TCP OOB, auth, RDMA leaks,
ZNS identify NULL deref). ZNS recv itself has been stable since 2021.
Record: [nvmet active; this function long-unchanged]
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
Step 8.1: DETERMINE WHO IS AFFECTED
Config-specific: `CONFIG_NVME_TARGET=y` and `CONFIG_BLK_DEV_ZONED=y`,
exporting a zoned bdev as a ZNS namespace. Initiators that send Zone
Management Receive with PR=0 and a buffer smaller than the matching
descriptor list.
Record: [Config-specific: nvmet + zoned bdev + PR=0 full report]
Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Deterministic: PR=0 + `numd` too small for every matching zone. Not a
race. Linux kernel host does not trigger it (PR=1). Userspace and non-
Linux NVMe-oF initiators can. Needs access to the subsystem, not a
random unprivileged syscall on a machine that is not an NVMe target.
Record: [Deterministic protocol path] [Common for tools that request a
full report with a small buffer] [Not a local unprivileged syscall
unless nvmet is configured]
Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Target kernel: no oops, leak, or deadlock — wrong SUCCESS instead of
Invalid Field.
Initiator: SUCCESS + Number of Zones = full match count + only a prefix
of descriptors transferred. A parser that trusts that count walks past
its buffer (userspace OOB / wrong zone map). Incomplete zone inventory
can then drive wrong zone management.
Record: [Incorrect SUCCESS / truncated full report] [Severity: MEDIUM —
protocol/data-integrity for zoned NVMe-oF, not a target oops]
Step 8.4: CALCULATE RISK-BENEFIT RATIO
Benefit: spec-correct failure instead of a silently truncated zone map;
avoids initiator over-read and incomplete inventory. Small but real for
nvmet ZNS users.
Risk: ~10 lines, error path only, same status pattern already used in
this file. Linux in-kernel host path unchanged.
Record: [Benefit: medium for affected nvmet ZNS users] [Risk: very low]
[Favor backport]
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
Step 9.1: COMPILE THE EVIDENCE
FOR:
- Real, deterministic spec violation in command completion (ZNS 1.2
§3.4.2)
- SUCCESS with Number of Zones > descriptors transferred can make
initiators over-read or act on an incomplete zone map
- ~10-line surgical change, obviously correct
- Hellwig Reviewed-by, Busch applied; author tested nvmet-tcp loopback
- Buggy code has been in this 6.12 tree since the original ZNS target
code (2021)
- Applies cleanly; all symbols exist (`NVME_STATUS_DNR`, `numd`,
`error_loc`)
- No new API/feature
- Fits stable-kernel-rules “oh, that's not good” for storage protocol
results
AGAINST:
- Not a target oops/UAF/deadlock
- No syzbot or production Reported-by
- Niche: nvmet + zoned + PR=0
- Linux in-kernel host always uses PR=1
- No Cc: stable (not a negative under these review rules)
UNRESOLVED:
- Could not `b4 dig` a local SHA (commit not in this checkout)
- lore.kernel.org and git.kernel.org fetches were blocked; thread
verified via gnuweeb mirror
- Exact mainline SHA not confirmed locally
Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? **PASS** — spec-aligned; Hellwig RB;
author tested loopback
2. Fixes a real bug that affects users? **PASS** — wrong SUCCESS on a
real NVMe-oF command
3. Important issue? **PASS** (MEDIUM) — not a kernel oops, but incorrect
zoned inventory / initiator over-read; “oh, that's not good” for a
storage protocol
4. Small and contained? **PASS** — one function, ~10 lines
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — pre-image matches 6.12.111
`zns.c`
Step 9.3: CHECK FOR EXCEPTION CATEGORIES
Not a device-ID, quirk, DT, build, or docs exception. It is a normal
small bugfix.
Step 9.4: MAKE YOUR DECISION
This 6.12 tree still has the original PR=0 hole. The fix is tiny,
maintainer-reviewed, and changes only the illegal success path. That is
appropriate for this stable tree.
Verification:
- [Phase 1] Tags: Xixin Liu SOB, Christoph Hellwig Reviewed-by, Keith
Busch SOB; no Fixes/Reported-by/Cc: stable
- [Phase 1] Body: PR=0 full report must fail when buffer cannot hold
every matching descriptor (ZNS 1.2 §3.4.2)
- [Phase 2] Diff: one hunk in `nvmet_bdev_zone_zmgmt_recv_work()`; PR=1
still capped; PR=0 adds `nr_zones > out_nr_zones` →
`NVME_SC_INVALID_FIELD | NVME_STATUS_DNR`
- [Phase 2] Callback already copies only `out_nr_zones` descriptors
while counting all matches — verified at
drivers/nvme/target/zns.c:227-246
- [Phase 3] `git blame`: function from aaf2e048af2704 (2021-06-09)
- [Phase 3] `git merge-base --is-ancestor aaf2e048af2704 HEAD` and `...
v6.12`: both true
- [Phase 3] No equivalent fix in `git log -- drivers/nvme/target/zns.c`
- [Phase 4] Thread: https://lore-kernel.gnuweeb.org/linux-
nvme/cover.1783579899.git.liuxixin@kylinos.cn/T/ — v1→v2 style only;
Hellwig RB; Busch applied to nvme-7.3
- [Phase 4] UNVERIFIED: b4 dig (SHA not in tree); lore.kernel.org HTTP
403
- [Phase 5] Callers: `nvmet_parse_io_cmd` →
`nvmet_bdev_zns_parse_io_cmd` → `nvmet_bdev_execute_zone_mgmt_recv`
(core.c:954-961, zns.c:315-318, 614-615)
- [Phase 5] Linux host uses `c.zmr.pr = NVME_REPORT_ZONE_PARTIAL`
(host/zns.c:197)
- [Phase 6] `make kernelversion` = 6.12.111; buggy `if
(req->cmd->zmr.pr)` still present; dry-run pre-image found;
`NVME_STATUS_DNR` and `numd` exist
- [Phase 7] Built only with `nvmet-$(CONFIG_BLK_DEV_ZONED)`
- [Phase 8] Failure mode: incorrect SUCCESS / truncated full report, not
a target crash
- UNVERIFIED: whether a later mainline follow-up changed this hunk after
nvme-7.3 apply
The problem is a spec-incorrect SUCCESS on a full zone report when the
host buffer is too small. That is a real command-handling bug in this
6.12 tree, the patch is small and already matches local code, and the
risk is limited to failing a command that was already returning
inconsistent data.
**YES**
drivers/nvme/target/zns.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 95af4b67108c7..abf3435ca2900 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -299,11 +299,18 @@ static void nvmet_bdev_zone_zmgmt_recv_work(struct work_struct *w)
}
/*
- * When partial bit is set nr_zones must indicate the number of zone
- * descriptors actually transferred.
+ * Partial report (PR bit set): the host accepts an incomplete listing,
+ * so cap Number of Zones to the descriptors that fit in the buffer.
+ * Full report (PR bit clear): Number of Zones is the match count; fail
+ * if the buffer cannot hold every matching zone descriptor.
*/
- if (req->cmd->zmr.pr)
+ if (req->cmd->zmr.pr) {
rz_data.nr_zones = min(rz_data.nr_zones, rz_data.out_nr_zones);
+ } else if (rz_data.nr_zones > rz_data.out_nr_zones) {
+ req->error_loc = offsetof(struct nvme_zone_mgmt_recv_cmd, numd);
+ status = NVME_SC_INVALID_FIELD | NVME_STATUS_DNR;
+ goto out;
+ }
nr_zones = cpu_to_le64(rz_data.nr_zones);
status = nvmet_copy_to_sgl(req, 0, &nr_zones, sizeof(nr_zones));
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.15] nvmet: zns: reject full zone report when buffer is too small
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.15] nvmet: zns: reject full zone report when buffer is too small Sasha Levin
@ 2026-10-19 14:15 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-19 14:15 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-nvme
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] nvmet: zns: broken PR=0 zone reports in nvmet_bdev_zone_zmgmt_recv_work()
--
commit 42bb4c679b1e6318e7ed82bbe79461a255997505
Author: Xixin Liu <liuxixin@kylinos.cn>
nvmet: zns: reject full zone report when buffer is too small
This patch updates nvmet_bdev_zone_zmgmt_recv_work() in
drivers/nvme/target/zns.c to fail Zone Management Receive commands with
NVME_SC_INVALID_FIELD | NVME_STATUS_DNR when the Partial Report (PR) bit is
clear and rz_data.nr_zones exceeds rz_data.out_nr_zones.
> diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
> index 15a579cf528c1..dfa9269b59992 100644
> --- a/drivers/nvme/target/zns.c
> +++ b/drivers/nvme/target/zns.c
> @@ -299,11 +299,18 @@ static void nvmet_bdev_zone_zmgmt_recv_work(struct work_struct *w)
> }
>
> /*
> - * When partial bit is set nr_zones must indicate the number of zone
> - * descriptors actually transferred.
> + * Partial report (PR bit set): the host accepts an incomplete listing,
> + * so cap Number of Zones to the descriptors that fit in the buffer.
> + * Full report (PR bit clear): Number of Zones is the match count; fail
> + * if the buffer cannot hold every matching zone descriptor.
> */
> - if (req->cmd->zmr.pr)
> + if (req->cmd->zmr.pr) {
> rz_data.nr_zones = min(rz_data.nr_zones, rz_data.out_nr_zones);
> + } else if (rz_data.nr_zones > rz_data.out_nr_zones) {
> + req->error_loc = offsetof(struct nvme_zone_mgmt_recv_cmd, numd);
> + status = NVME_SC_INVALID_FIELD | NVME_STATUS_DNR;
> + goto out;
> + }
[Severity: High]
Won't rejecting req->cmd->zmr.pr == 0 commands when rz_data.nr_zones >
rz_data.out_nr_zones break valid non-partial Zone Management Receive
queries?
Per the NVMe Zoned Namespace Command Set specification (section 3.4.2), the
Partial Report (PR) bit controls the value placed in the Number of Zones
field of the 64-byte Report Zones header (struct nvme_zone_report):
- When PR is 0, Number of Zones indicates the total number of zones matching
the ZRASF criteria from SLBA to the end of the namespace, regardless of the
size of the data buffer, while only as many zone descriptors as fit in the
buffer are transferred.
- When PR is 1, Number of Zones indicates the number of zone descriptors
actually transferred in the buffer.
The only NUMD buffer size condition that aborts the command with Invalid
Field in Command is when the buffer is smaller than the 64-byte Report Zones
header itself, which nvmet_bdev_validate_zone_mgmt_recv() already checks:
drivers/nvme/target/zns.c:nvmet_bdev_validate_zone_mgmt_recv() {
...
if (out_bufsize < sizeof(struct nvme_zone_report)) {
req->error_loc = offsetof(struct nvme_zone_mgmt_recv_cmd, numd);
return NVME_SC_INVALID_FIELD | NVME_STATUS_DNR;
}
...
}
The existing helpers in drivers/nvme/target/zns.c are built around this
behavior. In particular, get_nr_zones_from_buf() explicitly allows a 64-byte
header-only buffer (returning out_nr_zones == 0) so an initiator can issue a
PR=0 command to discover the total number of matching zones before allocating
a full descriptor buffer:
drivers/nvme/target/zns.c:get_nr_zones_from_buf() {
if (bufsize <= sizeof(struct nvme_zone_report))
return 0;
return (bufsize - sizeof(struct nvme_zone_report)) /
sizeof(struct nvme_zone_descriptor);
}
And nvmet_bdev_report_zone_cb() copies descriptors into the host SGL only
while rz->nr_zones < rz->out_nr_zones, yet continues incrementing
rz->nr_zones across all remaining matching zones specifically to compute the
total match count for PR=0:
drivers/nvme/target/zns.c:nvmet_bdev_report_zone_cb() {
...
if (rz->nr_zones < rz->out_nr_zones) {
...
status = nvmet_copy_to_sgl(rz->req, rz->out_buf_offset, &zdesc,
sizeof(zdesc));
if (status)
return -EINVAL;
rz->out_buf_offset += sizeof(zdesc);
}
rz->nr_zones++;
return 0;
}
With this new check in nvmet_bdev_zone_zmgmt_recv_work(), any 64-byte
header-only PR=0 query (out_nr_zones == 0) fails with NVME_SC_INVALID_FIELD |
NVME_STATUS_DNR whenever at least one zone matches, and any bounded-buffer
PR=0 query fails after nvmet_bdev_report_zone_cb() has already written
out_nr_zones descriptors into the host SGL.
>
> nr_zones = cpu_to_le64(rz_data.nr_zones);
> status = nvmet_copy_to_sgl(req, 0, &nr_zones, sizeof(nr_zones));
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-69-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] nvme: bound ns descriptor header and body to identify buffer
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.15] nvmet: zns: reject full zone report when buffer is too small Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] nvme-auth: use crypto_memneq for DH-HMAC-CHAP response comparison Sasha Levin
` (6 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Hari Mishal, Christoph Hellwig, Greg Kroah-Hartman, Keith Busch,
Sasha Levin, axboe, sagi, linux-nvme, linux-kernel
From: Hari Mishal <harimishal1@gmail.com>
[ Upstream commit 3c568b35a0d309acb40746552bec2af24cd550ef ]
nvme_identify_ns_descs() allocates a buffer and gives it to the
controller, which populates it and then iterates the buffer with
variable byte increments that vary by type and body size. But, there is
no bounds check inside the iteration itself except the loop bound
itself. Fix this by checking and stopping iteration if the next header
or its declared body would go past the buffer itself.
Assisted-by: gkh_clanker_t1000
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[nvme]** **[bound]** bound namespace descriptor header and
body to the Identify buffer so iteration cannot walk off the end.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Assisted-by:** gkh_clanker_t1000 (LLM-assisted; not a substitute for
review)
- **Reviewed-by:** Christoph Hellwig \<hch@lst.de\> (NVMe maintainer)
- **Signed-off-by:** Hari Mishal \<harimishal1@gmail.com\> (author)
- **Signed-off-by:** Greg Kroah-Hartman \<gregkh@linuxfoundation.org\>
(submitted the series)
- **Signed-off-by:** Keith Busch \<kbusch@kernel.org\> (NVMe maintainer;
applied)
- **No Fixes:** tag (expected for this review pipeline)
- **No Cc: stable** (expected)
- **No Reported-by / Tested-by / Link / syzbot**
- Ignore pipeline markers: none in the supplied text beyond the usual
SOBs
Notable: reviewed and applied by the two primary NVMe maintainers.
Submitted as **[PATCH 1/2]** with an unrelated sibling (FDP clamp).
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `nvme_identify_ns_descs()` allocates a 4096-byte Identify
buffer, lets the controller fill it, then walks variable-length
namespace ID descriptors. The only guard was `pos <
NVME_IDENTIFY_DATA_SIZE`.
- **Symptom/failure mode:** out-of-bounds header/body access when a
descriptor header sits near the end of the buffer, or `nidl` claims a
body that extends past 4096 bytes. That is a heap OOB read (possible
crash, KASAN report, or adjacent-heap leak via `memcpy` in
`nvme_process_ns_desc()`).
- **Version info:** none in the message. The walker has existed since
2017.
- **Root cause:** device-controlled stride (`sizeof(header) + nidl`)
with no check that the next header or declared body still fits in the
allocation.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not a disguised cleanup.** It is an explicit bounds-check fix.
The verb “bound” here means “do not read past the buffer.”
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `drivers/nvme/host/core.c` — **+4 / −0**
- **Function:** `nvme_identify_ns_descs()`
- **Scope:** single-file surgical fix (two `if` + `break` pairs)
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record: **Before:** loop at `pos`, form `cur = data + pos`, read
`cur->nidl` immediately, then call `nvme_process_ns_desc()` which
`memcpy`s the body. **After:** (1) if the 4-byte header would pass the
buffer, stop; (2) if `nidl == 0`, stop (unchanged); (3) if header+body
would pass the buffer, stop; then process. Still an Identify/scan path,
not an error-recovery rewrite. Stopping with `break` matches existing
`nidl == 0` / `len < 0` handling (no new error code for a truncated
trailing descriptor).
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Memory safety / out-of-bounds]** Device-controlled OOB read.
Mechanism, verified against `struct nvme_ns_id_desc` (`nidt`, `nidl`,
reserved — 4 bytes) and `nvme_process_ns_desc()`:
- If `pos` is 4093–4095, reading the header itself is OOB.
- If the header fits but `nidl` does not leave enough remaining bytes,
known-type `memcpy` (EUI64/NGUID/UUID/CSI) reads past the
`kzalloc(4096)` buffer after the length-equals-expected check.
The fix stops before those accesses. Header check is first, so
`cur->nidl` is only read when the header is in-bounds.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: **Obviously correct and minimal.** Check order is right. `nidl`
is `__u8` (max 255); `pos` is non-negative and `< 4096`; `pos + 4 +
nidl` cannot overflow a 32-bit `int`. On spec-compliant lists that fit
in 4096 bytes the new branches never fire. **Regression risk: very
low.** Truncating a malformed last descriptor is safer than reading past
the buffer. If CSI is truncated on a multi-CSS controller, the existing
`!csi_seen` path can still return `-EINVAL` — that is the conservative
failure.
No API, locking, or data-structure change.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record: Loop introduced by **`3b22ba2682b432`** (“nvme: get list of
namespace descriptors”, Johannes Thumshirn, 2017-06-15) — first
contained in **v4.13-rc1**. The two bounds checks are
**`3c568b35a0d309`** (this commit, 2026-07-14). The walker has been in-
tree for many years.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Buggy pattern comes from `3b22ba2682b432`,
which is an ancestor of this checkout.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Same-author sibling **`bc7f75eba5001`** — `nvmet: passthru: fix OOB
reads when parsing ns id descriptor list` (same descriptor-list OOB
class on the target passthru path). Independent file.
- Series sibling **`29261f8bb4166`** — `nvme: clamp FDP nruhsd...` is a
different function (`nvme_query_fdp_info`). **This 1/2 does not depend
on 2/2.**
- Later `core.c` changes (kmalloc_obj, unsigned nsid print,
scan/SRCU/FDP) do not rewrite this loop. **Standalone.**
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Hari Mishal is not an NVMe maintainer; three NVMe commits, all
device-controlled bounds issues. Weight comes from **Christoph Hellwig
Reviewed-by** and **Keith Busch** applying it to nvme-7.3.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **None.** Adds checks in an existing loop.
`nvme_process_ns_desc()`, `NVME_IDENTIFY_DATA_SIZE` (4096), and `struct
nvme_ns_id_desc` already exist here. Applies standalone.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig:** https://patch.msgid.link/2026070931-stopping-
cost-239a@gregkh
- **v1 only** (`b4 dig -a`); applied version is that v1
- Thread (mbox): Christoph: “Looks good: Reviewed-by”; Keith: “Thanks,
series applied to nvme-7.3.”
- **No NAK, no stability concerns, no request for Cc: stable**
WebFetch of lore.kernel.org returned **403**; discussion recovered via
`b4 dig -m`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc included **linux-nvme**, **Keith Busch, Jens
Axboe, Christoph Hellwig, Sagi Grimberg** (all NVM EXPRESS DRIVER
maintainers). Appropriate audience.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No user/syzbot/bugzilla report.** Found by analysis (Greg’s
clanker + author). Still a real, reachable OOB given a malformed
Identify NS descriptor list.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: **[PATCH 1/2]** / **[PATCH 2/2]** share an author and file but
not code. 1/2 is self-contained. Target passthru fix is a related class,
not a prerequisite.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable WebFetch **403**. **UNVERIFIED** whether
stable@ already discussed this hash. Absence of Cc: stable is expected
here and is not a negative.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: KEY FUNCTIONS**
Record: **`nvme_identify_ns_descs()`** modified;
**`nvme_process_ns_desc()`** callee (memcpy of descriptor bodies).
**Step 5.2: TRACE CALLERS**
Record:
- Only caller: `nvme_scan_ns()` in `drivers/nvme/host/core.c`
- `nvme_scan_ns()` ← `nvme_scan_ns_async()` /
`nvme_scan_ns_sequential()`
- Those ← `nvme_scan_work()` on `ctrl->scan_work`
Context: **process-context workqueue**, NVMe namespace scan on probe,
reset, and NS-changed AEN. Common path for NVMe hosts, not IRQ/hot-
cache.
**Step 5.3: TRACE CALLEES**
Record: `kzalloc` / `nvme_submit_sync_cmd` / `nvme_process_ns_desc` /
`kfree`. The OOB is in the parse loop and in `nvme_process_ns_desc()`
memcpy, not in the admin command itself.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable on any NVMe 1.3+ controller (or multi-CSS) that does
not have `NVME_QUIRK_NO_NS_DESC_LIST`. Userspace does not call this
syscall-direct; a **malicious or buggy controller** (local PCIe or NVMe-
oF target) controls the buffer. That is a standard untrusted-device
memory-safety bug.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same class already bounded in `drivers/nvme/target/passthru.c`
(`pos + sizeof(*cur)` and CSI body). Host-side
`nvme_identify_ns_descs()` was the gap this commit closes.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Local tree: **`git describe HEAD` = `v7.3-rc4-27-gf0100363d8c37`**,
**`make kernelversion` = 7.3.0-rc4**
- Function and loop exist at `drivers/nvme/host/core.c`
(`nvme_identify_ns_descs`)
- Bug introduced **v4.13** (`3b22ba2682b432`), long before this branch
- The two checks are present on HEAD (this commit is an ancestor). That
is historical fact, **not** used as “already selected for stable”
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** to this `nvme_identify_ns_descs()` shape.
Context is the same 4.13-era loop; no extra helpers required. **Expected
backport difficulty: clean apply** on trees that still have this
function.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No earlier host-side equivalent. Passthru OOB fix is a different
file. No conflicting alternate host fix.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: SUBSYSTEM AND CRITICALITY**
Record: **NVMe host core** (`drivers/nvme/host/core.c`). **IMPORTANT** —
common block/storage path, not mm/VFS core, but widely deployed
(servers, laptops, NVMe-oF).
**Step 7.2: SUBSYSTEM ACTIVITY**
Record: Actively maintained (Keith Busch, Christoph Hellwig, et al.).
This is a small correctness fix in old parse code, not a follow-on to a
brand-new feature in 7.3.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Record: **Driver-specific, but broad:** NVMe hosts scanning namespaces
(almost all NVMe users on 1.3+). Triggered by device Identify data, not
a rare Kconfig.
**Step 8.2: TRIGGER CONDITIONS**
Record: Namespace scan (probe/reset/NS-changed). **Not** every I/O.
Needs a **malformed descriptor list** (buggy firmware or untrusted
target). Unprivileged local users typically cannot inject Identify
payloads without device control; NVMe-oF to an untrusted target can.
Timing-independent; deterministic given bad data.
**Step 8.3: FAILURE MODE SEVERITY**
Record: **Heap out-of-bounds read** → KASAN/oops, possible info leak, or
silent heap corruption adjacent to the Identify buffer. **Severity:
HIGH.** Not a guaranteed panic on production kernels, but it is a real
memory-safety bug in a storage driver.
**Step 8.4: RISK-BENEFIT RATIO**
Record:
- **Benefit: high** — stops device-controlled OOB on a common NVMe scan
path; 4 lines; maintainers acked.
- **Risk: very low** — two range checks; no lock/API/behavior change for
valid lists.
- **Ratio: strongly favors backport.**
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real OOB read from controller-controlled Identify NS descriptor list
- Path is namespace scan on ordinary NVMe hosts
- 4-line, obviously correct bounds checks; header checked before `nidl`
- Same bug class already recognized on the passthru parser
- Reviewed-by Christoph Hellwig; applied by Keith Busch
- Code has existed since v4.13; no missing prerequisites in this tree
- No new feature/API
**AGAINST:**
- No Fixes:/Cc: stable/syzbot/user report (not a negative per the rules)
- LLM-assisted origin (offset by maintainer review)
- Trigger needs a bogus/malicious controller, not typical I/O
- Series labeled 1/2 (sibling is independent)
**UNRESOLVED:**
- lore.kernel.org HTTP 403; thread recovered via b4 mbox
- Whether stable@ already discussed this commit: **UNVERIFIED**
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — two range checks; Reviewed-
by maintainer; applied to nvme-7.3
2. Fixes a real bug that affects users? **PASS** — OOB parse of device
Identify data
3. Important (crash/security/corruption/deadlock)? **PASS** — HIGH
memory-safety (OOB read; crash/leak potential)
4. Small and contained? **PASS** — +4 lines, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — same function/context; no
extra deps
**Step 9.3: EXCEPTION CATEGORIES**
Record: **None** (not an ID/quirk/DT/build/doc exception). It qualifies
as a normal small memory-safety fix.
**Step 9.4: DECISION**
This is a surgical host-side bounds check for a long-standing, device-
controlled OOB in NVMe namespace scanning. It meets stable rules for
this checkout (7.3-rc4; buggy walker present since v4.13). Benefit is
high; regression risk is very low.
---
### What the commit solves (for this tree)
`nvme_identify_ns_descs()` walks a 4096-byte Identify Namespace ID
Descriptor List with a device-chosen stride. Without checking that the
next 4-byte header and `nidl` body fit, a truncated or oversized last
descriptor causes an out-of-bounds header read and/or `memcpy` in
`nvme_process_ns_desc()`. That hits every NVMe scan of a 1.3+ (or multi-
CSS) controller that returns a bad list. The patch stops the loop before
those accesses. Compliant devices are unchanged.
### Risk vs benefit
Four lines of range checks, no locking or ABI change. Worst case on a
truncated list is skipping remaining descriptors (already the `nidl ==
0` policy). Benefit is preventing a heap OOB in a widely used storage
driver.
### Concerns
None that block backport: 1/2 is independent of the FDP 2/2 patch; no
extra commits required; apply is clean against this
`nvme_identify_ns_descs()`.
---
### Verification
- **[Phase 1]** Parsed tags: Reviewed-by Christoph Hellwig; SOB Hari
Mishal, Greg Kroah-Hartman, Keith Busch; Assisted-by
gkh_clanker_t1000; no Fixes/Reported-by/Cc:stable/syzbot
- **[Phase 2]** Diff: +4 lines in `nvme_identify_ns_descs()`; two `pos +
sizeof(*cur)` / `+ cur->nidl` checks vs `NVME_IDENTIFY_DATA_SIZE`
(4096). Confirmed `struct nvme_ns_id_desc` is 4 bytes; `nidl` is
`__u8`
- **[Phase 2]** `nvme_process_ns_desc()` memcpy of EUI64/NGUID/UUID/CSI
only after `nidl` matches expected length — body check is what keeps
those copies in-buffer
- **[Phase 2]** Integer overflow: max `pos+hdr+nidl` ≈ 4354, no 32-bit
overflow
- **[Phase 3]** `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
kernelversion` = 7.3.0-rc4
- **[Phase 3]** Commit `3c568b35a0d309`; `git merge-base --is-ancestor`
→ ancestor of HEAD
- **[Phase 3]** Blame: loop `3b22ba2682b432` (2017, v4.13-rc1); checks
from this commit
- **[Phase 3]** Pre-image `git show 3c568b35a0d30^:...` matches the
supplied diff (no header/body checks)
- **[Phase 3]** Sibling `29261f8bb4166` is `nvme_query_fdp_info()` only
— not a prereq. Related `bc7f75eba5001` is target passthru
- **[Phase 4]** `b4 dig -c 3c568b35a0d309`: lore/msgid
2026070931-stopping-cost-239a@gregkh; v1 only; CC linux-nvme + all
NVMe maintainers
- **[Phase 4]** mbox: Hellwig Reviewed-by; Busch “series applied to
nvme-7.3”; no NAK
- **[Phase 4]** WebFetch lore.kernel.org / patch.msgid.link / lore
stable search: **403** — discussion taken from b4 mbox instead
- **[Phase 5]** Only caller `nvme_scan_ns()` → `nvme_scan_work()`
(probe/reset/NS-changed scan)
- **[Phase 5]** Similar bounds already in
`drivers/nvme/target/passthru.c`
- **[Phase 6]** Buggy walker exists in this tree’s history since v4.13;
current file has the checks; apply would be clean
- **[Phase 6]** MAINTAINERS: NVM EXPRESS DRIVER — Busch, Axboe, Hellwig,
Grimberg
- **[Phase 8]** Failure mode: heap OOB read on malformed Identify NS
descriptor list; severity HIGH
- **UNVERIFIED:** whether stable@ already discussed this commit (lore
stable HTTP 403)
**YES**
drivers/nvme/host/core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d5fb6612fa229..5ed62e070f715 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1564,8 +1564,12 @@ static int nvme_identify_ns_descs(struct nvme_ctrl *ctrl,
for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
struct nvme_ns_id_desc *cur = data + pos;
+ if (pos + sizeof(*cur) > NVME_IDENTIFY_DATA_SIZE)
+ break;
if (cur->nidl == 0)
break;
+ if (pos + sizeof(*cur) + cur->nidl > NVME_IDENTIFY_DATA_SIZE)
+ break;
len = nvme_process_ns_desc(ctrl, &info->ids, cur, &csi_seen);
if (len < 0)
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH AUTOSEL 6.18-6.6] nvme-auth: use crypto_memneq for DH-HMAC-CHAP response comparison
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.10] nvme: bound ns descriptor header and body to identify buffer Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.15] nvme-tcp: look up host_iface in the current netns Sasha Levin
` (5 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: Xixin Liu, Hannes Reinecke, Christoph Hellwig, Keith Busch,
Sasha Levin, hare, axboe, sagi, kch, linux-nvme, linux-kernel
From: Xixin Liu <liuxixin@kylinos.cn>
[ Upstream commit 3a6d89836ab09d53f4c32c57588689a378ce4363 ]
DH-HMAC-CHAP authentication compares HMAC response digests with memcmp().
Standard memcmp() may stop at the first differing byte, which can leak
timing information to a remote attacker and allow incremental recovery
of the expected digest.
Use crypto_memneq() for constant-time comparison on both the host path
that validates the controller Success1 response and the target path that
validates the host Reply digest. Other memcmp() uses in the NVMe auth
code (e.g. fixed string prefix checks) are not security-sensitive and
are left unchanged.
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **v7.3-rc4** (`7.3.0-rc4`, `f0100363d8c37`). The
commit under review is `3a6d89836ab09` (“nvme-auth: use crypto_memneq
for DH-HMAC-CHAP response comparison”). It is already an ancestor of
HEAD (landed in v7.3-rc1). That does not decide the backport question;
the decision below is on technical merit and whether the bug exists in
this lineage.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[nvme-auth]** **[use]** Replace `memcmp()` with
`crypto_memneq()` when comparing DH-HMAC-CHAP HMAC response digests.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Fixes:` — none (expected)
- `Reported-by:` — none
- `Tested-by:` — none
- `Reviewed-by: Hannes Reinecke <hare@kernel.org>` (author of the
original NVMe in-band auth code)
- `Reviewed-by: Christoph Hellwig <hch@lst.de>`
- `Link:` — none in the commit
- `Cc: stable@vger.kernel.org` — none (expected)
- `Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>` (author)
- `Signed-off-by: Keith Busch <kbusch@kernel.org>` (NVMe maintainer)
- Ignore pipeline `Signed-off-by: Sasha Levin` if present; it is not in
this commit
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** DH-HMAC-CHAP compares HMAC response digests with `memcmp()`,
which can return at the first differing byte.
- **Symptom/failure mode:** Timing leak of how far the computed HMAC
matches the expected digest; a remote attacker could try to recover
the expected digest incrementally.
- **Version information:** none in the message. Keith later said on-list
it was applied “for 7.2”; `git merge-base --is-ancestor` shows it is
**not** in `v7.2` and **is** in `v7.3-rc1`.
- **Root cause (author):** variable-time comparison of authentication
MAC material.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. It is an explicit security/correctness
change: secret MAC comparison must be constant-time. The remaining
`memcmp()` uses (string prefixes, GUIDs) are called out as non-sensitive
and left alone.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- `drivers/nvme/host/auth.c`: +2 / −1 (`#include <crypto/utils.h>`;
`memcmp` → `crypto_memneq` in `nvme_auth_process_dhchap_success1`)
- `drivers/nvme/target/fabrics-cmd-auth.c`: +2 / −1 (same include;
`memcmp` → `crypto_memneq` in `nvmet_auth_reply`)
- **Scope:** two-file surgical fix, 4 insertions, 2 deletions. No API,
Kconfig, or data-structure changes.
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Host (`nvme_auth_process_dhchap_success1`):** after `data->hl ==
chap->hash_len` and `data->rvalid`, the host used to
`memcmp(chap->response, data->rval, data->hl)`. Now
`crypto_memneq(...)`. Failure still sets
`NVME_AUTH_DHCHAP_FAILURE_FAILED` and returns `-ECONNREFUSED`.
- **Target (`nvmet_auth_reply`):** after computing the expected host
HMAC into `response`, the target used to `memcmp(data->rval, response,
data->hl)`. Now `crypto_memneq(...)`. Failure still frees `response`
and returns `NVME_AUTH_DHCHAP_FAILURE_FAILED`.
- Affected path: authentication success/reply validation only, not
probe/init/error-unrelated code.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Memory safety / crypto side channel]** Variable-time
`memcmp()` on HMAC digests. `crypto_memneq()` (`include/crypto/utils.h`)
is documented as “Compare two areas of memory without leaking timing
information” and returns 0 if equal, 1 otherwise — same boolean sense as
`if (memcmp(...))`.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct and minimal. Length `data->hl` is already
checked against `chap->hash_len` on the host path. Return-value polarity
matches. Regression risk is very low: same control flow, slightly more
work per compare (HMAC length is 32–64 bytes). No new locks, APIs, or
userspace ABI.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Host `memcmp`: introduced in `f50fff73d620cd` (“nvme: implement In-
Band authentication”, Hannes Reinecke, 2022). **In `v6.0`**, not in
`v5.19`.
- Target `memcmp`: introduced in `db1312dd95488b` (“nvmet: implement
basic In-Band Authentication”). Same merge window, **v6.0**.
- The buggy comparison has been in tree for many years.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. The introducing commits above are the
functional equivalent. Both are ancestors of this tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent work on these files is auth bounds-checking, hashing
updates, and secure-channel concatenation. This commit is standalone
(subject is not `N/M`). No prerequisite refactor is required for the
two-line swap plus include.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Xixin Liu is a contributor, not the NVMe auth maintainer. Weight
comes from **Reviewed-by Hannes Reinecke** (original auth author) and
**Christoph Hellwig**, applied by **Keith Busch**.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Depends only on `crypto_memneq()` / `<crypto/utils.h>`. In this
tree, `include/crypto/utils.h` exists (`c616fb0cbae8`, 2023) and
`CONFIG_CRYPTO` selects `CRYPTO_LIB_UTILS` (`lib/crypto/memneq.c`).
`NVME_AUTH` already `select CRYPTO`. Standalone on this tree.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 3a6d89836ab09` → https://patch.msgid.link/0001.nvme-auth-
memneq.1781751816.git.liuxixin@kylinos.cn
- `b4 dig -a`: **v1 only** (no v2/v3). Applied as submitted.
- Thread (mbox): Christoph: “Looks good” + Reviewed-by. Hannes:
Reviewed-by. No NAK, no stable discussion, no requested changes.
- Keith (2026-09-15): applied earlier; pointed at `3a6d89836ab09`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc included `linux-nvme@lists.infradead.org`,
Keith Busch, Jens Axboe, Christoph Hellwig, Sagi Grimberg, Chaitanya
Kulkarni, Hannes Reinecke. Right maintainers were on the patch.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No `Reported-by`, bugzilla, or syzbot. This is code-audit /
crypto-hygiene, not a user crash report.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Cover was `[PATCH v1 0/1]` / `[PATCH v1 1/1]` — single patch.
Precedent in this tree: `ksmbd: Compare MACs in constant time`
(`c5794709bc910`, had `Cc: stable`); `scsi: target: Use constant-time
crypto_memneq() for CHAP digests` (`cf14fc2be8688`); TPM/SMB client
similar replacements.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** on lore.kernel.org/stable (WebFetch 403,
WebSearch failed). The original NVMe thread never mentions `stable@`.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `nvme_auth_process_dhchap_success1`, `nvmet_auth_reply`.
Includes only otherwise.
**Step 5.2: TRACE CALLERS**
Record:
- `nvme_auth_process_dhchap_success1` ← `nvme_queue_auth_work` in
`drivers/nvme/host/auth.c` (DH-HMAC-CHAP Success1 after AUTH_RECEIVE).
Queued from connect / `dhchap_auth_work` / sysfs re-auth.
- `nvmet_auth_reply` ← `nvmet_execute_auth_send` when the DH-HMAC-CHAP
step is Reply. `nvmet_execute_auth_send` is the fabrics AUTH_SEND
execute handler (`drivers/nvme/target/fabrics-cmd.c`).
**Step 5.3: TRACE CALLEES**
Record: Target computes expected HMAC via `nvmet_auth_host_hash()` (HMAC
over challenge `dhchap_c1`, transformed host key, etc.), then compares.
Host compares precomputed `chap->response` to controller `rval`. No
allocation/locking change in the hunks.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: **Reachable from the network** on an NVMe-oF target with
`CONFIG_NVME_TARGET_AUTH`. An unauthenticated remote host sending
AUTH_SEND Reply hits `nvmet_auth_reply`. Host path is hit when
connecting with in-band auth / bidirectional controller auth. Not a
syscall smash; it is the authentication protocol itself.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: After this commit, no remaining `memcmp` of HMAC material in
`drivers/nvme/host/auth.c` or `drivers/nvme/target/fabrics-cmd-auth.c`.
Other NVMe `memcmp` uses are prefix/GUID/debugfs checks, which the
commit correctly left alone.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes, in this lineage since v6.0. This checkout already contains
the fix (`crypto_memneq` at `drivers/nvme/host/auth.c:365` and
`drivers/nvme/target/fabrics-cmd-auth.c:189`). The pre-fix `memcmp` is
in `3a6d89836ab09^`.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply on this tree** (already applied). Include is
`<crypto/utils.h>`, which this 7.3 tree has. For much older stables the
header was `crypto/algapi.h`; that is not a problem **here**.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This *is* the fix in this tree. No earlier alternate memneq
conversion on these files.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: NVMe over Fabrics in-band authentication
(`drivers/nvme/host/auth.c`, `drivers/nvme/target/fabrics-cmd-auth.c`).
**IMPORTANT** for NVMe-oF deployments that enable auth; not core mm/VFS.
Configs: `NVME_HOST_AUTH` / `NVME_TARGET_AUTH` (help text “If unsure,
say N”), both select `NVME_AUTH`.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (recent OOB/auth bounds fixes on the same
files). Auth itself has been stable in design since 2022.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Config-specific (`NVME_HOST_AUTH` / `NVME_TARGET_AUTH`) and only
during DH-HMAC-CHAP. Target comparison is the access-control check for
remote hosts.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Every in-band auth exchange that validates an HMAC response.
Target path: remote peer during AUTH_SEND Reply. Host path:
bidirectional controller auth when `rvalid` is set. Unprivileged remote
initiator can hit the target compare if the subsystem is enabled.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Timing side channel on authentication MACs.** Severity
**HIGH** as a security defect (auth protocol using variable-time
compare). Practical “byte-by-byte recover the static secret” is weaker
than the commit text implies: `nvmet_auth_host_hash()` HMACs a per-
session challenge (`dhchap_c1`), so the expected digest is not a stable
value across attempts. Same caveat as the SCSI CHAP memneq patch. It is
still the wrong primitive for MAC compare; ksmbd’s analogous change was
`Cc: stable`.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record: **Benefit: high** for NVMe-oF auth users (closes a real class of
crypto bug on a remote-facing check). **Risk: very low** (4 insertions,
same branches, standard helper). Ratio strongly favors applying it.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real crypto bug: `memcmp` on HMAC responses in DH-HMAC-CHAP
- Remote-facing target auth path
- 4-line, obviously correct replacement with the kernel’s constant-time
helper
- Reviewed by Hannes Reinecke and Christoph Hellwig; applied by Keith
Busch
- Bug present since v6.0; `crypto_memneq` already available here
- Same pattern as other MAC compares that stable has taken (e.g. ksmbd)
- No new feature/API; no locking/ABI change
**AGAINST:**
- Optional Kconfig (default N)
- No CVE/syzbot/user report
- Incremental digest recovery is harder than the message states because
HMAC input includes a unique challenge
- No in-thread `Cc: stable`
**UNRESOLVED:**
- Whether stable@ ever discussed this (lore stable search failed)
- Practical remote exploitability over NVMe-oF (network jitter)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — polarity of `crypto_memneq`
matches `memcmp`; reviewed by auth author and NVMe core reviewers; in
mainline since 7.3-rc1.
2. Fixes a real bug that affects users? **PASS** — wrong comparison
primitive on live HMAC auth; users with NVMe-oF in-band auth are
affected.
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
security (timing leak on authentication MAC), HIGH.
4. Small and contained? **PASS** — 2 files, +4/−2.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — already applied;
`<crypto/utils.h>` and `CRYPTO_LIB_UTILS` are present.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. It is a small
security fix, which is in-scope for stable on its own.
**Step 9.4: MAKE YOUR DECISION**
This is a small, maintainer-reviewed security fix for NVMe-oF DH-HMAC-
CHAP: the host Success1 path and the target Reply path compared HMAC
digests with `memcmp()`. That can leak match length. `crypto_memneq()`
is the established kernel helper; control flow is unchanged. The code
has existed since v6.0, applies cleanly here, and does not need extra
commits.
Even if full incremental recovery of a static digest is unlikely because
each HMAC includes a fresh challenge, stable trees should not leave
authentication MACs on a variable-time compare. Risk is negligible;
benefit is closing a real crypto-hygiene hole on a network-facing auth
check.
### What problem it solves and why it matters
DH-HMAC-CHAP is NVMe-oF in-band authentication. The target decides
whether the host’s Reply HMAC is correct; the host (when bidirectional)
decides whether the controller’s Success1 HMAC is correct. Using
`memcmp()` can stop at the first mismatch and leak timing. Stable users
who enable this optional auth still need that check to be constant-time.
### Stable rules
The patch is obviously correct, tiny, already reviewed, and does not add
features. It fixes a security issue, not a style nit.
### Risk vs benefit
Very low regression risk versus a meaningful security improvement for
NVMe-oF auth. Header/`CRYPTO_LIB_UTILS` dependencies are already
satisfied in this 7.3 tree.
### Concerns
Exploitability is weaker than a static-secret `memcmp` leak; feature is
optional. Neither outweighs applying a 4-line, standard crypto fix.
Verification:
- [Phase 1] Parsed commit `3a6d89836ab09`: no Fixes/Reported-
by/Cc:stable; Reviewed-by Hannes Reinecke and Christoph Hellwig; SOB
Keith Busch
- [Phase 2] Diff: +`#include <crypto/utils.h>` and
`memcmp`→`crypto_memneq` in `nvme_auth_process_dhchap_success1` and
`nvmet_auth_reply` (+4/−2)
- [Phase 2] `include/crypto/utils.h`: `crypto_memneq` returns 0 if
equal, 1 otherwise — matches `if (memcmp(...))`
- [Phase 3] `git blame` on parent: host memcmp from `f50fff73d620cd`
(v6.0); target from `db1312dd95488b` (v6.0)
- [Phase 3] `git merge-base --is-ancestor f50fff73d620cd v6.0` yes;
`v5.19` no
- [Phase 3] No Fixes tag to follow
- [Phase 3] Standalone 1/1 series; `NVME_AUTH` selects `CRYPTO`;
`CRYPTO` selects `CRYPTO_LIB_UTILS`
- [Phase 4] `b4 dig -c 3a6d89836ab09`: lore msgid `0001.nvme-auth-
memneq.1781751816.git.liuxixin@kylinos.cn`
- [Phase 4] `b4 dig -a`: v1 only
- [Phase 4] `b4 dig -w`: linux-nvme plus
Busch/Axboe/Hellwig/Grimberg/Kulkarni/Reinecke
- [Phase 4] mbox: Christoph “Looks good”; Hannes Reviewed-by; no NAK;
Keith: applied as `3a6d89836ab09`
- [Phase 4] WebFetch lore 403; WebSearch failed; stable-list discussion
UNVERIFIED
- [Phase 5] Callers: host auth workqueue; target
`nvmet_execute_auth_send` ← fabrics AUTH_SEND (remote)
- [Phase 5] `nvmet_auth_host_hash` HMACs `dhchap_c1` (per-session
challenge) — verified in `drivers/nvme/target/auth.c`
- [Phase 5] No leftover HMAC `memcmp` in the two auth files
- [Phase 6] `git describe HEAD`: `v7.3-rc4-27-gf0100363d8c37`; `make
kernelversion`: 7.3.0-rc4
- [Phase 6] `git merge-base --is-ancestor 3a6d89836ab09 HEAD` yes; not
in `v7.2`; yes in `v7.3-rc1`
- [Phase 6] Local files already use `crypto_memneq` at the two call
sites
- [Phase 8] Failure mode: timing leak on auth MAC compare; severity HIGH
(security); trigger: NVMe-oF in-band auth
- UNVERIFIED: practical remote exploitability; whether stable@ already
discussed the patch
**YES**
drivers/nvme/host/auth.c | 3 ++-
drivers/nvme/target/fabrics-cmd-auth.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index 8f3ccb317e4de..9d73713d6f828 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -9,6 +9,7 @@
#include <linux/unaligned.h>
#include <crypto/hash.h>
#include <crypto/dh.h>
+#include <crypto/utils.h>
#include "nvme.h"
#include "fabrics.h"
#include <linux/nvme-auth.h>
@@ -381,7 +382,7 @@ static int nvme_auth_process_dhchap_success1(struct nvme_ctrl *ctrl,
return 0;
/* Validate controller response */
- if (memcmp(chap->response, data->rval, data->hl)) {
+ if (crypto_memneq(chap->response, data->rval, data->hl)) {
dev_dbg(ctrl->device, "%s: qid %d ctrl response %*ph\n",
__func__, chap->qid, (int)chap->hash_len, data->rval);
dev_dbg(ctrl->device, "%s: qid %d host response %*ph\n",
diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index 35bf1a2357d49..5b010318286e4 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -10,6 +10,7 @@
#include <linux/nvme-auth.h>
#include <crypto/hash.h>
#include <crypto/kpp.h>
+#include <crypto/utils.h>
#include "nvmet.h"
static void nvmet_auth_expired_work(struct work_struct *work)
@@ -187,7 +188,7 @@ static u8 nvmet_auth_reply(struct nvmet_req *req, void *d, u32 tl)
return NVME_AUTH_DHCHAP_FAILURE_FAILED;
}
- if (memcmp(data->rval, response, data->hl)) {
+ if (crypto_memneq(data->rval, response, data->hl)) {
pr_info("ctrl %d qid %d host response mismatch\n",
ctrl->cntlid, req->sq->qid);
pr_debug("ctrl %d qid %d rval %*ph\n",
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH AUTOSEL 6.18-5.15] nvme-tcp: look up host_iface in the current netns
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (5 preceding siblings ...)
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.6] nvme-auth: use crypto_memneq for DH-HMAC-CHAP response comparison Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 14:43 ` sashiko-bot
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18] nvme-tcp: delay nvme_tcp_reclassify_socket() Sasha Levin
` (4 subsequent siblings)
11 siblings, 1 reply; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: Geliang Tang, Hannes Reinecke, Keith Busch, Sasha Levin, axboe,
sagi, linux-nvme, linux-kernel
From: Geliang Tang <tanggeliang@kylinos.cn>
[ Upstream commit 58202950e39c127d593ec4f0624d8b8985a285b1 ]
nvme_tcp_alloc_ctrl() looks opts->host_iface up in &init_net, the boot-time
netns. When called from any other netns - e.g. the selftest's ns2, where
ns2eth1 actually lives - the lookup misses and the controller setup fails
with "invalid interface passed":
nvmet: adding nsid 1 to subsystem nqn.2014-08.org.nvmexpress.mptcpdev
nvmet_tcp: enabling port 24660 (0.0.0.0:24099)
# nvme discover -a 10.1.1.1 --tos=0x10 --host-iface=ns2eth1
nvme_tcp: invalid interface passed: ns2eth1
# failed to add controller, error invalid interface
Look the device up in current->nsproxy->net_ns instead so the check sees
the calling task's netns.
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This is a one-line netns-correctness fix in existing NVMe-TCP `host-
iface` validation. The buggy lookup is in this 6.12 tree; the patch
applies cleanly.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
Step 1.1: PARSE THE SUBJECT LINE
Record: [nvme-tcp] [look up] [validate `host_iface` in the calling
task’s netns instead of `init_net`]
Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
Record:
- Reviewed-by: Hannes Reinecke \<hare@kernel.org\> (NVMe/storage
maintainer)
- Signed-off-by: Geliang Tang \<tanggeliang@kylinos.cn\> (author; MPTCP
contributor)
- Signed-off-by: Keith Busch \<kbusch@kernel.org\> (NVMe maintainer;
applied the patch)
- No Fixes:, Reported-by, Tested-by, Link:, or Cc: stable (absence
expected for this review)
- Ignore pipeline markers (Upstream commit / Sasha Levin SOB) — none in
the origin commit `58202950e39c1`
Step 1.3: ANALYZE THE COMMIT BODY TEXT
Record:
- Bug: `nvme_tcp_alloc_ctrl()` looks up `opts->host_iface` with
`__dev_get_by_name(&init_net, ...)`. In any other netns the name is
missing, so controller setup fails.
- Symptom: `nvme_tcp: invalid interface passed: ns2eth1` then `failed to
add controller, error invalid interface`. Concrete repro: `nvme
discover -a 10.1.1.1 --tos=0x10 --host-iface=ns2eth1` from netns
`ns2`.
- Failure mode: connect/discover abort with `-ENODEV`. Not a crash,
leak, or corruption.
- Root cause: hardcoded boot netns instead of the calling task’s netns.
- Version info: none in the message. The log line
`nqn.2014-08.org.nvmexpress.mptcpdev` is from an MPTCP NVMe selftest
that is not in this tree; the mechanism is not MPTCP-specific.
Step 1.4: DETECT HIDDEN BUG FIXES
Record: Not a disguised cleanup. It is an explicit correctness fix:
wrong netns for a name lookup. No hidden leak/race/UAF.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
Step 2.1: INVENTORY THE CHANGES
Record:
- File: `drivers/nvme/host/tcp.c` (+2/−1)
- Function: `nvme_tcp_alloc_ctrl()`
- Scope: single-file, one-line surgical change
Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
Record: In the `NVMF_OPT_HOST_IFACE` validation path, existence check
was `__dev_get_by_name(&init_net, opts->host_iface)`. After:
`__dev_get_by_name(current->nsproxy->net_ns, opts->host_iface)`. Normal
connect path only; if the name is missing, still `-ENODEV` /
`out_free_ctrl`.
Step 2.3: IDENTIFY THE BUG MECHANISM
Record:
- Category: logic/correctness (wrong network namespace)
- Mechanism: interface names are per-netns. Lookup in `init_net` rejects
names that only exist in the caller’s netns (veths, container NICs).
The later `SO_BINDTODEVICE` already uses `sock_net(sk)`
(`net/core/sock.c` `sock_setbindtodevice()`), and `sock_create()`
already uses `current->nsproxy->net_ns`. Only this existence check was
wrong.
- Not UAF/leak/race/crash. Pre-existing `__dev_get_by_name` without
RTNL/RCU is unchanged (comment in `net/core/dev.c` says RTNL is
required). Later MPTCP RFCs added RCU; mainline did not.
Step 2.4: ASSESS THE FIX QUALITY
Record: Obviously correct and consistent with `sock_create()`. Minimal.
`current->nsproxy` is valid here: `nvme_tcp_alloc_ctrl()` is only called
from `nvme_tcp_create_ctrl()` → `nvmf_create_ctrl()` →
`nvmf_dev_write()` (userspace write to `/dev/nvme-fabrics`), not from
reconnect workqueues. Regression risk very low: a caller that relied on
init_net names while running in another netns would still fail later at
`SO_BINDTODEVICE`.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
Step 3.1: BLAME THE CHANGED LINES
Record:
- Feature: `3ede8f72a9a28` (“nvme-tcp: allow selecting the network
interface for connections”), 2021-06-03, first in v5.14-rc1. Used
`dev_get_by_name(&init_net, ...)`.
- Switch to `__dev_get_by_name`: `8b43ced64d2bac` (2021-07-13), Fixes:
3ede8f72, to avoid holding a netdev ref that blocked unregister. Still
`&init_net`.
- Both are ancestors of this 6.12 tree. Bug present since the feature
landed.
Step 3.2: FOLLOW THE FIXES: TAG
Record: No Fixes: tag. Buggy pattern comes from `3ede8f72a9a28` /
`8b43ced64d2bac`, both in HEAD.
Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
Record: Recent `tcp.c` history in this tree is TLS/PDU hardening, not
host_iface. Origin after this commit has no further host_iface change on
this file. Standalone v1, not “patch X/Y”.
Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Record: Geliang Tang is an MPTCP contributor, not an NVMe maintainer. In
this tree his nvme work is unrelated (`nvmet-tcp` leak, unused
variable). On origin this is his only `tcp.c` host_iface change. Weight
comes from Hannes’s Reviewed-by and Keith applying it.
Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Record: No new helpers. `__dev_get_by_name` and
`current->nsproxy->net_ns` exist here. Applies standalone (hunk
succeeded at offset 32). `inet_pton_with_scope(&init_net, ...)` for
addresses is left as-is; that is a separate scope issue, not a
dependency.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Record:
- SHA: `58202950e39c127d593ec4f0624d8b8985a285b1` (not in this tree; on
origin)
- b4 dig: https://patch.msgid.link/c14cd4633c27abe2dec863480168691023694
aee.1785033817.git.tanggeliang@kylinos.cn
- Thread (gnuweeb mirror; lore.kernel.org returned 403/bot wall):
standalone `[PATCH]`, 2026-07-26
- b4 dig -a: v1 only; applied version is that v1
- Hannes Reinecke 2026-07-27: Reviewed-by. Note that discovery-log
addresses in a netns may not be reachable from the initiator; not a
NAK of this lookup fix.
- Keith Busch 2026-08-10: “Thanks, applied to nvme-7.3.”
- No NAK, no Cc: stable in the thread
- Earlier MPTCP RFC series combined netns + RCU (`dev_get_by_name_rcu` /
`netdev_name_in_use`); the committed patch is the netns one-liner only
Step 4.2: CHECK WHO REVIEWED THE PATCH
Record: b4 dig -w recipients: Keith Busch, Jens Axboe, Christoph
Hellwig, Sagi Grimberg, Hannes Reinecke, linux-nvme. Appropriate
maintainers. Hannes reviewed; Keith applied.
Step 4.3: SEARCH FOR THE BUG REPORT
Record: No syzbot/bugzilla. Evidence is the author’s selftest log
(`ns2eth1` in ns2). That MPTCP NVMe selftest is not in this 6.12 tree.
Bug is still real for any netns + `host-iface`.
Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Record: Applied as a standalone linux-nvme patch, not as part of the
MPTCP NVMe series. No sibling patch required for this lookup.
Step 4.5: CHECK STABLE MAILING LIST HISTORY
Record: gnuweeb `/stable/` search 404; lore.kernel.org 403. No verified
stable-list discussion. UNVERIFIED: whether anyone already nominated or
rejected this for stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF
Record: `nvme_tcp_alloc_ctrl()` only.
Step 5.2: TRACE CALLERS
Record:
- `nvme_tcp_alloc_ctrl` ← `nvme_tcp_create_ctrl` (`.create_ctrl` of
`nvme_tcp_transport`)
- ← `nvmf_create_ctrl()` in `drivers/nvme/host/fabrics.c`
- ← `nvmf_dev_write()` (misc device write)
- Userspace: nvme-cli `connect`/`discover` writing to `/dev/nvme-
fabrics`
- Reconnect (`nvme_tcp_setup_ctrl` / `nvme_tcp_alloc_queue`) does not
call `alloc_ctrl`; it reuses `opts->host_iface` on `SO_BINDTODEVICE`
Step 5.3: TRACE CALLEES
Record: `__dev_get_by_name(net, name)` → `netdev_name_node_lookup()`.
Existence check only; no ref taken (that was the point of
`8b43ced64d2bac`).
Step 5.4: FOLLOW THE CALL CHAIN
Record: Reachable from userspace via `/dev/nvme-fabrics` when
`host_iface=` is set. Privileged fabrics connect, not an unprivileged
syscall smash. In this tree `nvme_tcp_alloc_queue()` uses
`sock_create()` → `current->nsproxy->net_ns`, then `sock_setsockopt(...,
SO_BINDTODEVICE, ...)` which looks up the name in `sock_net(sk)`.
Validation was the only `init_net` step on that path.
Step 5.5: SEARCH FOR SIMILAR PATTERNS
Record: Only this `__dev_get_by_name` in `drivers/nvme/`. RDMA transport
has no `NVMF_OPT_HOST_IFACE`. `inet_pton_with_scope(&init_net, ...)`
remains for `traddr`/`host_traddr` (numeric parse; different issue).
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Record: Yes. `drivers/nvme/host/tcp.c:3000`:
`if (!__dev_get_by_name(&init_net, opts->host_iface))`
Tree: pending-6.12, `make kernelversion` 6.12.111, `git describe HEAD`
p-6.12-3-gc504cf0baa3d8. Bug since v5.14. `git merge-base --is-ancestor
58202950e39c1 HEAD` is false (fix not present).
Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Record: `git apply --check` of origin diff: hunk succeeded at offset 32.
Local context still `kcalloc` (mainline later `kzalloc_objs`); that is
after the hunk, not in it. Expected: clean apply / trivial offset.
Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
Record: No later host_iface netns fix. `1be52169` (sock_create_kern) and
`b013b817` (netns UAF on kernel TCP socket) are not in this tree; this
patch does not depend on them.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
Record: NVMe over Fabrics TCP host (`drivers/nvme/host/tcp.c`,
`CONFIG_NVME_TCP`). Criticality: IMPORTANT — datacenter remote block
storage, not core mm/vfs, not staging.
Step 7.2: ASSESS SUBSYSTEM ACTIVITY
Record: Actively maintained (TLS, PDU validation, etc.). `host_iface`
itself has been stable since 5.14 with only the 2021 refcount change
until this 2026 netns fix.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
Step 8.1: DETERMINE WHO IS AFFECTED
Record: Users of NVMe-TCP who pass `host_iface=` from a non-init netns,
when that name is not also in `init_net`. Driver-specific + config
(`CONFIG_NVME_TCP`) + optional option. Typical host-net `eth0` cases
often pass the init_net check by accident; unique names (`ns2eth1`,
container veths) fail.
Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Record: Write connect/discover options including `host_iface=<name>`
from a process whose netns is not `init_net`. Not every boot; only this
option. Needs fabrics connect privilege, not unprivileged
exploitability.
Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Record: Controller creation fails (`-ENODEV`); no NVMe device. No oops,
hang, or corruption. Severity: MEDIUM (availability of an existing
option), HIGH for those who need interface pinning in a netns.
Step 8.4: CALCULATE RISK-BENEFIT RATIO
Record:
- Benefit: medium for netns/container NVMe-TCP with `host-iface`; low
for default host-net users
- Risk: very low (1-line, same helper, same error path)
- Ratio: favorable — almost no regression surface, real connectivity bug
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
Step 9.1: COMPILE THE EVIDENCE
FOR:
- Real, reproduced failure: `host-iface` rejected in non-init netns
- Feature and buggy code exist in 6.12 since v5.14
- Matches how sockets are created and bound in this tree
- One-line, obviously correct; applies with offset only
- Reviewed by Hannes Reinecke; applied by Keith Busch
- No new API/feature; restores intended `host-iface` behavior
- Storage connect failure is user-visible for affected deployments
(containers, netns isolation, multi-homed pinning)
AGAINST:
- Not a crash, UAF, corruption, deadlock, or security bug
- Optional `host-iface` plus non-init netns is a relatively narrow path
- Motivating selftest (MPTCP NVMe) is not in 6.12
- Went into nvme-7.3 updates, not a -fixes pull; no stable nomination in
the thread
- Hannes noted broader discovery/netns limitations (not a NAK)
UNRESOLVED:
- Stable mailing list discussion (lore blocked / gnuweeb stable 404)
- Whether distro/k8s NVMe-TCP actually uses `host-iface` from a
container netns (plausible, not counted)
Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? PASS — one-line netns fix; maintainer
Reviewed-by; author reproduced
2. Fixes a real bug that affects users? PASS — connect fails with
`-ENODEV` for a valid interface in the caller’s netns
3. Important issue? PASS — availability of NVMe-TCP when `host-iface` is
used from a netns; not a panic, but a functional showstopper on that
path. Small correctness fixes of this kind are routine in stable
4. Small and contained? PASS — +2/−1 in one function
5. No new features or APIs? PASS
6. Can apply to the local tree? PASS — buggy line present; apply-check
succeeded at offset 32
Step 9.3: CHECK FOR EXCEPTION CATEGORIES
Record: None (not a device ID, quirk, DT, build, or docs fix). Qualifies
as a normal small bugfix.
Step 9.4: MAKE YOUR DECISION
The local 6.12 tree still looks `host_iface` up in `init_net` while
creating the TCP socket in the caller’s netns. That is a long-standing,
self-contained correctness bug. Backporting this line restores
connect/discover with `--host-iface` from containers and other netns
without new infrastructure. Risk is negligible compared with leaving
NVMe-TCP interface pinning broken off host netns.
===============================================================
What the commit solves for this tree
===============================================================
`host_iface` is supposed to force NVMe-TCP onto a named NIC via
`SO_BINDTODEVICE`. Validation used `&init_net`, so a name that only
exists in the calling netns is treated as invalid and controller setup
aborts. In 6.12 the socket is already created with `sock_create()`
(current netns) and bind-to-device already searches that netns. The
check is the broken piece. Stable users who pin NVMe-TCP to a NIC from a
netns/container hit a hard connect failure; this is the whole fix.
Risk vs benefit: benefit is restoring an existing option in netns; risk
is a one-line lookup change with the same `-ENODEV` path. No
prerequisites. `inet_pton_with_scope(&init_net)` is untouched and not
required for this fix.
===============================================================
Verification
===============================================================
- [Phase 1] Parsed origin commit `58202950e39c1`: Reviewed-by Hannes
Reinecke; SOB Geliang Tang + Keith Busch; no Fixes/Cc-stable
- [Phase 2] Diff: +2/−1 in `nvme_tcp_alloc_ctrl()`; `&init_net` →
`current->nsproxy->net_ns`
- [Phase 2] Confirmed `sock_setbindtodevice()` uses `sock_net(sk)`;
`sock_create()` uses `current->nsproxy->net_ns` (`net/socket.c:1614`)
- [Phase 3] blame: `3ede8f72a9a28` (v5.14-rc1) introduced init_net
lookup; `8b43ced64d2bac` switched to `__dev_get_by_name`, still
init_net; both ancestors of HEAD
- [Phase 3] `git merge-base --is-ancestor 58202950e39c1 HEAD` → not an
ancestor (fix absent)
- [Phase 4] `b4 dig -c 58202950e39c1`: lore msgid `c14cd4633c27abe2...`;
v1 only
- [Phase 4] `b4 dig -w`: linux-nvme maintainers on To/Cc
- [Phase 4] Hannes: Reviewed-by plus discovery/netns remark, no NAK;
Keith: applied to nvme-7.3
- [Phase 4] lore.kernel.org 403; used lore-kernel.gnuweeb.org.
UNVERIFIED: stable@ thread
- [Phase 5] Callers via grep: only `nvme_tcp_create_ctrl` →
`nvmf_dev_write`; reconnect does not call `alloc_ctrl`
- [Phase 6] `git describe HEAD` p-6.12-3-gc504cf0baa3d8; `make
kernelversion` 6.12.111; buggy line at `tcp.c:3000`
- [Phase 6] `git apply --check` of origin diff: succeeded at offset 32
- [Phase 6] `1be52169` / `b013b817` not in this tree; not required
- [Phase 8] Failure: `-ENODEV` / invalid interface; severity MEDIUM–HIGH
availability, not crash
- UNVERIFIED: production hit rate of host-iface from container netns;
MPTCP selftest not in this tree
**YES**
drivers/nvme/host/tcp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index beef2b576102a..1f26e35d7a99e 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2999,7 +2999,8 @@ static struct nvme_tcp_ctrl *nvme_tcp_alloc_ctrl(struct device *dev,
}
if (opts->mask & NVMF_OPT_HOST_IFACE) {
- if (!__dev_get_by_name(&init_net, opts->host_iface)) {
+ if (!__dev_get_by_name(current->nsproxy->net_ns,
+ opts->host_iface)) {
pr_err("invalid interface passed: %s\n",
opts->host_iface);
ret = -ENODEV;
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.15] nvme-tcp: look up host_iface in the current netns
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.15] nvme-tcp: look up host_iface in the current netns Sasha Levin
@ 2026-10-19 14:43 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-19 14:43 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-nvme
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] nvme-tcp: use-after-free in nvme_tcp_alloc_ctrl() via __dev_get_by_name()
--
commit 04aa71fe23c7dec86f240fa7773c4020c75d2bb7
Author: Geliang Tang <geliang@kernel.org>
nvme-tcp: look up host_iface in the current netns
This commit changes nvme_tcp_alloc_ctrl() to look up opts->host_iface in
current->nsproxy->net_ns rather than init_net so that interfaces in the
calling task's network namespace can be found.
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index 9a96df1a511c0..fb6858023f19d 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -2935,7 +2935,8 @@ static struct nvme_tcp_ctrl *nvme_tcp_alloc_ctrl(struct device *dev,
> }
>
> if (opts->mask & NVMF_OPT_HOST_IFACE) {
> - if (!__dev_get_by_name(&init_net, opts->host_iface)) {
> + if (!__dev_get_by_name(current->nsproxy->net_ns,
> + opts->host_iface)) {
[Severity: Critical]
Is it safe to call __dev_get_by_name() here without holding rtnl_lock()?
When userspace writes connection parameters with host_iface=<name> to
/dev/nvme-fabrics, the call chain:
nvmf_dev_write()
-> nvmf_create_ctrl()
-> nvme_tcp_create_ctrl()
-> nvme_tcp_alloc_ctrl()
runs in sleepable process context holding only nvmf_dev_mutex, with
neither rtnl_lock() nor rcu_read_lock() held.
In net/core/dev.c, __dev_get_by_name() requires the RTNL semaphore and
calls netdev_name_node_lookup(), which traverses the per-netns hash bucket
with non-RCU list primitives:
net/core/dev.c:netdev_name_node_lookup() {
struct hlist_head *head = dev_name_hash(net, name);
struct netdev_name_node *name_node;
hlist_for_each_entry(name_node, head, hlist)
if (!strcmp(name_node->name, name))
return name_node;
return NULL;
}
net/core/dev.c:__dev_get_by_name() {
struct netdev_name_node *node_name;
node_name = netdev_name_node_lookup(net, name);
return node_name ? node_name->dev : NULL;
}
If a network device in current->nsproxy->net_ns is concurrently
unregistered via unregister_netdevice_many_notify() ->
rollback_registered_many() (which unlinks the node in unlist_netdevice(),
waits for an RCU grace period via synchronize_net(), and then frees the
nodes via netdev_name_node_alt_flush() and netdev_name_node_free()), has an
altname deleted via netdev_name_node_alt_destroy() ->
__netdev_name_node_alt_destroy(), or is renamed via dev_change_name(),
neither synchronize_net() nor call_rcu() will wait for a preempted reader
in nvme_tcp_alloc_ctrl().
Could this trigger a use-after-free on name_node or name_node->name during
the hash list traversal, and should this lookup use dev_get_by_name_rcu()
under rcu_read_lock() (or netdev_name_in_use()) instead?
> pr_err("invalid interface passed: %s\n",
> opts->host_iface);
> ret = -ENODEV;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-231-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH AUTOSEL 6.18] nvme-tcp: delay nvme_tcp_reclassify_socket()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (6 preceding siblings ...)
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.15] nvme-tcp: look up host_iface in the current netns Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 14:50 ` sashiko-bot
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.12] nvme: handle positive error codes in nuse_show() Sasha Levin
` (3 subsequent siblings)
11 siblings, 1 reply; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Eric Dumazet, Nilay Shroff, Shin'ichiro Kawasaki, Keith Busch,
Sasha Levin, axboe, sagi, linux-nvme, linux-kernel
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit 447a7a3026eda85ac2738cbe6ee5ab066a39dad0 ]
Commit 841aee4d75f1 ("nvme-tcp: lockdep: annotate in-kernel sockets")
introduced nvme_tcp_reclassify_socket() to resolve a lockdep WARN by
distinguishing userspace sockets from nvme-tcp sockets. After that, TLS
encryption support was introduced to nvme-tcp and it added another lock
dependency on userspace socket handling by tlshd, which resulted in a
new lockdep WARN. This WARN was hidden by the commit 19bdb70c77d3
("nvme-tcp: lockdep: use dynamic lockdep keys per socket instance").
However, it turned out the commit 19bdb70c77d3 has a bug of lockdep key
lifetime management, and it is to be reverted by another patch in this
series. After the revert, the WARN was unveiled and observed at the
blktests test case nvme/062:
WARNING: possible circular locking dependency detected
7.3.0-rc2+ #455 Tainted: G W
------------------------------------------------------
tlshd/75867 is trying to acquire lock:
ffffffff91ada5e0 (fs_reclaim){+.+.}-{0:0}, at: __kmalloc_cache_noprof+0x64/0x6d0
but task is already holding lock:
ffff888197808258 (sk_lock-AF_INET-NVME){+.+.}-{0:0}, at: do_tcp_setsockopt+0x499/0x26a0
which lock already depends on the new lock.
To fix the lockdep WARN caused by the TLS encryption support, delay the
reclassification of the nvme-tcp sockets. Currently the reclassification
happens before the TLS handshake starts, which pulls in the additional
lock dependencies related to tlshd. Reclassify the sockets after the TLS
handshake instead, to cut that dependency.
Link: https://lore.kernel.org/lkml/CANn89i+wnTLC==UnXCpjsS4YxvEfhe5oK0N7fttbqr1zKyqdug@mail.gmail.com/
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This tree is **6.18.57** (`v6.18.57-155-gcdf239ca885b2`). The question
is whether `nvme-tcp: delay nvme_tcp_reclassify_socket()` should be
backported **here**. The buggy pattern is present in 6.18, the change is
three lines, and it does not depend on the 7.3-era revert that
accompanied it upstream.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[nvme-tcp] [delay] delay nvme_tcp_reclassify_socket() until
after the TLS handshake`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Link: https://lore.kernel.org/lkml/CANn89i+wnTLC==UnXCpjsS4YxvEfhe5oK
0N7fttbqr1zKyqdug@mail.gmail.com/` (Eric Dumazet report of the
lockdep-key lifetime bug)
- `Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>`
- `Signed-off-by: Eric Dumazet <edumazet@google.com>` (author)
- `Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>`
- `Signed-off-by: Keith Busch <kbusch@kernel.org>` (NVMe maintainer)
- No `Fixes:` tag (expected)
- No `Cc: stable` (expected)
- No `Reported-by:` / syzbot tag on this commit itself (the Link is the
related lockdep-key discussion)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** After `841aee4d75f1` put nvme-tcp sockets in a private lock
class, TLS support made `tlshd` operate on an already-reclassified
NVME socket. That pulled userspace TLS lock dependencies into
`sk_lock-AF_INET-NVME`.
- **Symptom:** lockdep `WARNING: possible circular locking dependency
detected` in blktests `nvme/062`. `tlshd` holds `sk_lock-AF_INET-NVME`
and tries to take `fs_reclaim` via `__kmalloc_cache_noprof`.
- **Hiding commit:** `19bdb70c77d3` (dynamic per-socket lockdep keys)
hid the WARN, then proved buggy and is reverted by patch 1/2 of the
same series.
- **Fix:** reclassify after the TLS handshake so `tlshd` runs against
the default socket class.
- **Version in the splat:** 7.3.0-rc2+; that is the mainline
observation, not a claim that 6.18 is unaffected.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. It is an explicit lockdep-isolation
fix. `nvme_tcp_reclassify_socket()` is already a no-op without
`CONFIG_DEBUG_LOCK_ALLOC`, so the runtime effect is lockdep-only.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/nvme/host/tcp.c` (`+2 / -1`)
- Function: `nvme_tcp_alloc_queue()`
- Scope: single-file surgical move of one call
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `sock_create_kern` → `sock_alloc_file` →
`sk_net_refcnt_upgrade` → **`nvme_tcp_reclassify_socket()`** →
sockopts/connect → optional `nvme_tcp_start_tls()` →
`nvme_tcp_init_connection()`
- **After:** same path, but reclassify runs **after** TLS handshake (or
after connect if TLS is not used), still **before**
`nvme_tcp_init_connection()`
- Affected path: queue allocation / controller connect, including the
TLS handshake path
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- Category: lockdep class mixing (userspace `tlshd` vs in-kernel nvme-
tcp), same family as `841aee4d75f1`
- Mechanism: early reclassify makes TLS setsockopt/`kmalloc` look like
NVME-class `sk_lock` → `fs_reclaim`. Combined with existing NVME-class
reclaim dependencies, lockdep reports a cycle.
- Fix: keep handshake on the default `AF_INET` class; switch to NVME
class only once kernel NVMe protocol I/O begins.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: one call moved past the userspace-like handshake.
- Minimal; no API/data-structure change.
- Regression risk is low. `sock_allow_reclassification()` only requires
the socket not to be user-owned or slocked; `nvme_tcp_start_tls()`
waits for handshake completion before returning. Failed TLS goes to
`err_init_connect` without reclassify, which is fine because the
socket is being torn down. Reviewers accepted this; blktests
`nvme/062` was reported to pass.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Reclassify call introduced by `841aee4d75f1` (Chris Leech, 2022-02-15;
in this tree, first appeared around 5.18).
- TLS handshake call introduced by `be8e82caa6859` (Hannes Reinecke,
2023-08-24; `v6.7-rc1`).
- The lockdep WARN exists from TLS introduction onward whenever
reclassify happens first.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Referenced commits:
- `841aee4d75f1`: **is** an ancestor of this tree.
- `19bdb70c77d3`: **is not** an ancestor of HEAD or of `v6.18` (dated
2026-06-04, contained around 7.x). The “hiding” commit never landed
here, so the TLS lockdep WARN was never suppressed in 6.18.y.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: This is **patch 2/2** of Shin'ichiro Kawasaki’s series “nvme-
tcp: do not use dynamic lockdep key for socket instances”:
1. Revert `19bdb70c77d3`
2. This delay of reclassify
Patch 1 is a no-op for this tree (nothing to revert). This delay patch
is standalone on 6.18.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Authored by Eric Dumazet (networking/lockdep); submitted by
Shin'ichiro Kawasaki; applied by Keith Busch. Cover letter: tested on
v7.3-rc2, `nvme/005` and `nvme/062` pass with no lockdep splat.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- Revert of `19bdb70c77d3`: **not needed** here.
- Cover letter also cites `0ba6912f7e97` (“Revert once: don't use a work
queue…”) to cut a **different** chain (`sk_lock → cpu_hotplug_lock`)
for `nvme/005`. That commit is **not** in this tree, and it is **not**
a prerequisite of this TLS reclassify move.
- `nvme_tcp_reclassify_socket()`, `nvme_tcp_start_tls()`, and the
alloc_queue control flow all exist in `v6.18` with the same ordering
the patch expects.
Can this apply standalone? **Yes.**
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 447a7a3026eda85ac2738cbe6ee5ab066a39dad0` → https://patch.m
sgid.link/20260914074853.3990397-3-
shinichiro.kawasaki@opensource.wdc.com
- Cover: https://patch.msgid.link/20260914074853.3990397-1-
shinichiro.kawasaki@opensource.wdc.com
- Single revision (v1 of a 2-patch series). Direct lore.kernel.org fetch
returned 403; full thread was retrieved via `b4 dig -m`.
- No NAKs. Keith Busch: “Thanks, applied to nvme-7.3.”
- No reviewer `Cc: stable` suggestion found in the thread.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): linux-nvme, Keith Busch, Jens Axboe, Christoph
Hellwig, Sagi Grimberg, Eric Dumazet, Nilay Shroff. Nilay: “Looks good
to me.” / `Reviewed-by`. Eric reviewed the companion revert.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: Observed in blktests `nvme/062` (NVMe-TCP TLS). The Link tag is
Eric’s report of the *dynamic key* lifetime WARN that motivated the
revert; this delay patch is the follow-up so that revert does not re-
expose the TLS class-mixing splat. Severity: lockdep warning, not a
reported production hang.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Companion revert is irrelevant to 6.18. A side discussion about
keeping per-queue dynamic keys was dropped; Eric argued static “kernel
vs user” classes are sufficient and per-instance keys hide real AB/BA
bugs. That does not affect this delay patch.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: Web search of lore.kernel.org/stable for this subject did not
turn up a prior stable discussion or an explicit rejection.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `nvme_tcp_alloc_queue()` (call moved); helper
`nvme_tcp_reclassify_socket()` unchanged; handshake still in
`nvme_tcp_start_tls()`.
**Step 5.2: TRACE CALLERS**
Record: `nvme_tcp_alloc_queue()` is called from
`nvme_tcp_alloc_admin_queue()` and `__nvme_tcp_alloc_io_queues()` during
controller setup. Userspace trigger: connecting an NVMe/TCP controller
(`nvme-cli` / nvmf), including TLS PSK when `CONFIG_NVME_TCP_TLS` is
enabled.
**Step 5.3: TRACE CALLEES**
Record: `nvme_tcp_start_tls()` → `tls_client_hello_psk()` + wait for
`tlshd`. Reclassify → `sock_lock_init_class_and_name()` only under
`CONFIG_DEBUG_LOCK_ALLOC`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable from normal NVMe/TCP connect. The lockdep splat needs
lockdep + TLS + `tlshd`. Without lockdep the moved function is an empty
stub in this tree (`static void nvme_tcp_reclassify_socket(struct socket
*sock) { }` in the `#else` of `CONFIG_DEBUG_LOCK_ALLOC`).
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same reclassify pattern exists in sunrpc (`xprtsock.c`,
`svcsock.c`), SMB client, Bluetooth. Those also guard with
`sock_allow_reclassification()`. This is the established in-kernel
socket isolation approach.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** On `v6.18`:
```
1801: nvme_tcp_reclassify_socket(queue->sock); /* immediately after
create */
1885: ret = nvme_tcp_start_tls(...); /* later */
1890: ret = nvme_tcp_init_connection(queue);
```
TLS (`be8e82caa6859`) and static reclassify (`841aee4d75f1`) are both in
6.18. Dynamic keys (`19bdb70c77d3`) are not, so the TLS lockdep WARN is
live here, not hypothetical.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean apply. The hunk is a single function-call move around
`nvme_tcp_start_tls()` / `nvme_tcp_init_connection()`, which have the
same shape in `v6.18`. Later `sockaddr_unsized` / `__fput_sync` churn
does not touch this call.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No equivalent delay of reclassify in 6.18 history. No other TLS
lockdep isolation fix on this path.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: NVMe over Fabrics TCP host (`drivers/nvme/host/tcp.c`).
**IMPORTANT** for NVMe/TCP users; not core mm/VFS. TLS is
`CONFIG_NVME_TCP_TLS` (“If unsure, say N”).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (TLS, digest, security, reconnect). This is
a targeted lockdep fix, not a refactor of new 7.3 infrastructure.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: NVMe/TCP users with `CONFIG_NVME_TCP_TLS` **and**
`CONFIG_DEBUG_LOCK_ALLOC`/`LOCKDEP`, during TLS connect (`tlshd`).
Production configs without lockdep: no functional change (stub
function).
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Controller connect with TLS PSK, lockdep enabled. Reproduced by
blktests `nvme/062`. Not an unprivileged syscall smash; it is a
privileged NVMe fabrics connect path. Timing: every TLS queue setup, not
a rare race.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: lockdep circular-dependency **WARNING** (kernel tainted `W`).
Discussion treats it as class mixing (false cycle from `tlshd` vs in-
kernel NVMe), not a confirmed production deadlock. The same isolation
hole has existed since TLS landed in 6.7; no production hang report was
found for this chain. Severity: **MEDIUM** for debug/CI kernels, **LOW**
for production 6.18 users.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- Benefit: restores the `841aee4d75f1` kernel-vs-user isolation for TLS;
silences a real blktests splat; keeps lockdep usable on NVMe/TCP+TLS.
- Risk: very low (3-line move, no-op without lockdep, reviewed, tested).
- Ratio: favorable. The “important issue” bar is the only weak point,
not correctness or applyability.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Buggy ordering exists in this 6.18 tree (reclassify before TLS).
- Tiny, obviously correct, reviewed by Nilay Shroff, applied by Keith
Busch, idea from Eric Dumazet.
- Tested (blktests `nvme/062`).
- Applies cleanly; no missing helpers or struct changes.
- Companion revert is **not** required here because `19bdb70c77d3` never
landed.
- Zero production behavioral change without lockdep.
- Same class of fix as the existing `841aee4d75f1` annotation; TLS had
re-broken that isolation.
AGAINST:
- Debug-only (`nvme_tcp_reclassify_socket()` is empty without
`CONFIG_DEBUG_LOCK_ALLOC`).
- Not a reported oops, hang, corruption, or CVE.
- Needs optional TLS + `tlshd` + lockdep to fire.
- Written as patch 2/2 of a 7.3 series whose other half does not apply
here (that is not a blocker; it just explains origin).
UNRESOLVED:
- Direct lore.kernel.org HTTP fetch returned 403; thread content was
still verified via `b4 dig -m`.
- No stable-list nomination/rejection found.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** (3-line move; Reviewed-by;
blktests `nvme/062`)
2. Fixes a real bug that affects users? **PASS** (reproducible lockdep
WARN on NVMe/TCP TLS connect; bothers debug/CI users)
3. Important issue? **PASS** (weak but acceptable: lockdep splat / “oh,
that's not good”; restores lock-class isolation that this tree
already relies on)
4. Small and contained? **PASS** (3 lines, one function)
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** (`v6.18` has the same call
sites)
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. Ordinary small
bugfix.
**Step 9.4: MAKE YOUR DECISION**
Backport. The defect is in this tree, the patch is self-contained
without the 7.3 revert, and the risk is negligible. Leaving early
reclassify in place keeps a known lockdep isolation hole in NVMe/TCP TLS
on 6.18.y.
---
### What the commit solves for this tree
On 6.18, nvme-tcp still reclassifies sockets immediately after create,
then may run a TLS handshake via `tlshd`. That puts userspace handshake
locking into the NVME socket class and produces a lockdep circular-
dependency warning (blktests `nvme/062`). Moving reclassify to after
handshake (and after connect when TLS is off) cuts that dependency.
Production kernels without lockdep do not change behavior.
### Stable rules / risk
Meets the written rules: small, obviously correct, tested, no new API,
applies here. Benefit is debug/CI lockdep correctness; regression risk
is essentially zero.
### Concerns
Do not pull in the companion revert of `19bdb70c77d3` (absent here). Do
not treat `0ba6912f7e97` as a dependency of this patch; that is a
separate `nvme/005` lock chain.
---
Verification:
- [Phase 1] Parsed tags: Link to Eric’s lore report; Reviewed-by Nilay
Shroff; SOBs Eric Dumazet, Shin'ichiro Kawasaki, Keith Busch; no
Fixes/Cc:stable
- [Phase 2] Diff: move `nvme_tcp_reclassify_socket()` in
`nvme_tcp_alloc_queue()` from after `sk_net_refcnt_upgrade` to after
optional `nvme_tcp_start_tls()`, +2/-1
- [Phase 2] Confirmed `#else` stub: function is empty without
`CONFIG_DEBUG_LOCK_ALLOC` in this tree
- [Phase 3] `git describe HEAD` / Makefile: 6.18.57
- [Phase 3] `git merge-base --is-ancestor 841aee4d75f1 HEAD`: YES
(reclassify present)
- [Phase 3] `git merge-base --is-ancestor be8e82caa6859 HEAD`: YES (TLS
present)
- [Phase 3] `git merge-base --is-ancestor 19bdb70c77d3 HEAD`: NO
(hiding/dynamic-key commit not in 6.18)
- [Phase 3] `git grep` on `v6.18:drivers/nvme/host/tcp.c`: reclassify at
line 1801, TLS at 1885 — buggy order confirmed
- [Phase 3] `git blame v6.18 -- drivers/nvme/host/tcp.c`: reclassify =
`841aee4d75f1`, TLS = `be8e82caa6859`
- [Phase 3] `git merge-base --is-ancestor 0ba6912f7e97 HEAD`: NO; not
required for this hunk
- [Phase 4] `b4 dig -c 447a7a3026…`: patch 2/2, msgid
`20260914074853.3990397-3-…`
- [Phase 4] `b4 dig -a`: v1 series only (cover + revert + this patch)
- [Phase 4] `b4 dig -w`: linux-nvme +
Busch/Axboe/Hellwig/Grimberg/Dumazet/Shroff
- [Phase 4] mbox: Nilay Reviewed-by; Keith applied to nvme-7.3; no NAK;
no Cc:stable
- [Phase 4] lore.kernel.org WebFetch: 403; used b4 mbox instead
- [Phase 4] stable list search: no prior discussion found
- [Phase 5] Callers: `nvme_tcp_alloc_admin_queue`,
`__nvme_tcp_alloc_io_queues` (controller setup)
- [Phase 5] `sock_allow_reclassification()`: `!sock_owned_by_user &&
!spin_is_locked(slock)`
- [Phase 6] Apply: same surrounding symbols in v6.18; expected clean
- [Phase 8] Failure mode: lockdep WARNING on TLS connect, not a verified
production deadlock
- UNVERIFIED: whether any distro 6.18 debug kernel has a live user
report of this splat (commit cites blktests, not a distro bugzilla)
The patch is small, the buggy ordering is in this 6.18 tree, and it does
not need the later revert series to be useful.
**YES**
drivers/nvme/host/tcp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 1f26e35d7a99e..9f15feff66132 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -1859,7 +1859,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
}
sk_net_refcnt_upgrade(queue->sock->sk);
- nvme_tcp_reclassify_socket(queue->sock);
/* Single syn retry */
tcp_sock_set_syncnt(queue->sock->sk, 1);
@@ -1948,6 +1947,8 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
goto err_init_connect;
}
+ nvme_tcp_reclassify_socket(queue->sock);
+
ret = nvme_tcp_init_connection(queue);
if (ret)
goto err_init_connect;
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH AUTOSEL 6.18] nvme-tcp: delay nvme_tcp_reclassify_socket()
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18] nvme-tcp: delay nvme_tcp_reclassify_socket() Sasha Levin
@ 2026-10-19 14:50 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-19 14:50 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-nvme
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] nvme-tcp: unsafe live lock reinit in nvme_tcp_reclassify_socket()
--
commit 168d45f90ed1a7b47f62fbd5085e5e0bbb05d5a8
Author: Eric Dumazet <edumazet@google.com>
nvme-tcp: delay nvme_tcp_reclassify_socket()
This commit delays nvme_tcp_reclassify_socket() in nvme_tcp_alloc_queue()
until after kernel_connect() and the optional nvme_tcp_start_tls() handshake
complete, in order to avoid pulling tlshd lock dependencies into the
nvme-tcp socket lockdep class.
Link: https://lore.kernel.org/lkml/CANn89i+wnTLC==UnXCpjsS4YxvEfhe5oK0N7fttbqr1zKyqdug@mail.gmail.com/
> diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> index beef2b576102a..da148092677fe 100644
> --- a/drivers/nvme/host/tcp.c
> +++ b/drivers/nvme/host/tcp.c
> @@ -1859,7 +1859,6 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
> }
>
> sk_net_refcnt_upgrade(queue->sock->sk);
> - nvme_tcp_reclassify_socket(queue->sock);
>
> /* Single syn retry */
> tcp_sock_set_syncnt(queue->sock->sk, 1);
> @@ -1948,6 +1947,8 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid,
> goto err_init_connect;
> }
>
> + nvme_tcp_reclassify_socket(queue->sock);
> +
[Severity: High]
Is it safe to call nvme_tcp_reclassify_socket() here after kernel_connect()
and nvme_tcp_start_tls() have already run?
When CONFIG_DEBUG_LOCK_ALLOC is enabled, nvme_tcp_reclassify_socket() checks
sock_allow_reclassification() and calls sock_lock_init_class_and_name() in
process context with bottom halves enabled and no lock held:
drivers/nvme/host/tcp.c:nvme_tcp_reclassify_socket() {
struct sock *sk = sock->sk;
if (WARN_ON_ONCE(!sock_allow_reclassification(sk)))
return;
switch (sk->sk_family) {
case AF_INET:
sock_lock_init_class_and_name(sk, "slock-AF_INET-NVME",
&nvme_tcp_slock_key[0],
"sk_lock-AF_INET-NVME",
&nvme_tcp_sk_key[0]);
break;
...
}
And sock_lock_init_class_and_name() reinitializes sk->sk_lock in-place:
include/net/sock.h:sock_lock_init_class_and_name()
sk_owner_set(sk, THIS_MODULE);
sk->sk_lock.owned = 0;
init_waitqueue_head(&sk->sk_lock.wq);
spin_lock_init(&(sk)->sk_lock.slock);
debug_check_no_locks_freed((void *)&(sk)->sk_lock,
sizeof((sk)->sk_lock));
lockdep_set_class_and_name(&(sk)->sk_lock.slock,
(skey), (sname));
lockdep_init_map(&(sk)->sk_lock.dep_map, (name), (key), 0);
At this point in nvme_tcp_alloc_queue() (reached from
nvme_tcp_alloc_admin_queue() or __nvme_tcp_alloc_io_queues()),
kernel_connect() has already transitioned queue->sock->sk to TCP_ESTABLISHED,
hashed it into inet_ehash, and armed TCP timers. In addition, when TLS is
used, handshake_nl_done_doit() -> handshake_complete() ->
nvme_tcp_tls_done() completes queue->tls_complete before userspace tlshd
closes its open file descriptor to queue->sock->file.
If incoming peer packets (such as TCP ACKs, window updates, TLS 1.3
post-handshake NewSessionTicket messages, or FIN/RST) trigger softirq TCP RX
(tcp_v4_rcv() / tcp_v6_rcv()), or if TCP timers (tcp_write_timer() /
tcp_delack_timer()) or tlshd activity coincide with
nvme_tcp_reclassify_socket():
1. Can softirq or tlshd acquire sk->sk_lock.slock or sk->sk_lock right after
sock_allow_reclassification(sk) succeeds, racing with spin_lock_init(),
init_waitqueue_head(), sk->sk_lock.owned = 0,
debug_check_no_locks_freed(), lockdep_set_class_and_name(), and
lockdep_init_map() to corrupt the spinlock, waitqueue, and lockdep state?
2. If softirq or tlshd already holds sk->sk_lock.slock or sk->sk_lock.owned
when sock_allow_reclassification(sk) is checked, won't this trigger
WARN_ON_ONCE(!sock_allow_reclassification(sk)) and abort reclassification,
leaving the socket in the default lockdep class?
> ret = nvme_tcp_init_connection(queue);
> if (ret)
> goto err_init_connect;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-249-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH AUTOSEL 6.18-6.12] nvme: handle positive error codes in nuse_show()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (7 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18] nvme-tcp: delay nvme_tcp_reclassify_socket() Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-6.6] nvme: don't reference NS after unlocking in nvme_ns_head_ctrl_ioctl() Sasha Levin
` (2 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: John Garry, Christoph Hellwig, Keith Busch, Sasha Levin, axboe,
sagi, linux-nvme, linux-kernel
From: John Garry <john.g.garry@oracle.com>
[ Upstream commit dd516cd7624648c71d338d99368587f10ecc9f0b ]
Function __nvme_submit_sync_cmd() returns a positive error code for NVMe
errors. Otherwise, we get 0 for success or a negative error code for a
kernel error.
In nuse_show() -> ns_{head}_update_nuse() -> nvme_identify_ns() ->
nvme_submit_sync_cmd() -> __nvme_submit_sync_cmd(), we then may get a
positive error code returned.
Function nuse_show() - being a device attr handler - should return the
number of bytes written to the buffer or a negative error code.
Convert any positive NVMe error code to -EIO.
Signed-off-by: John Garry <john.g.garry@oracle.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have the mailing-list thread and the sysfs/NVMe return-code path. Next
I’ll confirm how a positive NVMe status is treated as a byte count, then
give the backport verdict for this tree.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[nvme] [handle] convert positive NVMe status codes from
nuse_show() into a proper sysfs errno`
- Subsystem: `nvme` (host sysfs)
- Verb: `handle` (bug-fix language: incorrect error handling)
- Intent: sysfs `nuse` show must not return a positive NVMe status as a
“bytes written” count
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Fixes:` — absent (expected for this review pipeline)
- `Reported-by:` — none
- `Tested-by:` — none
- `Reviewed-by: Christoph Hellwig <hch@lst.de>` (core NVMe reviewer)
- `Acked-by:` — none
- `Link:` — none in the commit; lore found separately
- `Cc: stable@vger.kernel.org` — absent (not a negative signal)
- `Signed-off-by: John Garry <john.g.garry@oracle.com>` (author)
- `Signed-off-by: Keith Busch <kbusch@kernel.org>` (NVMe maintainer;
applied to nvme-7.3)
- Ignore pipeline `Signed-off-by: Sasha Levin` if present — not in this
commit
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `__nvme_submit_sync_cmd()` returns 0, a negative Linux errno,
or a **positive NVMe status**. That value is propagated `nuse_show() →
ns_{head}_update_nuse() → nvme_identify_ns() →
nvme_submit_sync_cmd()`.
- **Symptom:** `nuse_show()` is a device attribute handler; it must
return a byte count or a negative errno. A positive NVMe status is
neither.
- **Fix:** map `ret > 0` to `-EIO`; keep `ret < 0` as-is.
- **Version info in message:** none. Feature itself dates to v6.8
(`a1a825ab6a603` “nvme: add csi, ms and nuse to sysfs”).
- **Root cause:** treating any non-zero `ret` as a sysfs error, which is
wrong for positive NVMe codes (they are not negative, so they were
returned as a success length).
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. Explicit error-handling fix. Same class as “add
missing check” / “handle X properly.”
---
## PHASE 2: DIFF ANALYSIS - LINE BY LINE
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/nvme/host/sysfs.c` — **+3 / −1**
- Function: `nuse_show()` only
- Scope: **single-file surgical fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `if (ret) return ret;` — negative errno returned
correctly; **positive NVMe status returned as the sysfs byte count**,
and `sysfs_emit()` is skipped so the buffer is never filled with the
NUSE value.
- **After:** `ret < 0` → pass through; `ret > 0` → `-EIO`; `ret == 0` →
`sysfs_emit()` of `head->nuse`.
- Path affected: **error path** of a sysfs read that issues Identify
Namespace (rate-limited).
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** logic/correctness of sysfs error conversion; can also
trip a kernel `WARN`.
- **Mechanism:** NVMe completion status is stored as
`le16_to_cpu(cqe->status) >> 1` (`nvme_try_complete_req()`).
`nvme_execute_rq()` returns that value when non-zero. Status often
includes `NVME_STATUS_DNR` (**0x4000 = 16384**).
- Sysfs (`sysfs_kf_seq_show` in `fs/sysfs/file.c`):
- `count < 0` → error
- `count >= PAGE_SIZE` → **`WARN(1, "OOB write or bad count ...")`**
then clamp
- otherwise `seq_commit(sf, count)` of a buffer that was `memset` to 0
but never written by `nuse_show()`
- On 4K pages (typical x86_64), DNR-tagged Identify failures (`status |
0x4000 >= 16384 > 4096`) **fire a kernel WARN**. Smaller statuses
become a short successful read of zeros instead of `-EIO`.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: matches `__nvme_submit_sync_cmd()` contract and the
existing `nvme_hwmon` pattern (`if (ret > 0) return -EIO`).
- Minimal; no unrelated edits; no API/struct changes.
- Regression risk **very low**: only the misclassified success path
becomes `-EIO`, which is the intended ABI.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Buggy `if (ret) return ret;` introduced in **`a1a825ab6a603`** (Daniel
Wagner, 2023-12-18), “nvme: add csi, ms and nuse to sysfs”.
- First appeared: **v6.8-rc1** (`git describe --contains` →
`v6.8-rc1~121^2~25^2~4`).
- Present for a long time in this 7.3 lineage.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Buggy commit identified by blame:
`a1a825ab6a603`. That commit **is** an ancestor of this tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- `8d0d2447394b1` — double-free of `nvme_id_ns` in `ns_update_nuse()`
(already in tree; different bug).
- `88c918d1ee2c88` — `nvme_disk_is_ns_head()` use in `nuse_show`.
- Standalone v1 patch; not “patch X/Y”.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: John Garry is an active NVMe/block contributor (cdev lifetime,
multipath, sysfs). Nearby commits are unrelated lifetime/locking work,
not prerequisites of this hunk.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. Touches only `nuse_show()` control flow. `nuse_show`,
`ns_update_nuse`, `ns_head_update_nuse`, and `nvme_identify_ns` all
exist here. **Applies standalone.**
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c dd516cd762464` → https://patch.msgid.link/20260630102717.26
71475-1-john.g.garry@oracle.com
- **v1 only** (no v2/v3). Applied as submitted.
- Author note: could print `nvme_get_error_status_str()`; rejected as a
bad idea (keeps the fix small).
- Christoph: “Looks good:” + `Reviewed-by`
- Keith Busch: “Thanks, applied to nvme-7.3.”
- No NAKs, no stable nomination, no risk flags.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc **hch@lst.de, kbusch@kernel.org,
sagi@grimberg.me, axboe@fb.com, linux-nvme@lists.infradead.org**. Right
maintainers; Hellwig reviewed; Busch applied.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No `Reported-by`, syzbot, or bugzilla. Found by code inspection
of the Identify → sysfs return path. Still a real, reachable bug (sysfs
read issues Identify NS).
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone single patch. Same conversion already used in
`drivers/nvme/host/hwmon.c`.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: Direct lore stable search was blocked (bot/403). Thread mbox has
**no** `Cc: stable` and no stable-list discussion. **UNVERIFIED**
whether anyone later asked for it on stable@ — does not change the
technical verdict.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `nuse_show` (modified). Callers of the bad return:
`ns_head_update_nuse`, `ns_update_nuse` → `nvme_identify_ns` →
`nvme_submit_sync_cmd` → `__nvme_submit_sync_cmd` → `nvme_execute_rq`.
**Step 5.2: TRACE CALLERS**
Record: `nuse_show` is `DEVICE_ATTR_RO(nuse)` (`0444`) on the NVMe
namespace disk. Reachable from userspace `read()` of
`/sys/block/nvmeXnY/nuse`. Documented in
`Documentation/ABI/stable/sysfs-nvme`; used so libnvme need not issue
Identify itself. **Not** IRQ/hot-path; process context, admin command.
**Step 5.3: TRACE CALLEES**
Record: Identify Namespace (admin), SRCU in the multipath helper, rate
limit 5s. On Identify failure, `nvme_identify_ns()` kfree’s the id
buffer and returns the status — no leak there.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: userspace `read` → sysfs/kernfs → `nuse_show` → Identify NS.
**Unprivileged users can trigger it** (world-readable). Failure of
Identify is uncommon on a healthy live NS, but expected during reset,
path loss, unmap, or device errors — exactly when libnvme/nvme-cli may
still read `nuse`.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same `if (ret > 0) return -EIO` in `nvme_get_temp_thresh` /
`nvme_set_temp_thresh` (`hwmon.c`). Other `nuse_show`-style sysfs
readers in this file mostly format cached fields and do not submit
commands.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- `git describe HEAD` → **v7.3-rc4-27-gf0100363d8c37**
- `make kernelversion` → **7.3.0-rc4**
- `nuse_show` exists in `drivers/nvme/host/sysfs.c`
- Bug introduced **v6.8** (`a1a825ab6a603`); **is** ancestor of HEAD
- Candidate **`dd516cd762464` is already an ancestor of HEAD** (evaluate
on merit, not “already selected”)
- Current file already contains the `ret < 0` / `ret > 0` logic
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** on this tree (hunk matches current `nuse_show`).
No conflicting refactor of this function beyond the fix itself.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: Double-free fix `8d0d2447394b1` is present and orthogonal. No
alternate conversion of positive NVMe status in `nuse_show` besides this
commit.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **NVMe host sysfs** (`drivers/nvme/host/`). Criticality:
**IMPORTANT** (common storage stack; not mm/VFS core). Affects NVMe
users who read namespace sysfs (management tools).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively developed (many recent sysfs/diag/timeout patches).
`nuse` itself is mature (since 6.8), not brand-new 7.3-only code.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Driver-specific** — NVMe namespace sysfs. Anyone with
`CONFIG_BLK_DEV_NVME` (default on most distros) and a namespace. Tools:
libnvme / nvme-cli discovery.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: `read` of `/sys/block/nvmeXnY/nuse` while Identify NS returns
a **non-zero NVMe status** (and the 5s ratelimit allows a command).
- Uncommon on a healthy device; realistic on reset, interconnect errors,
or a controller that fails Identify.
- Unprivileged readable (`DEVICE_ATTR_RO` = 0444).
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- DNR (or any status ≥ `PAGE_SIZE`): **kernel WARN** in
`sysfs_kf_seq_show` (“OOB write or bad count”) — **HIGH** (WARNING;
panic if `panic_on_warn`).
- Smaller status: successful short read of zeros instead of an error —
**MEDIUM** (wrong ABI; tools may treat NUSE as 0 or fail to parse).
- Not UAF, not filesystem corruption, not a hard deadlock.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** medium–high — stops a real WARN and a bogus sysfs success
on a widely deployed driver.
- **Risk:** very low — 3 lines, error path only, established `-EIO`
mapping.
- **Ratio:** favorable.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR backporting**
- Real bug: sysfs show returns a positive NVMe status as a byte count.
- Can `WARN` when status includes `NVME_STATUS_DNR` (0x4000) on 4K/16K
pages.
- Userspace can get a successful empty/short read instead of an error.
- 3-line, obviously correct; Hellwig reviewed; Busch applied.
- Same pattern already used in NVMe hwmon.
- Code and bug exist in this 7.3 tree (since v6.8).
- No new API/feature; no series dependencies.
**AGAINST backporting**
- No user/syzbot report; found by inspection.
- Identify NS rarely fails on a live healthy namespace.
- Not a UAF/panic/data-corruption class bug in the common success path.
**UNRESOLVED**
- Stable@ archives could not be fetched (403/bot). No evidence of a
prior rejection.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — reviewed by Hellwig, applied
by Busch, matches documented return contract.
2. Fixes a real bug that affects users? **PASS** — wrong sysfs return;
WARN on DNR Identify failures.
3. Important issue? **PASS** — WARN_ON-class (and wrong userspace ABI).
Severity HIGH for the WARN path, MEDIUM otherwise.
4. Small and contained? **PASS** — +3/−1, one function.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — `nuse_show` present; hunk
matches.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. Ordinary small
driver bugfix.
**Step 9.4: MAKE YOUR DECISION**
This tree is **7.3.0-rc4**. The `nuse` sysfs attribute exists here. A
positive NVMe Identify status must not be returned as a sysfs byte
count; that can WARN and confuse management tools. The patch is tiny,
already the established NVMe conversion, and has maintainer review.
Benefit outweighs risk.
### What the commit solves for this tree
`nuse_show()` refreshes Namespace Utilization via Identify NS. Command
errors come back as **positive** NVMe status. Returning that from a
`DEVICE_ATTR_RO` show handler makes kernfs treat it as a length: zeros
to userspace, or a **sysfs “bad count” WARN** when the status is ≥
`PAGE_SIZE` (typical with DNR). Mapping `ret > 0` to `-EIO` restores the
sysfs contract.
### Risk vs benefit
Very low risk, clear correctness win on a user-visible NVMe sysfs file.
---
## Verification
- **[Phase 1]** Parsed message: no Fixes/Reported-by/stable Cc;
Reviewed-by Hellwig; SOB Garry + Busch.
- **[Phase 2]** Diff: +3/−1 in `nuse_show()` only; `if (ret)` → `ret <
0` / `ret > 0` → `-EIO`.
- **[Phase 2]** `__nvme_submit_sync_cmd()` comment and
`nvme_execute_rq()`: positive value is NVMe status
(`nvme_req(rq)->status`).
- **[Phase 2]** `nvme_try_complete_req()`: `rq->status =
le16_to_cpu(status) >> 1`; `NVME_STATUS_DNR = 0x4000`.
- **[Phase 2]** `fs/sysfs/file.c` `sysfs_kf_seq_show`: `count >=
PAGE_SIZE` → `WARN(1, "OOB write or bad count")`.
- **[Phase 2]** Same conversion in `drivers/nvme/host/hwmon.c` lines
31–32 and 54–55.
- **[Phase 3]** Blame: buggy `if (ret)` from `a1a825ab6a603` (v6.8); fix
is `dd516cd762464`.
- **[Phase 3]** `git merge-base --is-ancestor a1a825ab6a603 HEAD` — yes;
`dd516cd762464` also ancestor.
- **[Phase 3]** Related: `8d0d2447394b1` double-free in `ns_update_nuse`
— different bug, already here.
- **[Phase 3]** Author log: standalone; not a multi-patch series.
- **[Phase 4]** `b4 dig -c dd516cd762464`: lore msgid
`20260630102717.2671475-1-john.g.garry@oracle.com`.
- **[Phase 4]** `b4 dig -a`: v1 only.
- **[Phase 4]** `b4 dig -w`: linux-nvme + Hellwig, Busch, Grimberg,
Axboe.
- **[Phase 4]** mbox: Hellwig “Looks good”; Busch “applied to nvme-7.3”;
no NAK.
- **[Phase 4]** WebFetch lore/stable: 403/bot — UNVERIFIED stable@
traffic.
- **[Phase 5]** semcode MCP not available; used grep. Callers: sysfs RO
attr `nuse`.
- **[Phase 5]** ABI: `/sys/block/nvmeXnY/nuse` since 6.8; Identify on
read; `DEVICE_ATTR_RO` → 0444.
- **[Phase 6]** `git describe HEAD`: v7.3-rc4-27-gf0100363d8c37;
kernelversion 7.3.0-rc4.
- **[Phase 6]** Current `nuse_show` already has the fix; buggy pattern
exists in history of this file.
- **[Phase 8]** Failure: WARN (HIGH) or bogus short sysfs read (MEDIUM);
trigger = Identify NS error on nuse read.
The fix is small, surgical, and meets the stable criteria for this tree.
**YES**
drivers/nvme/host/sysfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/sysfs.c b/drivers/nvme/host/sysfs.c
index 16c6fea4b2db6..12478e0297356 100644
--- a/drivers/nvme/host/sysfs.c
+++ b/drivers/nvme/host/sysfs.c
@@ -239,8 +239,10 @@ static ssize_t nuse_show(struct device *dev, struct device_attribute *attr,
ret = ns_head_update_nuse(head);
else
ret = ns_update_nuse(disk->private_data);
- if (ret)
+ if (ret < 0)
return ret;
+ else if (ret > 0)
+ return -EIO;
return sysfs_emit(buf, "%llu\n", head->nuse);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH AUTOSEL 6.18-6.6] nvme: don't reference NS after unlocking in nvme_ns_head_ctrl_ioctl()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (8 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.12] nvme: handle positive error codes in nuse_show() Sasha Levin
@ 2026-10-19 13:51 ` Sasha Levin
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-6.1] nvmet: passthru: fix OOB reads when parsing ns id descriptor list Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.10] nvme-fc: do not warn on controller removal race Sasha Levin
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:51 UTC (permalink / raw)
To: patches, stable
Cc: John Garry, Christoph Hellwig, Keith Busch, Sasha Levin, axboe,
sagi, linux-nvme, linux-kernel
From: John Garry <john.g.garry@oracle.com>
[ Upstream commit 77c57daf98a2ad95ec9ac1371caeff8939cc1f58 ]
In nvme_ns_head_ctrl_ioctl(), once we drop the SRCU read lock we should
not reference the NS to lookup the controller, so use the available
controller pointer directly.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
candidate is a one-line use-after-unlock fix in the NVMe multipath ioctl
path; the rest of this write-up is the forensic check of whether that
fix belongs here as a stable backport.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[nvme] [don't reference] Avoid using the namespace object after
dropping the SRCU lock in nvme_ns_head_ctrl_ioctl()`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Reviewed-by: Christoph Hellwig <hch@lst.de>` (NVMe co-maintainer)
- `Signed-off-by: John Garry <john.g.garry@oracle.com>` (author)
- `Signed-off-by: Keith Busch <kbusch@kernel.org>` (NVMe maintainer;
applied the patch)
- No `Fixes:` tag (expected for this review pipeline)
- No `Cc: stable@vger.kernel.org` (expected)
- No `Reported-by:`, `Tested-by:`, `Link:`, or syzbot tags
Later mailing-list review also produced `Reviewed-by: Hannes Reinecke
<hare@kernel.org>` (not copied into the committed message).
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** After `srcu_read_unlock(&head->srcu)`, the code still did
`nvme_ctrl_ioctl(ns->ctrl, ...)`.
- **Symptom/failure mode:** Use of `ns` after the SRCU critical section
ends. `nvme_find_path()` does not take an `ns` kref; SRCU is what
keeps `ns` alive. After unlock, `nvme_ns_remove()` can finish
`synchronize_srcu(&ns->head->srcu)` and `nvme_put_ns()` → `kfree(ns)`.
Loading `ns->ctrl` is then a use-after-free.
- **Version info:** None in the message. History shows the pattern dates
to `48145b62563a9` (v5.13-rc1 era, 2021).
- **Root cause:** The function already saved `struct nvme_ctrl *ctrl =
ns->ctrl` and used it for `nvme_put_ctrl(ctrl)`, but the ioctl call
still went through `ns->ctrl` after unlock.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. The subject is explicit. This is a real lifetime/UAF
fix, not style cleanup. The local `ctrl` variable existed specifically
to survive the unlock; the ioctl call was the leftover unsafe use.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/nvme/host/ioctl.c` — 1 insertion, 1 deletion
- Function: `nvme_ns_head_ctrl_ioctl()`
- Scope: single-file, one-line surgical fix under
`CONFIG_NVME_MULTIPATH`
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
Before:
```
struct nvme_ctrl *ctrl = ns->ctrl;
nvme_get_ctrl(ns->ctrl); /* still under SRCU — OK */
srcu_read_unlock(&head->srcu, srcu_idx);
ret = nvme_ctrl_ioctl(ns->ctrl, ...); /* ns used after unlock — BUG */
nvme_put_ctrl(ctrl);
```
After:
```
ret = nvme_ctrl_ioctl(ctrl, ...); /* uses the pointer saved under SRCU
*/
```
The `nvme_get_ctrl()` before unlock keeps the controller object alive;
only the namespace object was left unprotected.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** use-after-free / lifetime (memory safety), SRCU
protection dropped too early relative to `ns` use
- **Mechanism:** Path lookup is SRCU-only. Callers drop SRCU *on
purpose* before controller ioctls so a Delete Namespace passthrough
cannot deadlock on `synchronize_srcu` (comment at the
`nvme_ns_head_ioctl` call site). Concurrent `nvme_ns_remove()` can
then free `ns` while the old code still loaded `ns->ctrl`.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: use the already-captured, refcounted `ctrl`.
- Minimal: one identifier change; no API, locking, or control-flow
change.
- Regression risk: very low. `ctrl` is assigned under SRCU and pinned
with `nvme_get_ctrl()` before unlock.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Buggy `nvme_ctrl_ioctl(ns->ctrl, ...)` after unlock introduced in
`48145b62563a9` (`nvme: fix controller ioctl through ns_head`, Minwoo
Im, 2021-04-22).
- First appeared around **v5.13** (`tags/v5.13-rc1~19^2~1^2~6`).
- `struct nvme_ctrl *ctrl = ns->ctrl` was added in that same commit —
they saved `ctrl` for `nvme_put_ctrl()` but still passed `ns->ctrl`
into the ioctl.
- The SRCU unlock in this function has been there since `3e7d1a55165bd`
/ the 2021 rewrite. This is long-lived buggy code, not a recent
regression.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. The introducing commit is `48145b62563a9`, and
`git merge-base --is-ancestor 48145b62563a9 HEAD` succeeded, so the
buggy code is in this lineage.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Nearby John Garry commits (`627e8bb91a8fd` requeue/SRCU order,
`a11e0a4cb4189` / `9952f3882ecba` cdev lifetime) are **separate**
bugs. This diff does not depend on them.
- Not labeled `patch X/Y`. Standalone one-liner.
- Later `86f9536c2d8f4` only changes the sparse annotation `__releases`
→ `__releases_shared`; not a prerequisite.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: John Garry is a regular NVMe contributor (Oracle). Around this
date he also fixed related lifetime issues, but this patch is
independent. Maintainers Christoph Hellwig, Keith Busch, and Hannes
Reinecke all reviewed/acked it.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. The preimage already has `ctrl`, `nvme_get_ctrl`, and
`srcu_read_unlock`. The patch applies standalone.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 77c57daf98a2ad`: https://patch.msgid.link/20260706125402.61
5277-1-john.g.garry@oracle.com
- `b4 dig -a`: **v1 only**; committed version is that v1.
- Thread (saved via `b4 dig -m`):
- Christoph Hellwig: “Looks good:” + `Reviewed-by`
- Keith Busch: “Thanks, applied to nvme-7.3.”
- Hannes Reinecke: `Reviewed-by: Hannes Reinecke <hare@kernel.org>`
- No NAKs, no requests for a v2, no reviewer concern about the approach.
- No explicit `Cc: stable` in the thread (not a negative; this pipeline
expects that).
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc were `hch@lst.de`, `kbusch@kernel.org`,
`sagi@grimberg.me`, `axboe@fb.com`, `linux-nvme@lists.infradead.org`.
Subsystem maintainers were on the original mail and reviewed it.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No `Reported-by` / syzbot / bugzilla. Found by code inspection
(same author was auditing NVMe lifetime vs SRCU around the same time).
The race is still concrete: ioctl + path removal.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone `[PATCH]`, not a numbered series. Sibling lifetime
patches are independent.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org `/stable/` returned HTTP 403 from this
environment. **UNVERIFIED:** no confirmed stable@ discussion. That is
expected for a candidate that has not been auto-selected.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
semcode MCP tools are not available; callers/callees were traced with
grep and file reads.
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `nvme_ns_head_ctrl_ioctl()` only.
**Step 5.2: TRACE CALLERS**
Record:
- `nvme_ns_head_ioctl()` — `block_device_operations.ioctl` for the
multipath ns-head disk (`/dev/nvmeXnY`)
- `nvme_ns_head_chr_ioctl()` — `file_operations.unlocked_ioctl` for the
generic ns-head char device (`/dev/ngXnY`)
Both call it when `is_ctrl_ioctl(cmd)` is true: `NVME_IOCTL_ADMIN_CMD`,
`NVME_IOCTL_ADMIN64_CMD`, or SED/opal ioctls.
**Step 5.3: TRACE CALLEES**
Record: `nvme_get_ctrl` / `nvme_put_ctrl` (`get_device`/`put_device` on
`ctrl->device`), `srcu_read_unlock`, `nvme_ctrl_ioctl` (admin
passthrough / SED).
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace ioctl on a multipath NVMe ns-head device. Reachable
from userspace. Admin commands typically need privileges, but that does
not make a kernel UAF acceptable; privileged management tools and path
teardown are exactly the production workload.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Non-multipath `nvme_ioctl()` / `nvme_ns_chr_ioctl()` use
`ns->ctrl` without this SRCU drop; those `ns` objects are tied to the
open disk/cdev, so they are a different lifetime model. No second copy
of this exact post-unlock `ns->ctrl` load in `nvme_ns_head_ctrl_ioctl`.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Tree: **7.3.0-rc4** (`git describe HEAD` =
`v7.3-rc4-27-gf0100363d8c37`)
- `git merge-base --is-ancestor 77c57daf98a2ad HEAD` → yes (the fix is
already on this mainline snapshot)
- Preimage (`77c57daf98a2ad^`) still has `nvme_ctrl_ioctl(ns->ctrl,
...)` after unlock
- Introducing commit `48145b62563a9` is an ancestor; the bug has been in
this lineage since ~v5.13
- Current `ioctl.c` already uses `nvme_ctrl_ioctl(ctrl, ...)`
This is a mainline 7.3-rc checkout, not a 6.x.y stable branch. The
question is still whether the *fix* is stable-worthy for this codebase.
The touched function, the `ctrl` local, and the SRCU unlock are all
present. The bug was not introduced after this tree branched.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean apply on this code. The hunk is one argument change. Would
also apply cleanly to any tree that still has the 2021
`nvme_ns_head_ctrl_ioctl` shape (the `ctrl` local + post-unlock ioctl).
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This is the fix. No alternate older fix of the same `ns->ctrl`
load. Related commits fix different lifetime bugs.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `drivers/nvme/host` — NVMe host ioctl/multipath. **IMPORTANT**
(common storage stack; not core mm/vfs, but widely deployed). Gated on
`CONFIG_NVME_MULTIPATH` (Kconfig: “If unsure, say Y.”).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Active (io_uring passthrough, multipath, ioctl hardening). This
particular function body has been stable in shape since 2021.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: NVMe **multipath** users who issue controller ioctls on the ns-
head block or char device (`NVME_IOCTL_ADMIN_CMD` / `ADMIN64` / SED).
Datacenter NVMe-oF / dual-ported SSD setups.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- `srcu_read_lock` → `nvme_find_path` → controller ioctl →
**intentional** `srcu_read_unlock` (to avoid deadlock with namespace
delete) → old code loads `ns->ctrl`
- Concurrent `nvme_ns_remove()` (path loss, controller delete, admin
Delete NS)
- Timing-dependent. Not every ioctl hits it; when it hits, it is a real
UAF.
- Triggerable from userspace ioctl; typically privileged admin ioctls.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Use-after-free of `struct nvme_ns` → KASAN report / oops /
potential memory corruption. **Severity: HIGH** (UAF in a production
ioctl path).
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** High — closes a UAF that has existed since v5.13 in a
standard multipath ioctl path.
- **Risk:** Very low — one-line substitution of an already-saved,
refcounted pointer.
- **Ratio:** Strongly favors backporting.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real UAF: `ns` is SRCU-protected only; used after `srcu_read_unlock`
- `nvme_find_path()` does not take an `ns` kref (verified)
- `nvme_ns_remove()` does `synchronize_srcu(&ns->head->srcu)` then
`nvme_put_ns()` → `kfree(ns)` (verified)
- Callers drop SRCU early on purpose so namespace delete can proceed —
the window is intentional
- One-line, obviously correct, uses the pointer the original 2021 code
already saved
- Reviewed by Hellwig, Busch, Reinecke; applied to nvme-7.3
- Bug present since ~v5.13; no extra prerequisites
- Userspace-reachable ioctl on common NVMe multipath hardware
AGAINST:
- No crash log / syzbot in the commit message (inspection-found)
- No `Fixes:`/`Cc: stable` (expected here; not a reason to reject)
- Gated on `CONFIG_NVME_MULTIPATH`
- This 7.3-rc4 snapshot already contains the commit (it landed for 7.3);
that does **not** reduce technical merit for selecting it as a stable
candidate
UNRESOLVED:
- Could not fetch lore.kernel.org in the browser (403); thread was
recovered via `b4 dig -m`
- Could not search lore `/stable/` (403)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — one identifier; reviewed by
three NVMe maintainers
2. Fixes a real bug that affects users? **PASS** — UAF on ns-head
controller ioctl vs path removal
3. Important issue (crash/security/corruption/deadlock)? **PASS** — UAF
→ oops/corruption; HIGH
4. Small and contained? **PASS** — 1 line, one function
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — function and `ctrl` local
exist; hunk is the existing post-fix form
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. It is a normal
memory-safety bugfix.
**Step 9.4: MAKE YOUR DECISION**
This is a textbook stable candidate: a long-standing, small, obviously
correct UAF fix in NVMe multipath ioctl handling, with maintainer review
and no dependencies.
---
### What the commit solves (for stable users)
On `CONFIG_NVME_MULTIPATH`, controller ioctls on the namespace-head
device drop `head->srcu` before running the ioctl so a Delete Namespace
passthrough cannot deadlock. The namespace pointer is not refcounted
across that unlock. Passing `ns->ctrl` into `nvme_ctrl_ioctl()` after
unlock can dereference a freed `nvme_ns` if a path is removed
concurrently. Using the `ctrl` pointer saved under SRCU (already pinned
with `nvme_get_ctrl()`) closes that hole.
### Risk vs benefit
Benefit is a UAF/crash fix on a real ioctl path used with NVMe
multipath. Risk is essentially nil.
### Concerns
None for applicability on this tree’s NVMe ioctl code. Nearby Garry
lifetime patches are not required.
---
Verification:
- [Phase 1] Parsed commit `77c57daf98a2ad`: subject, Hellwig Reviewed-
by, Garry/Busch SOBs; no Fixes/Reported-by/Cc:stable
- [Phase 1] Body: only describes not using NS after SRCU unlock
- [Phase 2] `git show 77c57daf98a2ad`: 1 line in
`nvme_ns_head_ctrl_ioctl`, `ns->ctrl` → `ctrl` after unlock
- [Phase 2] Read current `drivers/nvme/host/ioctl.c`: fix present;
caller comment documents early SRCU drop to avoid delete-namespace
deadlock
- [Phase 3] `git blame -L 729,743 drivers/nvme/host/ioctl.c`: buggy
ioctl-through-ns-head from `48145b62563a9` (2021)
- [Phase 3] `git name-rev --tags 48145b62563a9`:
`tags/v5.13-rc1~19^2~1^2~6`
- [Phase 3] `git merge-base --is-ancestor 48145b62563a9 HEAD`: yes
- [Phase 3] Parent `627e8bb91a8fd` is a different requeue/SRCU bug; not
a dependency of this hunk
- [Phase 4] `b4 dig -c 77c57daf98a2ad`: lore at
patch.msgid.link/20260706125402.615277-1-john.g.garry@oracle.com
- [Phase 4] `b4 dig -a`: v1 only
- [Phase 4] `b4 dig -w`: linux-nvme + Hellwig/Busch/Grimberg/Axboe
- [Phase 4] `b4 dig -m /tmp/nvme-ns-unlock.mbox`: Hellwig R-b, Busch
applied to nvme-7.3, Reinecke R-b; no NAK
- [Phase 4] WebFetch lore.kernel.org: 403 — UNVERIFIED via HTTP; thread
recovered from mbox
- [Phase 5] grep: callers are `nvme_ns_head_ioctl` and
`nvme_ns_head_chr_ioctl`; wired from `multipath.c` fops
- [Phase 5] `nvme_find_path()` returns path under SRCU only; no
`nvme_get_ns()`
- [Phase 5] `nvme_free_ns()` kfree’s `ns`; `nvme_ns_remove()`
synchronize_srcu then `nvme_put_ns()`
- [Phase 6] `make kernelversion` / Makefile: 7.3.0-rc4; `git describe
HEAD`: v7.3-rc4-27-gf0100363d8c37
- [Phase 6] `git merge-base --is-ancestor 77c57daf98a2ad HEAD`: fix
already in this mainline snapshot
- [Phase 6] Preimage `git show
77c57daf98a2ad^:drivers/nvme/host/ioctl.c`: confirmed
`nvme_ctrl_ioctl(ns->ctrl, ...)` after unlock
- [Phase 7] `drivers/nvme/host/Kconfig`: `NVME_MULTIPATH` “If unsure,
say Y”
- [Phase 8] Failure mode: UAF of `struct nvme_ns` on controller ioctl vs
path remove — HIGH
- UNVERIFIED: stable@ mailing-list history (lore /stable/ HTTP 403)
- UNVERIFIED: no public crash reproducer attached to the patch (does not
override the verified lifetime hole)
The patch is obviously correct, fixes a real UAF, is one line, and
applies to this tree’s NVMe ioctl code.
**YES**
drivers/nvme/host/ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/ioctl.c b/drivers/nvme/host/ioctl.c
index fb8533127116a..b3e58de6f5cc5 100644
--- a/drivers/nvme/host/ioctl.c
+++ b/drivers/nvme/host/ioctl.c
@@ -727,7 +727,7 @@ static int nvme_ns_head_ctrl_ioctl(struct nvme_ns *ns, unsigned int cmd,
nvme_get_ctrl(ns->ctrl);
srcu_read_unlock(&head->srcu, srcu_idx);
- ret = nvme_ctrl_ioctl(ns->ctrl, cmd, argp, open_for_write);
+ ret = nvme_ctrl_ioctl(ctrl, cmd, argp, open_for_write);
nvme_put_ctrl(ctrl);
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH AUTOSEL 6.18-6.1] nvmet: passthru: fix OOB reads when parsing ns id descriptor list
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (9 preceding siblings ...)
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-6.6] nvme: don't reference NS after unlocking in nvme_ns_head_ctrl_ioctl() Sasha Levin
@ 2026-10-19 13:51 ` Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.10] nvme-fc: do not warn on controller removal race Sasha Levin
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:51 UTC (permalink / raw)
To: patches, stable
Cc: Hari Mishal, Keith Busch, Sasha Levin, sagi, kch, linux-nvme,
linux-kernel
From: Hari Mishal <harimishal1@gmail.com>
[ Upstream commit bc7f75eba50012ed447654d8ce169ebaba695193 ]
nvmet_passthru_override_id_descs() walks a namespace identification
descriptor list populated from the underlying passthru controller's
Identify response, which is device reported. The loop advanced pos by
device controlled amounts (sizeof(*cur) + nidl) without checking that
the next descriptor header actually fits inside the buffer, so a
malicious device could push pos to within a few bytes of the buffer end
and cause cur->nidl, cur->nidt or the reserved field to be read past the
allocation.
Additionally, when a CSI descriptor lands exactly at the last valid
header offset, cur + 1 points one byte past the end of the buffer.
The unconditional memcpy(&csi, cur + 1, NVME_NIDT_CSI_LEN) could read
that out-of-bounds byte and copy it back to the initiator via
nvmet_copy_to_sgl(), leaking adjacent heap memory.
Bounds check both the descriptor header and the CSI value before
dereferencing them.
Signed-off-by: Hari Mishal <harimishal1@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111** (`pending-6.12`). The buggy descriptor
parser is present here; the fix is not. This is a small, obviously
correct OOB-read/info-leak fix in NVMe target passthrough.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[nvmet/passthru] [fix] [OOB reads when parsing namespace ID
descriptor list]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- Signed-off-by: Hari Mishal \<harimishal1@gmail.com\> (author)
- Signed-off-by: Keith Busch \<kbusch@kernel.org\> (NVMe maintainer,
applied the patch)
- No Fixes:, Reported-by, Tested-by, Reviewed-by, Acked-by, Link:, or
Cc: stable tags (absence is expected for this review pipeline)
- Ignore any pipeline “Upstream commit” / Sasha Levin markers — none
were on the upstream commit itself
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `nvmet_passthru_override_id_descs()` walks a 4096-byte
Identify NS descriptor list filled by the backend passthrough
controller. The loop advances `pos` by `sizeof(*cur) + nidl` (`nidl`
is device-controlled) without checking that the next descriptor header
still fits in the buffer.
- **Symptom/failure:** OOB read of `cur->nidl` / `cur->nidt` / reserved
past the allocation. If a CSI descriptor sits at the last valid header
offset, `memcpy(&csi, cur + 1, NVME_NIDT_CSI_LEN)` reads one byte past
the buffer and that byte can be copied to the initiator via
`nvmet_copy_to_sgl()` — heap info leak.
- **Version info:** none in the message. Introducing code is from v5.19
(see Phase 3).
- **Root cause:** missing bounds checks on descriptor header and CSI
payload before dereference.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. The subject and body describe a real
OOB read / leak. The diff is only bounds checks.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/nvme/target/passthru.c` — **+9 / -0** (upstream: 9
insertions)
- Function: `nvmet_passthru_override_id_descs()`
- Scope: single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** loop `for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos +=
len)` sets `cur = data + pos`, then immediately reads `cur->nidl` /
`cur->nidt`. No check that `pos + sizeof(*cur)` is in range. CSI path
does `memcpy(&csi, cur + 1, NVME_NIDT_CSI_LEN)` with no payload-size
check.
- **After:**
1. If `pos + sizeof(*cur) > NVME_IDENTIFY_DATA_SIZE`, break (stop
before OOB header read).
2. On CSI, if `pos + sizeof(*cur) + NVME_NIDT_CSI_LEN >
NVME_IDENTIFY_DATA_SIZE`, break (stop before OOB CSI byte read).
- Path: Identify CNS=NS_DESC_LIST success path when `clear_ids` is set.
Fail-safe: truncated/malformed list → `csi_seen` stays false → buffer
zeroed, CSI not echoed.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** memory safety — out-of-bounds read + heap info leak
- **Mechanism:** device-controlled `nidl` can place `pos` so a 4-byte
`struct nvme_ns_id_desc` (`nidt`, `nidl`, `__le16 reserved`) does not
fit. `NVME_NIDT_CSI_LEN` is 1; with header at offset 4092, `cur + 1`
is byte 4096 (one past a 4096-byte `kzalloc`). That byte can be
returned to the initiator.
Verified sizes in this tree (`include/linux/nvme.h`):
- `struct nvme_ns_id_desc` = 4 bytes
- `NVME_NIDT_CSI_LEN` = 1
- `NVME_IDENTIFY_DATA_SIZE` = 4096
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: two `break`s before dereference.
- Minimal: no unrelated changes.
- Regression risk: low. Breaking on a truncated list skips CSI override;
`clear_ids` already zeros the rest of the list. Compliant devices put
CSI well inside 4096 bytes.
- Host-side parser later also checks `pos + sizeof(*cur) + cur->nidl`;
this passthru patch does not check non-CSI body size, but non-CSI
payloads are not read here — only used to advance `pos`. Next
iteration’s header check still stops OOB. Not a blocker.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Buggy function introduced by **`34ad61514c4c3`** (“nvmet: add a
clear_ids attribute for passthru targets”, Alan Adamson, 2022-06-27).
- First appeared: **v5.19-rc2** (`git describe`:
`v5.19-rc2-35-g34ad61514c4c3`).
- **In this tree:** yes (`git merge-base --is-ancestor 34ad61514c4c3
HEAD` → yes). Blame on the local file still points every line of this
function at that commit.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No Fixes: tag. Bug introduced by `34ad61514c4c3`, which is in
this 6.12 tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Local recent history of `passthru.c` includes `e38dad438fc08` (clear
EUID/NGUID/UUID on loop) — does not change this loop.
- Upstream: `bc7f75eba5001` is the latest change to this file on
`origin/master`; **no follow-up** after it.
- Standalone, not “patch X/Y”. Sibling host fix `3c568b35a0d30` (“nvme:
bound ns descriptor header and body to identify buffer”) is a parallel
pattern in `drivers/nvme/host/core.c`, **not a prerequisite** for this
passthru patch.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record:
- Author is not the subsystem maintainer. Related upstream work: host NS
descriptor bounds (`3c568b35a0d30`) and FDP clamp (`29261f8bb4166`).
- Applied by **Keith Busch** (NVMe maintainer). Weight is from
maintainer application + Christoph Hellwig review, not author
identity.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- Uses existing `NVME_IDENTIFY_DATA_SIZE`, `struct nvme_ns_id_desc`,
`NVME_NIDT_CSI`, `NVME_NIDT_CSI_LEN` — all in this tree.
- `git apply --check` of the upstream diff against this tree: **clean**.
- Can apply standalone. **No prerequisites.**
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c bc7f75eba5001` →
https://patch.msgid.link/20260717144353.19436-1-harimishal1@gmail.com
- `b4 dig -a`: **v1 only** (single revision). Keith split the long line
when applying (`if (pos + sizeof(*cur) + NVME_NIDT_CSI_LEN >`
wrapped).
- `b4 dig -m /tmp/nvmet-oob.mbox`: full thread saved.
- Christoph Hellwig (20 Jul 2026): **“Overly long line here. Otherwise
this looks sane.”** — no NAK.
- Keith Busch (10 Aug 2026): **“Thanks, applied to nvme-7.3 with fix to
overly long line.”**
- No reviewer `Cc: stable` in the thread.
- WebFetch to lore.kernel.org returned **403**; discussion recovered via
`b4 dig -m`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc Hari Mishal, Christoph Hellwig, Sagi
Grimberg, Chaitanya Kulkarni, linux-nvme@lists.infradead.org, linux-
kernel@vger.kernel.org. Hellwig reviewed; Busch applied. Appropriate
NVMe maintainers.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No Reported-by / syzbot / bugzilla. Found by code review (same
author class as the host parser bounds fix). Bug is still real from
reading the loop.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone v1. Related but independent: host `3c568b35a0d30`
(not in this tree). Not required to land this passthru fix.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** — lore.kernel.org/stable via WebFetch returned
403. No stable discussion in the captured linux-nvme/lkml thread.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: KEY FUNCTIONS IN THE DIFF**
Record: `nvmet_passthru_override_id_descs()` only.
**Step 5.2: TRACE CALLERS**
Record:
- Called from `nvmet_passthru_execute_cmd_work()` when Identify succeeds
and `cns == NVME_ID_CNS_NS_DESC_LIST`.
- That work is used because `nvmet_parse_passthru_admin_cmd()` sets
`req->execute = nvmet_passthru_execute_cmd` and `req->p.use_workqueue
= true` for CNS CTRL/NS/NS_DESC_LIST.
- Reachable from an NVMe-oF initiator Identify command on a passthrough
subsystem.
**Step 5.3: TRACE CALLEES**
Record: `kzalloc(4096)`, `nvmet_copy_from_sgl` (backend Identify data),
descriptor walk, `memset` + rebuild CSI, `nvmet_copy_to_sgl` (to
initiator), `kfree`. No new locks/allocators in the patch.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record:
- Userspace/admin enables `CONFIG_NVME_TARGET` +
`CONFIG_NVME_TARGET_PASSTHRU`, exports a backend NVMe controller.
- Function only runs if `ctrl->subsys->clear_ids` is set. Default **1
for loop** (`core.c`: “By default, set loop targets to clear IDS”);
other transports via configfs `clear_ids`.
- Trigger: initiator Identify NS descriptor list. Backend-controlled
bytes; initiator can receive leaked heap. Reachable from the fabrics
initiator, not from an unprivileged syscall on the target without an
NVMe-oF target setup.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same missing-bounds pattern in `nvme_identify_ns_descs()`
(`drivers/nvme/host/core.c`) in this tree. Upstream fixed it separately
(`3c568b35a0d30`). Confirms a systematic parser bug; this commit is the
target/passthru instance.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** Local `drivers/nvme/target/passthru.c` lines 53–64 are
the unbounded loop. `git describe HEAD` = `p-6.12-3-gc504cf0baa3d8`,
`make kernelversion` = **6.12.111**. Upstream fix `bc7f75eba5001` is
**not** an ancestor of HEAD. Bug since v5.19.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** Surrounding function matches upstream pre-fix
(aside from later mainline kmalloc_obj churn, which is after this fix
and not in 6.12). `git apply --check` succeeded. Difficulty: clean
apply.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No commit with this subject in this tree. Host-side sibling
`3c568b35a0d30` also not in HEAD. No alternate fix for this passthru
loop.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **NVMe target passthrough** (`drivers/nvme/target/passthru.c`).
**IMPORTANT** for NVMe-oF target/passthru users (storage appliances,
loop testing), not core mm/VFS. Not staging.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Active. This 6.12 tree already has many nvmet stable-style fixes
(auth OOB, TCP PDU OOB, rdma leaks). Passthru file itself is relatively
mature since 2022.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Config-specific / driver-specific:** `CONFIG_NVME_TARGET=y/m`
and `CONFIG_NVME_TARGET_PASSTHRU=y`, and `clear_ids` enabled (default on
loop; optional elsewhere). Not all 6.12 users, but real production NVMe-
oF passthru and loop setups.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Identify NS descriptor list on a passthru namespace with `clear_ids`.
- Needs a backend that returns a truncated/malformed list (malicious or
buggy NVMe device).
- Not a hot every-syscall path; realistic for passthru Identify.
- Unprivileged local user on the target: no, unless they are an
initiator to that target. Remote initiator + malicious backend: yes,
leak across the fabric.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- OOB read of header fields (KASAN / possible oops depending on slab).
- 1-byte heap leak to the initiator via Identify response.
- **Severity: HIGH** (memory safety + info leak). Not a universal panic,
but security-class.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for passthru+clear_ids users — stops OOB read and
initiator-visible leak. Same class as other nvmet bounds fixes already
in this tree.
- **Risk:** very low — 9 lines, two bounds checks, fail-closed.
- **Ratio:** strongly favors backport.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real OOB read and heap leak, documented and confirmed in this tree’s
code.
- Bug present since v5.19; fully present on 6.12.111.
- 9-line, obviously correct bounds checks; `git apply --check` clean.
- NVMe maintainer applied it; Christoph Hellwig: “looks sane.”
- No follow-up rework on `origin/master`.
- Fail-closed: malformed list just omits CSI, which matches `clear_ids`
sanitizing.
**AGAINST:**
- Passthru is `default n`; needs `clear_ids`.
- No syzbot/CVE/Fixes/Cc: stable.
- Trigger is a malicious/buggy backend device, not a generic local
syscall.
- Slightly weaker than the host parser (no `nidl` body check on skip).
**UNRESOLVED:**
- Stable list search (lore 403).
- No public reproducer beyond the described layout.
None of the against items outweigh a small, verified memory-safety fix
whose buggy code is in this tree.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — Hellwig + Busch; logic
verified against struct sizes.
2. Fixes a real bug that affects users? **PASS** — OOB read/leak on
Identify for passthru+clear_ids.
3. Important issue? **PASS** — HIGH: OOB read + heap leak to initiator.
4. Small and contained? **PASS** — 9 lines, one function.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — clean apply; code present; no
missing deps.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. Qualifies as a
normal security/memory-safety bug fix.
**Step 9.4: MAKE YOUR DECISION**
Backport to this 6.12 tree. The parser the commit fixes is here, the
patch applies cleanly, and the failure is an OOB read that can leak heap
to an NVMe-oF initiator.
---
## What the commit solves (for this tree)
`nvmet_passthru_override_id_descs()` copies a 4096-byte Identify NS
descriptor list from the backend, walks it with device-controlled
strides, and may reconstruct a CSI descriptor for the initiator when
`clear_ids` is set. Without header/CSI bounds checks, a crafted list can
read past the allocation and send that byte to the initiator. On 6.12
this is the original 2022 loop; two range checks close it.
## Stable rules / risk vs benefit
Meets stable rules: real, important, small, no new API, applies here.
Benefit is stopping a leak/OOB on a path this tree already ships. Risk
is two `break`s on malformed input.
## Concerns
None that block backport. Host-side sibling `3c568b35a0d30` is a
separate candidate, not a dependency. `CONFIG_NVME_TARGET_PASSTHRU` is
optional; that narrows impact, not correctness.
---
## Verification
- [Phase 1] Parsed subject/body/tags from `git show bc7f75eba5001`;
tags: two SOBs only.
- [Phase 2] Diff: +9 in `nvmet_passthru_override_id_descs()`; compared
to local lines 53–64.
- [Phase 2] Sizes: `include/linux/nvme.h` — desc 4 bytes, CSI len 1,
Identify size 4096.
- [Phase 3] `git blame -L 33,78 drivers/nvme/target/passthru.c` →
`34ad61514c4c3`.
- [Phase 3] `git describe 34ad61514c4c3` → v5.19-rc2-35; `merge-base
--is-ancestor 34ad61514c4c3 HEAD` → yes.
- [Phase 3] `merge-base --is-ancestor bc7f75eba5001 HEAD` → no (not in
this tree).
- [Phase 3] `git log origin/master --oneline
bc7f75eba5001..origin/master -- drivers/nvme/target/passthru.c` →
empty (no follow-up).
- [Phase 3] `git show bc7f75eba5001 | git apply --check` → clean.
- [Phase 4] `b4 dig -c/-a/-w/-m`: v1 only; lore URL; Hellwig “looks
sane”; Busch applied to nvme-7.3.
- [Phase 4] WebFetch lore/patch.msgid.link: 403. Discussion from mbox,
not WebFetch.
- [Phase 5] Callers: `nvmet_passthru_execute_cmd_work` ← Identify
CNS_NS_DESC_LIST; parse path sets workqueue.
- [Phase 5] `clear_ids` default 1 for loop in
`drivers/nvme/target/core.c:1457-1459`.
- [Phase 5] Host analog unbounded loop in this tree at
`nvme_identify_ns_descs()`; upstream `3c568b35a0d30` not in HEAD.
- [Phase 6] `git describe HEAD` = p-6.12-3-gc504cf0baa3d8; `make
kernelversion` = 6.12.111; branch `pending-6.12`.
- [Phase 6] Local function matches pre-fix; origin/master post-fix
matches this commit.
- [Phase 8] Failure: OOB read + 1-byte heap leak to initiator; HIGH.
- UNVERIFIED: lore.kernel.org/stable discussion (HTTP 403). Not used to
drive the verdict.
**YES**
drivers/nvme/target/passthru.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/nvme/target/passthru.c b/drivers/nvme/target/passthru.c
index 0c361b1e35665..6d7469c94610e 100644
--- a/drivers/nvme/target/passthru.c
+++ b/drivers/nvme/target/passthru.c
@@ -53,13 +53,22 @@ static u16 nvmet_passthru_override_id_descs(struct nvmet_req *req)
for (pos = 0; pos < NVME_IDENTIFY_DATA_SIZE; pos += len) {
struct nvme_ns_id_desc *cur = data + pos;
+ if (pos + sizeof(*cur) > NVME_IDENTIFY_DATA_SIZE)
+ break;
+
if (cur->nidl == 0)
break;
+
if (cur->nidt == NVME_NIDT_CSI) {
+ if (pos + sizeof(*cur) + NVME_NIDT_CSI_LEN >
+ NVME_IDENTIFY_DATA_SIZE)
+ break;
+
memcpy(&csi, cur + 1, NVME_NIDT_CSI_LEN);
csi_seen = true;
break;
}
+
len = sizeof(struct nvme_ns_id_desc) + cur->nidl;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH AUTOSEL 6.18-5.10] nvme-fc: do not warn on controller removal race
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (10 preceding siblings ...)
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-6.1] nvmet: passthru: fix OOB reads when parsing ns id descriptor list Sasha Levin
@ 2026-10-19 13:52 ` Sasha Levin
11 siblings, 0 replies; 15+ messages in thread
From: Sasha Levin @ 2026-10-19 13:52 UTC (permalink / raw)
To: patches, stable
Cc: Maurizio Lombardi, Daniel Wagner, Keith Busch, Sasha Levin,
justin.tee, nareshgottumukkala83, paul.ely, axboe, sagi,
linux-nvme, linux-kernel
From: Maurizio Lombardi <mlombard@redhat.com>
[ Upstream commit 62b70e6c87121ae5b79ca20c3a5b005590336ec5 ]
In nvme_fc_reconnect_or_delete(), the system attempts to remove
the controller when reconnect limits are reached by calling
nvme_delete_ctrl(). This call is currently wrapped in a WARN_ON().
If a user concurrently deletes the controller via sysfs, the
controller's state may have already transitioned out of the
CONNECTING state. When this happens, nvme_delete_ctrl() correctly
rejects the operation and returns -EBUSY. Because of the WARN_ON()
wrapper, this race condition triggers an unnecessary kernel warning
stack trace.
Remove the WARN_ON() macro around nvme_delete_ctrl();
this allows the system to safely ignore the -EBUSY return value
if the controller is already being deleted.
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have the local-tree, diff, history, and review evidence. Below is the
full nine-phase analysis for this 6.18.y checkout.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[nvme-fc] [do not warn] suppress a false-positive WARN_ON when
controller deletion races with reconnect exhaustion`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>` (author)
- `Reviewed-by: Daniel Wagner <dwagner@suse.de>`
- `Signed-off-by: Keith Busch <kbusch@kernel.org>` (NVMe maintainer)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Link:`, or `Cc: stable`
(absence is expected)
- Pipeline-only tags ignored: `[Upstream commit 62b70e6c8712...]`,
`Signed-off-by: Sasha Levin`
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `nvme_fc_reconnect_or_delete()` wraps `nvme_delete_ctrl()` in
`WARN_ON()`.
- **Symptom:** concurrent sysfs delete can move the controller out of
`CONNECTING`; `nvme_delete_ctrl()` then correctly returns `-EBUSY`,
and `WARN_ON()` dumps a kernel warning stack.
- **Root cause:** TOCTOU between the `NVME_CTRL_CONNECTING` check and
the delete call. `-EBUSY` is a valid result, not a programming error.
- **Version info:** none stated. The `WARN_ON` has been on this call
since 2017.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not disguised. It is an explicit false-positive `WARN_ON` fix
for a real race. Functional delete behavior is unchanged.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `drivers/nvme/host/fc.c` (`1 insertion, 1 deletion`)
- Function: `nvme_fc_reconnect_or_delete()`
- Scope: single-file, one-line surgical change
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** after reconnect limits / `dev_loss_tmo` expiry,
`WARN_ON(nvme_delete_ctrl(&ctrl->ctrl))` fires on any non-zero return.
- **After:** `nvme_delete_ctrl()` is called and `-EBUSY` is ignored.
- Path: reconnect-failure / teardown, not the successful connect path.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** race / incorrect assertion (`WARN_ON` on a legitimate
`-EBUSY`)
- **Mechanism:** `nvme_delete_ctrl()` fails `nvme_change_ctrl_state(...,
NVME_CTRL_DELETING)` or fails to queue `delete_work` if delete is
already in progress. The reconnect path already checked
`nvme_ctrl_state() == NVME_CTRL_CONNECTING` without a lock, so a
concurrent sysfs/`nvme_delete_ctrl()` can win. The warning is the bug;
deletion is already handled.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: other `nvme_delete_ctrl()` sites in the same file
(remoteport unregister, module unload) and RDMA/TCP already omit
`WARN_ON`.
- Minimal, no API/data-structure change.
- Regression risk is very low: the only behavior change is not warning
on an expected `-EBUSY`. A still-`CONNECTING` controller is still
deleted.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- `WARN_ON(nvme_delete_ctrl(...))` blamed to `c5017e85705bfe` (“nvme:
move controller deletion to common code”, Christoph Hellwig,
2017-10-29).
- That commit replaced `WARN_ON(__nvme_fc_del_ctrl(ctrl))` with
`WARN_ON(nvme_delete_ctrl(...))`. The common helper can return
`-EBUSY` on a concurrent delete; the old wrapper’s assumption no
longer holds.
- Present since ~v4.15-era NVMe-FC code; long-lived in this 6.18 series.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable. Introducer is `c5017e85705bfe`,
which is in this tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Recent `fc.c` work is unrelated (DMA unmap, double-free, tagset, rport
locking).
- Precedent in-tree: `6622f9acd29cd` “nvme-loop: do not warn for deleted
controllers during reset” (2021) — same class of false `WARN_ON`
during concurrent delete.
- Standalone; not `patch X/Y`.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Maurizio Lombardi (Red Hat) is a regular NVMe contributor.
Applied by Keith Busch (NVMe maintainer). Reviewed by Daniel Wagner
(SUSE, active NVMe-FC). Thread also has `Reviewed-by: Hannes Reinecke`.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None. Uses existing `nvme_delete_ctrl()`. Applies standalone.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 62b70e6c87121ae5b79ca20c3a5b005590336ec5` →
https://patch.msgid.link/20260908143434.105513-1-mlombard@redhat.com
- `b4 dig -a`: single v1, no later revisions
- Full thread saved via `b4 dig -m`
- Daniel Wagner: “Yes, this makes sense.” + Reviewed-by
- Hannes Reinecke: Reviewed-by
- No NAKs, no risk flags, no `Cc: stable` in review (not a negative)
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: Recipients included `kbusch@kernel.org`, `dwagner@suse.de`,
`linux-nvme@lists.infradead.org`. Right maintainers and list. Keith
Busch signed off.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No `Reported-by` / bugzilla / syzbot. Description is a concrete
sysfs-vs-reconnect race, not theoretical cleanup.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone one-patch series. Sibling transports already call
`nvme_delete_ctrl()` without `WARN_ON`.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org WebFetch returned 403. The captured NVMe thread
has no stable discussion. **UNVERIFIED:** whether `stable@` discussed
this commit.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `nvme_fc_reconnect_or_delete()`; callee `nvme_delete_ctrl()`.
**Step 5.2: TRACE CALLERS**
Record:
- `nvme_fc_reset_ctrl_work()` → `nvme_fc_reconnect_or_delete(ctrl,
-ENOTCONN)` when the remoteport is not `ONLINE`
- `nvme_fc_connect_ctrl_work()` → `nvme_fc_reconnect_or_delete(ctrl,
ret)` when `nvme_fc_create_association()` fails
Both run on `nvme_wq` after link loss / failed reconnect — a normal SAN
path.
**Step 5.3: TRACE CALLEES**
Record: `nvme_delete_ctrl()` does `nvme_change_ctrl_state(ctrl,
NVME_CTRL_DELETING)` then `queue_work(nvme_delete_wq,
&ctrl->delete_work)`, returning `-EBUSY` if either fails. Sysfs
`delete_controller` uses `nvme_delete_ctrl_sync()` (same `DELETING`
transition).
**Step 5.4: FOLLOW THE CALL CHAIN**
Record:
```
fabric reconnect work / reset work
→ nvme_fc_reconnect_or_delete()
[state == CONNECTING?]
→ nvme_delete_ctrl() // races with:
sysfs delete_controller (CAP_SYS_ADMIN, S_IWUSR)
nvme_fc_unregister_remoteport()
nvme_fc_delete_controllers() (module unload)
```
Reachable in production NVMe-FC setups. Sysfs delete needs privilege;
reconnect exhaustion does not.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: At `v6.18.57`, `WARN_ON(nvme_delete_ctrl` exists only in
`drivers/nvme/host/fc.c`. RDMA (`nvme_rdma_reconnect_or_remove`), TCP,
and the other two FC call sites already ignore the return. This site is
the leftover.
semcode MCP: not available; used `git grep` / `rg`.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Local tree: `v6.18.57` (`VERSION=6`, `PATCHLEVEL=18`, `SUBLEVEL=57`;
`git describe HEAD` = `v6.18.57-155-gcdf239ca885b2`)
- At `v6.18.57`, `drivers/nvme/host/fc.c:3337` is
`WARN_ON(nvme_delete_ctrl(&ctrl->ctrl));`
- Upstream fix `62b70e6c8712` is **not** an ancestor of `v6.18.57`
- Bug introduced 2017; present in this series
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Clean apply. Context around the line at `v6.18.57` matches the
patch. `git apply --check` against the `v6.18.57` file succeeded.
Difficulty: clean apply.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No other in-tree commit removes this FC `WARN_ON`. The loop
analog (`6622f9acd29cd`) is already present and does not replace this
fix.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: NVMe over Fabrics FC host (`drivers/nvme/host/fc.c`).
**IMPORTANT** (enterprise SAN storage), not core mm/VFS, not staging.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (recent FC locking, tagset, DMA fixes). This
line itself has been stable since 2017.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Config-specific / driver-specific: `CONFIG_NVME_FC` users.
Typical audience is enterprise FC-SAN, i.e. LTS/stable deployments.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: reconnect attempts exhausted **and** a concurrent delete
(sysfs, remoteport teardown, or module unload)
- Not every I/O; realistic during link flaps / admin teardown
- Unprivileged users cannot write `delete_controller`; they can
contribute to reconnect failure. Privilege needed for the sysfs side
of the described race.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- Default: `WARN_ON` stack dump (no data loss; delete still happens on
the other path)
- With `panic_on_warn`: kernel panic during controller teardown
- Severity: **MEDIUM** normally, **HIGH/CRITICAL** if `panic_on_warn` is
enabled (CI, hardened, some Android/enterprise)
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** medium — stops a user-visible warning and a possible
`panic_on_warn` crash on a path enterprise NVMe-FC users actually hit
- **Risk:** very low — one line, no locking/API change, matches other
transports
- **Ratio:** benefit exceeds risk
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real race; `-EBUSY` is correct, `WARN_ON` is not
- `WARN_ON` is a listed strong-yes class (and can panic)
- One-line, obviously correct, reviewed by Wagner/Reinecke, applied by
Busch
- Buggy line exists in `v6.18.57`; applies cleanly; no prerequisites
- Same file’s other delete sites and RDMA/TCP already omit the warning
- Precedent: nvme-loop “do not warn for deleted controllers”
**AGAINST:**
- Not data corruption / UAF / deadlock
- No public user report or syzbot
- Default impact is a warning, not a functional hang
- NVMe-FC is not universal
**UNRESOLVED:**
- Whether `stable@` discussed it (lore search 403)
- How often the race is hit in the field (no `Reported-by`)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — one-line, two NVMe
reviewers, maintainer SOB
2. Fixes a real bug that affects users? **PASS** — false `WARN_ON` on a
reachable race
3. Important issue? **PASS** — `WARN_ON` / possible `panic_on_warn`
panic; not silent corruption, but in the documented yes class
4. Small and contained? **PASS** — 1 line, 1 file
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — clean on `v6.18.57`
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: None (not a device ID / quirk / DT / build / docs exception).
Qualifies as a normal small bugfix.
**Step 9.4: DECISION**
This tree is 6.18.y. The `WARN_ON` is still in `v6.18.57`. The patch is
a one-line removal of a false assertion on a real reconnect-vs-delete
race. It does not change successful-delete behavior, matches every other
NVMe transport, and is the kind of warning-to-panic issue stable should
take.
---
## What the commit solves, and why it matters here
On 6.18.y NVMe-FC, when reconnects are exhausted,
`nvme_fc_reconnect_or_delete()` tries to delete the controller. If
userspace (`delete_controller`) or another teardown path already moved
the controller to `DELETING`, `nvme_delete_ctrl()` returns `-EBUSY` by
design. Wrapping that in `WARN_ON()` turns a handled race into a kernel
warning (or a panic if `panic_on_warn` is set). Removing the wrapper is
the same policy RDMA, TCP, and the other FC delete sites already use.
Risk is negligible; benefit is avoiding a production warning/panic on
enterprise FC fabrics that 6.18.y actually ships.
---
## Verification
- [Phase 1] Parsed tags: author SOB Maurizio Lombardi; Reviewed-by
Daniel Wagner; SOB Keith Busch; no Fixes/Reported-by/Cc:stable
- [Phase 2] Diff: 1 line in `nvme_fc_reconnect_or_delete()`;
`WARN_ON(nvme_delete_ctrl())` → `nvme_delete_ctrl()`
- [Phase 2] `nvme_delete_ctrl()` at `drivers/nvme/host/core.c:257`
returns `-EBUSY` if state change or `queue_work` fails
- [Phase 2] State machine: `CONNECTING` → `DELETING` or `LIVE` only;
concurrent delete explains `-EBUSY`
- [Phase 3] `git blame` at parent: line from `c5017e85705bfe` (2017);
originally `WARN_ON(__nvme_fc_del_ctrl)`
- [Phase 3] `git log -S 'WARN_ON(nvme_delete_ctrl'` on `fc.c`:
introducer + this removal
- [Phase 3] Related: `6622f9acd29cd` nvme-loop “do not warn…” is already
an ancestor
- [Phase 3] No later `fc.c` commits after the candidate rewrite this
line
- [Phase 4] `b4 dig -c 62b70e6c8712`:
https://patch.msgid.link/20260908143434.105513-1-mlombard@redhat.com
- [Phase 4] `b4 dig -a`: v1 only
- [Phase 4] `b4 dig -w`: kbusch, dwagner, linux-nvme CC’d
- [Phase 4] `b4 dig -m`: Wagner + Reinecke Reviewed-by; no NAK
- [Phase 4] lore WebFetch 403 — used b4 mbox instead
- [Phase 4] UNVERIFIED: stable@ mailing list history (403)
- [Phase 5] Callers verified: `nvme_fc_reset_ctrl_work`,
`nvme_fc_connect_ctrl_work`
- [Phase 5] Concurrent deleters: `nvme_sysfs_delete` →
`nvme_delete_ctrl_sync`; `nvme_fc_unregister_remoteport`;
`nvme_fc_delete_controllers`
- [Phase 5] `git grep` at v6.18.57: only FC reconnect path uses
`WARN_ON(nvme_delete_ctrl`
- [Phase 6] `git describe HEAD` / `make kernelversion`: 6.18.57
- [Phase 6] `git show v6.18.57:drivers/nvme/host/fc.c` line 3337:
`WARN_ON` present
- [Phase 6] `git merge-base --is-ancestor 62b70e6c8712 v6.18.57` / HEAD:
not in the 6.18.57 base
- [Phase 6] Apply check of the patch onto the v6.18.57 file: success
- [Phase 7] `CONFIG_NVME_FC`; MAINTAINERS: Keith Busch + NVMe FC
transport
- [Phase 8] Failure mode: WARN_ON stack / panic_on_warn; severity MEDIUM
(HIGH if panic_on_warn)
- UNVERIFIED: field frequency (no Reported-by)
The fix is small, surgical, present as a real bug in this 6.18.y tree,
and meets the stable criteria.
**YES**
drivers/nvme/host/fc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index fc7aa3fc6c2b5..186d1beecf32b 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3334,7 +3334,7 @@ nvme_fc_reconnect_or_delete(struct nvme_fc_ctrl *ctrl, int status)
ctrl->cnum, min_t(int, portptr->dev_loss_tmo,
(ctrl->ctrl.opts->max_reconnects *
ctrl->ctrl.opts->reconnect_delay)));
- WARN_ON(nvme_delete_ctrl(&ctrl->ctrl));
+ nvme_delete_ctrl(&ctrl->ctrl);
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 15+ messages in thread