* [PATCH AUTOSEL 6.18-5.10] s390/dasd: Snapshot intrc before freeing the request block
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 14:54 ` sashiko-bot
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] s390/mm: Fix handling of vmalloc area in do_secure_storage_access() Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18] s390/mm: Add missing mm check to do_secure_storage_access() Sasha Levin
2 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Stefan Haberland, Jan Höppner, Jens Axboe, Sasha Levin, hca,
gor, agordeev, linux-s390, linux-kernel
From: Stefan Haberland <sth@linux.ibm.com>
[ Upstream commit dc3e3f7306cb74066f231251602b1be5aaec7bd8 ]
__dasd_cleanup_cqr() maps the completion result to a block status by
reading cqr->intrc, but only after discipline->free_cp() has returned the
request block to its memory pool (dasd_eckd_free_cp() ends in
dasd_sfree_request()). On SMP another CPU can reallocate that block and
overwrite cqr->intrc before it is read, completing the request with the
wrong error. proc_bytes is already snapshotted before free_cp() for the
same reason; do the same for intrc.
Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>
Signed-off-by: Stefan Haberland <sth@linux.ibm.com>
Link: https://patch.msgid.link/20260805111612.1285190-5-sth@linux.ibm.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **5.10.270** (`pending-5.10`). The DASD cleanup path
here still reads `cqr->intrc` after the CQR is returned to the per-
device pool, so this is a real, self-contained race fix for this
checkout.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
- Record: **[s390/dasd] [Snapshot] Snapshot `intrc` before freeing the
request block**
- Subsystem: `s390/dasd` (IBM mainframe block driver). Action is
“snapshot”, which here means copy a field before free so it cannot be
overwritten — a use-after-reuse fix.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
- Record:
- `Reviewed-by: Jan Höppner <hoeppner@linux.ibm.com>` (DASD co-
maintainer)
- `Signed-off-by: Stefan Haberland <sth@linux.ibm.com>` (DASD
maintainer / author)
- `Link:
https://patch.msgid.link/20260805111612.1285190-5-sth@linux.ibm.com`
- `Signed-off-by: Jens Axboe <axboe@kernel.dk>` (block maintainer;
applied to the block tree)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Cc: stable` (absence
expected for this review)
- Ignore pipeline `Signed-off-by: Sasha Levin` if present
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
- Record:
- **Bug:** `__dasd_cleanup_cqr()` maps completion to `blk_status_t` by
reading `cqr->intrc` *after* `discipline->free_cp()`.
`dasd_eckd_free_cp()` ends in `dasd_sfree_request()`, which returns
the CQR to the device CCW memory pool.
- **Failure mode:** On SMP another CPU can reallocate that block and
overwrite `intrc`; the original request is completed with the
**wrong error**.
- **Root cause:** Same class of bug already handled for `proc_bytes`
(snapshotted before `free_cp()`); `intrc` was left unsaved.
- **Version info:** None in the message. The pattern exists since
detailed I/O-error mapping (2013).
**Step 1.4: DETECT HIDDEN BUG FIXES**
- Record: **Not hidden.** Explicit race / use-after-reuse. Cover letter
for the 19-patch series (`[PATCH 00/19] s390/dasd: ESE Performance
improvements`) states: “The first 4 patches are fixes for existing
bugs in this area.” This is patch 04/19. Later patches are ESE
performance features; this one is standalone.
---
## PHASE 2: DIFF ANALYSIS - LINE BY LINE
**Step 2.1: INVENTORY THE CHANGES**
- Record:
- File: `drivers/s390/block/dasd.c` only (`+8 / -2`)
- Function: `__dasd_cleanup_cqr()`
- Scope: **single-file surgical fix**
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
- Record:
- **Before:** `proc_bytes = cqr->proc_bytes;` then `status =
...->free_cp(cqr, req);` then `switch (cqr->intrc)` on the failed
path (`status == 0`).
- **After:** Also `intrc = cqr->intrc;` before `free_cp()`, then
`switch (intrc)`.
- Path: block-layer completion / error mapping after I/O or ERP; not
an init-only path.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
- Record:
- **Category:** Use-after-free / use-after-reuse (custom memory pool),
with a race.
- **Mechanism:** `dasd_sfree_request()` drops `device->mem_lock` after
`dasd_free_chunk(&device->ccw_chunks, ...)`.
`dasd_smalloc_request()` on another CPU takes that lock, allocates
the same chunk, and `memset(cqr, 0, sizeof(*cqr))`, then may set a
new `intrc`. Cleanup then maps garbage/zero/`intrc` from a
*different* request.
- Verified in this tree: `dasd_eckd_free_cp()`, `dasd_fba_free_cp()`,
and `dasd_diag_free_cp()` all call `dasd_sfree_request()` after
capturing `status = cqr->status == DASD_CQR_DONE`. `status` is
already a return value; only `intrc` was still loaded from freed
memory.
**Step 2.4: ASSESS THE FIX QUALITY**
- Record:
- **Obviously correct:** copies the already-accepted `proc_bytes`
pattern.
- **Minimal:** one local, one assignment, switch operand change,
comment.
- **Regression risk:** very low. Does not change locking, APIs, or
success/fail (`status` was already snapshotted via the return
value).
- Context in this tree uses `BLK_STS_NEXUS` (not
`BLK_STS_RESV_CONFLICT`, renamed later in `7ba150834b840`). Backport
needs that context tweak only; the logic is identical.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
- Record:
- `__dasd_cleanup_cqr` since `8e09f21574ea30` (2008).
- `switch (cqr->intrc)` after `free_cp()`: `d1ffc1f8660e74`
(“block/dasd: detailed I/O errors”, 2013) then `2a842acab109f4`
(blk_status_t, 2017).
- `proc_bytes` snapshot: `5e6bdd37c5526` (2020, ESE data-corruption
fix, `Cc: stable`).
- Bug present for ~13 years; `proc_bytes` was fixed in 2020 and **is**
in this tree; `intrc` was not.
**Step 3.2: FOLLOW THE FIXES: TAG**
- Record: No `Fixes:` tag. Introducing commits `d1ffc1f8660e74` and
`2a842acab109f4` **are** ancestors of HEAD (`git merge-base --is-
ancestor` exit 0).
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
- Record: Recent `dasd.c` work is other DASD correctness fixes (ESE,
hanging unbind, etc.). This patch does not depend on them. Series
patches 02–03 touch `dasd_erp.c` / `dasd_eckd.c`, not this hunk.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
- Record: Stefan Haberland is listed in MAINTAINERS for S390 DASD (`M:
Stefan Haberland`, `M: Jan Hoeppner`). Jan Höppner reviewed. High-
confidence maintainer fix.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
- Record: **Standalone.** Needs only `__dasd_cleanup_cqr` + `cqr->intrc`
after `free_cp()`, both present. Not blocked on ESE full-track patches
05–19. Applied by Axboe as `dc3e3f7306cb74066f231251602b1be5aaec7bd8`
(block tree); that SHA is not in this checkout, which is expected.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
- Record:
- Lore/msgid: `20260805111612.1285190-5-sth@linux.ibm.com` (patch
04/19; msgid `-5` is 1-based including cover).
- `b4 mbox` retrieved a 40-message thread (WebFetch to lore returned
403).
- Cover letter: first four patches are **existing bug fixes**; rest is
ESE performance.
- Jens Axboe: “Applied, thanks!” and listed this as commit
`dc3e3f7306cb74066f231251602b1be5aaec7bd8`.
- No NAK. Sashiko flagged **pre-existing** UAFs (flush path `cqr->dq`
after free, `req->q` after `blk_mq_end_request`, etc.) — not
introduced here; author said leftover findings would be handled
separately.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
- Record: DASD maintainers (author + Reviewed-by Höppner); linux-block
via Axboe. Correct audience.
**Step 4.3: SEARCH FOR THE BUG REPORT**
- Record: No user/syzbot report. Bug is pool reuse, not `kfree`, so
KASAN often will not fire. Analysis is from code review by the
maintainer.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
- Record: 19-patch ESE series. This hunk does not need patches 05–19.
Independent of 01–03 for apply/correctness.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
- Record: **UNVERIFIED** — lore.kernel.org/stable WebFetch returned 403.
No evidence in the retrieved thread of a stable NAK.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
- Record: `__dasd_cleanup_cqr()` only.
**Step 5.2: TRACE CALLERS**
- Record (this tree):
1. Block tasklet (`dasd_block_tasklet` path ~2979): `dq = cqr->dq;`
lock; `__dasd_cleanup_cqr(cqr);` unlock. Normal I/O completion.
2. `dasd_flush_block_queue()` ~3085: same cleanup during flush/unbind.
- Called from tasklet / flush, not a rare ioctl-only path.
**Step 5.3: TRACE CALLEES**
- Record: `dasd_profile_end` (before free), `discipline->free_cp` →
`dasd_sfree_request` → pool free, then `blk_mq_end_request` /
`blk_mq_complete_request` / `blk_update_request`.
**Step 5.4: FOLLOW THE CALL CHAIN**
- Record: userspace I/O → `do_dasd_request` (`blk_mq_ops.queue_rq`) →
`build_cp` / `dasd_smalloc_request` → interrupt/tasklet →
`__dasd_cleanup_cqr`. **Reachable from ordinary block I/O** on DASD.
Default `nr_hw_queues = 4`, so cleanup on one queue does **not**
serialize `smalloc` on another (`do_dasd_request` only holds that
queue’s `dq->lock`; pool uses `device->mem_lock`). Internal CCWs also
`smalloc` without `dq->lock`. Race is real on SMP.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
- Record: `proc_bytes` already snapshotted in the same function.
`dasd_erp.c` also snapshots `proc_bytes` around ERP. No other
`cqr->intrc` use after `free_cp()` in this function.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
- Record: **Yes.** `drivers/s390/block/dasd.c` `__dasd_cleanup_cqr`
(around 2782–2835):
```2792:2797:drivers/s390/block/dasd.c
proc_bytes = cqr->proc_bytes;
status = cqr->block->base->discipline->free_cp(cqr, req);
if (status < 0)
error = errno_to_blk_status(status);
else if (status == 0) {
switch (cqr->intrc) {
```
- `make kernelversion` / Makefile: **5.10.270**. `git describe HEAD`:
`p-5.10-1-gce56b7a73b64b`.
- Introduced: 2013 (`d1ffc1f8660e74`); present for the entire 5.10
lifetime.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
- Record: **Minor context only.** This tree maps `-EPERM` to
`BLK_STS_NEXUS` without the later reservation comment. Same function
shape. Expected apply: fuzz on the `case -EPERM` block, or a 3-line
manual edit. No structural rework.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
- Record: `proc_bytes` snapshot is here (`5e6bdd37c5526`). `intrc` is
still read after free. No substitute fix.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
- Record: `drivers/s390/block/dasd*` — **IMPORTANT** on s390 (primary
disk driver for IBM Z). Not core mm/VFS, but production storage for
every DASD user.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
- Record: Mature driver, still receiving correctness fixes. Not staging.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
- Record: **Driver-specific / platform-specific:** s390 DASD
(`CONFIG_DASD`). Default 4 HW queues. Typical enterprise mainframe
disks.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
- Record:
- Failed block I/O (`free_cp` returns 0) **and** another CPU
reallocates the CQR from the 2-page `ccw_chunks` pool before `intrc`
is read.
- Window is a few instructions, but the pool is tiny and multi-queue
I/O makes reuse likely under load.
- Unprivileged users can issue disk I/O; they cannot freely choose the
race. Not a typical local-root primitive; it is a
correctness/memory-safety bug.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
- Record:
- `status` is already captured, so success vs fail of the request
stays correct.
- Wrong `intrc` maps `-EPERM` (stolen DASD lock → `BLK_STS_NEXUS`),
`-ENOLINK` (failfast/transport), `-ETIMEDOUT` to whatever the reused
block contains (usually 0 after `memset` → `BLK_STS_IOERR`).
- Stolen-lock reported as generic I/O error can break HA/cluster
handling (retry on a volume another system owns).
- **Severity: HIGH** (wrong block completion status; use-after-reuse).
Not a panic by itself.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
- Record:
- **Benefit:** High for s390 DASD — correct
timeout/transport/reservation errors; closes a real SMP reuse race.
- **Risk:** Very low — ~6 lines, local stack copy, same pattern as
`proc_bytes`.
- **Ratio:** Strongly favors backport.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real SMP use-after-reuse after `dasd_sfree_request()`.
- Wrong `blk_status_t` on failed I/O, including stolen-lock (`-EPERM`)
vs generic `IOERR`.
- Tiny, obviously correct, matches existing `proc_bytes` snapshot in
this file.
- DASD maintainer + co-maintainer review; Axboe applied it.
- Author classified it as a bug fix (series patches 1–4), not an ESE
feature.
- Buggy code is in 5.10.270; default 4 HW queues make the race
plausible.
- No new API/feature.
**AGAINST:**
- No `Reported-by` / syzbot / panic.
- Typical visible effect is misclassified errors, not a crash.
- Original patch context uses `BLK_STS_RESV_CONFLICT` (rename not in
5.10).
- Related pre-existing UAFs (flush `cqr->dq`, `req->q`) are **not**
fixed here — separate issues, not a reason to drop this snapshot.
**UNRESOLVED:**
- Stable-list discussion (lore 403).
- How often the race hits in production (unverified; maintainer asserts
it).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — same snapshot as
`proc_bytes`; reviewed; in mainline block tree.
2. Fixes a real bug that affects users? **PASS** — wrong completion
status on DASD I/O.
3. Important (crash, security, corruption, deadlock)? **PASS** — use-
after-reuse plus misreported reservation/timeout/transport errors on
the s390 disk path.
4. Small and contained? **PASS** — one function, ~6 lines of logic.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — hunk is here; map
`BLK_STS_RESV_CONFLICT` context to `BLK_STS_NEXUS`.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
- Record: Not a device-ID/quirk/DT/build/doc exception. Ordinary small
correctness fix.
**Step 9.4: DECISION**
Backport. The bug exists in this 5.10 tree, the fix is surgical and
independent of the ESE performance series, and the risk is negligible
compared with completing I/O with a stale `intrc`.
---
## What the commit solves and why it matters here
`__dasd_cleanup_cqr()` already saves `proc_bytes` before `free_cp()`
because that call returns the CQR to a 2-page per-device pool. It still
reads `cqr->intrc` afterward. With default four blk-mq queues, another
CPU can `dasd_smalloc_request()` the same chunk (`memset` to zero, then
a new `intrc`). Failed requests then get the wrong `blk_status_t`
(timeout, transport, or stolen lock reported as generic I/O error). That
is a real SMP race on the main s390 disk driver, not an ESE feature.
## Stable rules / risk vs benefit
Meets the stable bar: real bug, small, no new API, applies with a
trivial `BLK_STS_NEXUS` context tweak. Benefit is correct I/O error
reporting and closing use-after-reuse; regression risk is a local
integer copy.
## Concerns
- Original diff context uses `BLK_STS_RESV_CONFLICT`; this tree still
has `BLK_STS_NEXUS`. Adjust context; do not pull the rename.
- Do not pull series patches 05–19 (ESE performance).
- Pre-existing `cqr->dq` use after free in `dasd_flush_block_queue()` is
out of scope.
## Verification
- **[Phase 1]** Tags: Reviewed-by Höppner; SOB Haberland + Axboe; Link
msgid `...1285190-5`; no Fixes/Reported-by.
- **[Phase 1]** Cover letter via `b4 mbox`: “first 4 patches are fixes
for existing bugs.”
- **[Phase 2]** Diff: `intrc` snapshot in `__dasd_cleanup_cqr` only;
`+8/-2`.
- **[Phase 2]** This tree: `free_cp` → `dasd_sfree_request` →
`dasd_free_chunk(&device->ccw_chunks)`; `smalloc` `memset`s the CQR.
- **[Phase 3]** `git blame`: `switch (cqr->intrc)` from `2a842acab109f4`
/ `d1ffc1f8660e74`; `proc_bytes` from `5e6bdd37c5526`.
- **[Phase 3]** `git merge-base --is-ancestor` those SHAs vs HEAD: all
present.
- **[Phase 3]** Author is MAINTAINERS DASD `M:`.
- **[Phase 4]** `b4 mbox` 40 messages; Axboe applied as `dc3e3f7306cb`;
Sashiko notes are pre-existing, not this hunk.
- **[Phase 4]** WebFetch lore/stable: 403 — UNVERIFIED.
- **[Phase 5]** Callers: block tasklet ~2979, flush ~3085; `queue_rq =
do_dasd_request`; `nr_hw_queues = 4`.
- **[Phase 6]** `make kernelversion` 5.10.270; buggy `switch
(cqr->intrc)` at dasd.c:2797.
- **[Phase 6]** `BLK_STS_RESV_CONFLICT` absent here
(`include/linux/blk_types.h` has `BLK_STS_NEXUS` only).
- **[Phase 8]** Failure mode verified: `status` from `free_cp` return;
only error *class* races.
**YES**
drivers/s390/block/dasd.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index e7845c8ccc9ff..f6e86692a21d7 100644
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2736,17 +2736,23 @@ static void __dasd_cleanup_cqr(struct dasd_ccw_req *cqr)
struct request *req;
blk_status_t error = BLK_STS_OK;
unsigned int proc_bytes;
- int status;
+ int status, intrc;
req = (struct request *) cqr->callback_data;
dasd_profile_end(cqr->block, cqr, req);
+ /*
+ * free_cp() returns the request block to its memory pool, so snapshot
+ * everything still needed from cqr before calling it - another CPU can
+ * reallocate and overwrite the block right after.
+ */
proc_bytes = cqr->proc_bytes;
+ intrc = cqr->intrc;
status = cqr->block->base->discipline->free_cp(cqr, req);
if (status < 0)
error = errno_to_blk_status(status);
else if (status == 0) {
- switch (cqr->intrc) {
+ switch (intrc) {
case -EPERM:
/*
* DASD doesn't implement SCSI/NVMe reservations, but it
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 6.18] s390/mm: Fix handling of vmalloc area in do_secure_storage_access()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] s390/dasd: Snapshot intrc before freeing the request block Sasha Levin
@ 2026-10-19 13:51 ` Sasha Levin
2026-10-19 15:08 ` sashiko-bot
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18] s390/mm: Add missing mm check to do_secure_storage_access() Sasha Levin
2 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:51 UTC (permalink / raw)
To: patches, stable
Cc: Heiko Carstens, Christian Borntraeger, Vasily Gorbik, Sasha Levin,
agordeev, gerald.schaefer, linux-s390, linux-kernel
From: Heiko Carstens <hca@linux.ibm.com>
[ Upstream commit a2e5a9d853e015bc876399eb18435aa88101c60a ]
Secure storage access exceptions in vmalloc area need to be handled
separately since virt_to_phys() does not work on such addresses. Given
that there cannot be a valid access let such exceptions fail
immediately. The kernel will either fixup or crash.
Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>> ---
Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have enough history, the series context, and the local `fault.c` path
to decide. The remaining work is to write the phase-by-phase analysis.
## Phase 1: Commit message forensics
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[s390/mm]** **[Fix]** handling of vmalloc addresses in
`do_secure_storage_access()` — kernel secure-storage exceptions on
vmalloc must not go through `virt_to_folio()` / `virt_to_phys()`.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- `Tested-by: Christian Borntraeger <borntraeger@linux.ibm.com>`
- `Signed-off-by: Heiko Carstens <hca@linux.ibm.com>` (author; s390 arch
maintainer)
- `Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>`
(KVM/s390)
- `Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>` (s390 tree
maintainer)
- No `Fixes:` tag (expected for this review pipeline)
- No `Reported-by:`, `Link:`, or `Cc: stable@vger.kernel.org`
- Ignore any later Sasha Levin SOB / “Upstream commit” markers (none on
the raw commit)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** PGM 0x3d (secure storage access) on a vmalloc address is fed
to `virt_to_folio()` / `virt_to_phys()`, which are not valid for
vmalloc.
- **Symptom:** wrong folio/PFN, then `uv_convert_from_secure()` on the
wrong physical page, or a failed get and an unresolved fault (retry
loop). Cover letter: kernel should fix up or crash, not convert a
random page.
- **Version info in message:** none. Git places it in v7.3-rc1
(`v7.3-rc1~73^2~35`).
- **Root cause (author):** vmalloc is not a linear kernel mapping; there
is no legitimate convert-from-secure for that address, so fail
immediately via the existing fault-error path.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not hidden. Subject and body say **Fix**. Cover letter for v4 of
the series lists this as a real pre-existing bug, distinct from two
later “cosmetics” patches the author explicitly said were not bug fixes.
---
## Phase 2: Diff analysis
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `arch/s390/mm/fault.c` (+2 / −0)
- Function: `do_secure_storage_access()`
- Scope: **single-file surgical fix**, two lines in the kernel-fault
branch
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `is_kernel_fault()` → `virt_to_folio((void *)addr)` →
`folio_try_get` → `uv_convert_from_secure(folio_to_phys(folio))`.
- **After:** if `is_vmalloc_addr(addr)`, `return
handle_fault_error_nolock(regs, 0)` (exception-table fixup, or
oops/`die`). Linear-map kernel faults unchanged.
- Path: kernel (not user) PGM 0x3d on a vmalloc address.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** memory-safety / logic — invalid virt-to-phys on vmalloc;
can become wrong-page UV export or an unresolved fault.
- **Mechanism:** `virt_to_folio()` uses `virt_to_page()` → `__pa()`. On
s390 `__pa()` is identity/KASLR linear-map math
(`arch/s390/include/asm/page.h`), not a page-table walk. A vmalloc
address yields a bogus PFN. `uv_convert_from_secure()` then talks to
the Ultravisor about that PFN.
- **Fix:** reject vmalloc up front; same helper already used on other
kernel-fault failures in this file.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: vmalloc is not a valid convert-from-secure target;
the author states there is no valid access.
- Minimal; no unrelated edits; no new API.
- Regression risk is low. Kernel-mode `handle_fault_error_nolock(...,
0)` does not take the user `WARN_ON_ONCE(!si_code)` path; it
`fixup_exception()` or oopses. That matches “fixup or crash.”
- Follow-up `ecb00aa6fb7f4` still keeps this check after switching to
`__pa(addr)`, which confirms the check is not a folio-API workaround.
---
## Phase 3: Git history investigation
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Handler introduced in `084ea4d611a3` (2020, “s390/mm: add (non)secure
page access exceptions handlers”) — `phys_to_page(addr)` on
KERNEL_FAULT (virt treated as phys).
- `b95e0e792822b` (v7.1-rc2) switched that to `virt_to_folio()` (“Fix
phys_to_folio() usage”). Correct for the linear map, still wrong for
vmalloc.
- The two new lines are this commit, `a2e5a9d853e01` (Heiko Carstens,
2026-07-20).
- Current tree still has them (blame on `arch/s390/mm/fault.c` lines
440–441).
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Closest introduction of the *current* wrong
helper is `b95e0e792822b`; the vmalloc hole exists conceptually since
`084ea4d611a3`. Both are ancestors of this checkout.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Same-day series (AuthorDate 2026-07-20):
1. `7cf227b2707ed` — NULL `mm` check (user path; Sashiko)
2. `79ef8d384fdfb` — `lock_mm_and_find_vma()` (user path)
3. **`a2e5a9d853e01` — this vmalloc check (kernel path)**
4. `ecb00aa6fb7f4` — drop folio_try_get race; use `__pa()`
5. `da1f5a7dcf182` / `e462200886fb5` — author-labeled cosmetics
6. `656db3022083a` — re-enable `DCACHE_WORD_ACCESS`
This hunk is on the kernel-fault branch only and does not depend on 1–2.
Standalone as a two-line insert. `handle_fault_error_nolock` already
existed (`a5600f05d3d7a`, v6.13 era) and was already used in this
function (`b00be77302d7e`).
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Heiko Carstens is the s390 arch maintainer and owns most of this
function’s recent history. Christian Borntraeger tested and reviewed.
Not a drive-by.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Needs `is_kernel_fault()`, `is_vmalloc_addr()` (`<linux/mm.h>`,
already included), and `handle_fault_error_nolock()`. All present here.
Does **not** require re-enabling `DCACHE_WORD_ACCESS`. `git merge-base
--is-ancestor a2e5a9d853e01 HEAD` succeeds — already in this tree as a
historical fact, not as a selection marker.
---
## Phase 4: Mailing list and external research
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c a2e5a9d853e01` →
https://patch.msgid.link/20260720085834.898025-5-hca@linux.ibm.com
- **[PATCH v4 4/8]** in “s390: Reintroduce support for
DCACHE_WORD_ACCESS”
- Revisions: v1 (2026-07-16) → v2 → v3 → **v4 (applied)**
- v3 changelog: “Fix two more pre-existing bugs reported by Sashiko”
- Cover letter lists this as one of the real problems, separate from
kcore VM_SPARSE and from the folio-race patch
- Direct review: Borntraeger `Reviewed-by`; no NAK
- Vasily Gorbik: “Applied, thank you!”
- lore.kernel.org HTTP fetch returned 403 (Anubis). Thread recovered via
`b4 dig -m`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): Heiko Carstens, Alexander Gordeev, Sven Schnelle,
Vasily Gorbik, Christian Borntraeger, Janosch Frank, Claudio Imbrenda,
David Hildenbrand; `linux-s390@`, `kvm@`, `linux-kernel@`. Right
maintainers and lists.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla. Concrete trigger is documented in the **same
series**:
- Cover letter: DCACHE_WORD_ACCESS was reverted in `37540b8c287fc`
because secure-storage handling was incomplete (endless exception
loop); that revert had `Cc: stable`.
- Patch 1 (`aab0734094cef`): “The guest variable storage area is
allocated with **vmalloc** and then donated to the ultravisor. Any
kernel access to that area will result in a secure storage access
exception.” `/proc/kcore` via `vread_iter()` was a known short-
read/exception.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: 8-patch v4 series. This patch is self-contained. Later
`ecb00aa6fb7f4` keeps the vmalloc check. Re-enabling DCACHE_WORD_ACCESS
is a **feature** and is not required for this fix.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: Direct lore.kernel.org/stable search blocked (403). Unverified
whether anyone mailed stable@ for this SHA. The related revert
`37540b8c287fc` **was** tagged `Cc: stable`, so stable trees that had
word-at-a-time already treated this class of bug as important.
---
## Phase 5: Code semantic analysis
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `do_secure_storage_access()` only.
**Step 5.2: TRACE CALLERS**
Record:
- `arch/s390/kernel/traps.c` program-check table `[0x3d] =
do_secure_storage_access`
- Hardware PGM 0x3d (secure-storage access), not a syscall helper
- Compiled only under `#if IS_ENABLED(CONFIG_KVM)` in `fault.c`
**Step 5.3: TRACE CALLEES**
Record: `is_vmalloc_addr()`, `handle_fault_error_nolock()` →
`fixup_exception()` / `kfence_handle_page_fault()` / `die()`. Pre-fix
callees: `virt_to_folio`, `folio_try_get`, `uv_convert_from_secure`,
`folio_put`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable from:
- Kernel access to UV-donated **vmalloc** (guest variable storage;
kcore)
- `load_unaligned_zeropad()` spanning into a donated page once
DCACHE_WORD_ACCESS is on (this tree has it again via `656db3022083a`)
Unprivileged user can trigger kcore only with privilege; zeropad/dcache
is a common kernel path (namei/dcache) and can touch adjacent pages.
Protected-virt hosts are the affected population.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: No other `virt_to_folio` in this handler’s kernel-fault path
after `ecb00aa6fb7f4`. `uv_convert_from_secure()` in
`arch/s390/kernel/uv.c` takes a **physical** address; callers must not
pass vmalloc virt.
---
## 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**
- This is mainline 7.3-rc, not a `-stable` y-tree
- Function exists at `arch/s390/mm/fault.c:408`
- Current kernel-fault path still has the vmalloc check (lines 440–441)
then `__pa(addr)`
- Bug is real in any tree that still does `virt_to_folio`/`__pa` on the
fault address without a vmalloc guard — including this line of
development before `a2e5a9d853e01`
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Against **this** checkout the patch is already applied; the
surrounding kernel-fault `if` matches the parent of `a2e5a9d853e01`.
Expected difficulty on a tree that still has `folio =
virt_to_folio((void *)addr);` in that `if`: **clean apply**. Helpers
exist here.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This exact fix is already an ancestor of HEAD. That is mainline
history, not “already selected for stable.” No substitute vmalloc guard
appears before `a2e5a9d853e01`.
---
## Phase 7: Subsystem and maintainer context
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **arch/s390 MM + KVM protected virtualization**. Criticality:
**IMPORTANT** for s390 KVM PV (Ultravisor / confidential guests) — not
universal, but confidentiality and host stability for that platform.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (Heiko/Vasily/Borntraeger). Handler dates to
2020; this is a long-standing hole, not a brand-new subsystem.
---
## Phase 8: Impact and risk assessment
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Platform-specific + config-specific:** s390, `CONFIG_KVM`,
protected-virt host (pages donated to UV). Not all Linux users; all such
hosts that can take PGM 0x3d on vmalloc.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Kernel access to UV-donated vmalloc (documented: former vmalloc
stor_var + `/proc/kcore`)
- Word-at-a-time / zeropad crossing into donated memory (this tree has
DCACHE_WORD_ACCESS again)
- Not a userspace-unprivileged primitive in the usual sense; kcore needs
privilege; zeropad is kernel-internal but common on PV hosts
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- Wrong PFN → `uv_convert_from_secure()` on the **wrong** page: guest-
confidentiality break / host page state corruption → **CRITICAL** on
PV
- `folio_try_get` fail → silent return → instruction retry → **endless
PGM 0x3d** (the failure mode that forced the DCACHE revert) →
**CRITICAL** (lockup)
- Severity: **CRITICAL** when it hits; trigger is not every boot, but it
is a documented real path
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for s390 PV — avoids wrong UV export and exception
storms
- **Risk:** very low — two lines, existing helper, linear-map path
unchanged
- **Ratio:** strongly favors backport
---
## Phase 9: Final synthesis
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real bug: `virt_to_folio`/`__pa` on vmalloc is invalid on s390
- Failure modes: wrong-page `uv_convert_from_secure`, unresolved fault /
infinite PGM 0x3d
- Documented trigger: vmalloc UV storage; kcore; later zeropad
- 2-line, surgical, uses existing `handle_fault_error_nolock`
- Written/acked by s390 maintainers; Tested-by + Reviewed-by Borntraeger
- Author treated it as a bug fix, not as the DCACHE feature patch
- Follow-up kept the same check with `__pa`
- Code and helpers exist in this 7.3-rc4 tree
**AGAINST:**
- s390 + KVM PV only (smaller user base than core mm)
- Part of a larger DCACHE series — but this hunk does not enable that
feature
- No `Fixes:` / `Cc: stable` (not a negative per the rules)
- Direct stable@ lore search could not be completed (403)
**UNRESOLVED:**
- Whether stable@ already discussed this exact SHA (lore HTTP blocked)
- Exact older-stable context outside this checkout — not used for the
verdict
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — two-line guard; Tested-by
Borntraeger
2. Fixes a real bug that affects users? **PASS** — wrong phys conversion
/ exception loop on PV hosts
3. Important (crash, security, corruption, deadlock)? **PASS** — UV
export of the wrong page and/or host lockup
4. Small and contained? **PASS** — +2 lines, one function
5. No new features or APIs? **PASS** — not the DCACHE re-enable
6. Can apply to the local tree? **PASS** — helpers and call site exist;
this checkout already contains the change as mainline history
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: None (not a device-ID/quirk/DT/build/doc exception). Qualifies
as a normal important bug fix.
**Step 9.4: MAKE YOUR DECISION**
This tree is **Linux 7.3-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
surrounding handler, `is_vmalloc_addr()`, and
`handle_fault_error_nolock()` are all here. The bug is using linear-map
virt-to-phys on a vmalloc PGM 0x3d, which can export the wrong page from
secure storage or never resolve the fault.
The patch is two lines, maintainer-written, tested, and independent of
re-enabling `DCACHE_WORD_ACCESS`. Benefit is high for s390 protected-
virt; regression risk is very low. It meets the stable-kernel rules for
**this** checkout.
---
### What the commit solves (for this tree)
`do_secure_storage_access()` handled kernel PGM 0x3d by converting the
fault address with `virt_to_folio()`. That is only valid for the
identity/KASLR linear map. Vmalloc (including UV guest variable storage
allocated with vmalloc, and later word-at-a-time probes) must not take
that path. The fix fails those faults immediately so the kernel can
exception-fixup or oops instead of converting a random physical page.
### Verification
- **[Phase 1]** Parsed tags: Tested-by/Reviewed-by Borntraeger; SOB
Heiko + Vasily; no Fixes/Cc stable/syzbot
- **[Phase 2]** Diff: +2 lines in `do_secure_storage_access()` kernel-
fault branch; `is_vmalloc_addr` then `handle_fault_error_nolock`
- **[Phase 2]** Read `arch/s390/include/asm/page.h` `__pa_nodebug`:
linear map only, not vmalloc
- **[Phase 2]** Read `include/linux/mm.h` `virt_to_folio`:
`virt_to_page` → `__pa`
- **[Phase 3]** `git describe HEAD` / `make kernelversion`: v7.3-rc4 /
7.3.0-rc4
- **[Phase 3]** `git merge-base --is-ancestor a2e5a9d853e01 HEAD`: yes
(mainline ancestor, not a selection marker)
- **[Phase 3]** `git show a2e5a9d853e01`: matches the provided diff
- **[Phase 3]** `git blame` on `fault.c` 408–470: handler from
`084ea4d611a3` (2020); vmalloc lines from this commit
- **[Phase 3]** `git show b95e0e792822b`: phys_to_folio → virt_to_folio
in v7.1-rc2; vmalloc still wrong
- **[Phase 3]** `git show ecb00aa6fb7f4`: later `__pa` path **keeps**
the vmalloc check
- **[Phase 3]** `git show 37540b8c287fc`: DCACHE revert; endless PGM
0x3d; `Cc: stable`
- **[Phase 4]** `b4 dig -c/-a/-w/-m`: v4 4/8; lore msgid
`20260720085834.898025-5-hca@linux.ibm.com`; v1–v4; s390/KVM CCs;
Borntraeger R-b; Gorbik applied
- **[Phase 4]** WebFetch lore.kernel.org: 403; discussion taken from b4
mbox instead
- **[Phase 4]** Cover letter: this is a listed pre-existing bug, not
cosmetics
- **[Phase 4]** `git show aab0734094cef`: stor_var was vmalloc then
donated to UV; kernel access → PGM 0x3d
- **[Phase 5]** Callers: `traps.c` `[0x3d]`; `#if
IS_ENABLED(CONFIG_KVM)`
- **[Phase 5]** Read `handle_fault_error_nolock`: kernel path fixup or
`die`
- **[Phase 6]** Read current `fault.c` 439–449: vmalloc check still
present
- **[Phase 8]** Failure mode: wrong UV export and/or infinite exception
— CRITICAL for s390 PV
- **UNVERIFIED:** stable@ mailing-list threads for this SHA (lore 403)
The fix is small, obviously correct, and addresses a real
crash/corruption/confidentiality bug on this tree’s s390 KVM PV path.
**YES**
arch/s390/mm/fault.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 46e5748b5b1b3..9d7d3ef99fc16 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -433,6 +433,8 @@ void do_secure_storage_access(struct pt_regs *regs)
panic("Unexpected PGM 0x3d with TEID bit 61=0");
}
if (is_kernel_fault(regs)) {
+ if (is_vmalloc_addr((void *)addr))
+ return handle_fault_error_nolock(regs, 0);
folio = virt_to_folio((void *)addr);
if (unlikely(!folio_try_get(folio)))
return;
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH AUTOSEL 6.18] s390/mm: Add missing mm check to do_secure_storage_access()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] s390/dasd: Snapshot intrc before freeing the request block Sasha Levin
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18] s390/mm: Fix handling of vmalloc area in do_secure_storage_access() Sasha Levin
@ 2026-10-19 13:53 ` Sasha Levin
2026-10-19 15:26 ` sashiko-bot
2 siblings, 1 reply; 6+ messages in thread
From: Sasha Levin @ 2026-10-19 13:53 UTC (permalink / raw)
To: patches, stable
Cc: Heiko Carstens, sashiko-bot, Christian Borntraeger, Vasily Gorbik,
Sasha Levin, agordeev, gerald.schaefer, linux-s390, linux-kernel
From: Heiko Carstens <hca@linux.ibm.com>
[ Upstream commit 7cf227b2707ed0129018c283ffc8f893945586db ]
Similar like the normal exception handler the secure storage access
exception handler needs to verify if mm is not NULL, which may happen
with kernel threads.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260717132345.539A11F00A3A@smtp.kernel.org/
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>> ---
Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
candidate is `7cf227b2707ed` (“s390/mm: Add missing mm check to
do_secure_storage_access()”). The function and the unguarded
`current->mm` path exist here; the decision is whether this fix belongs
in this tree on technical merit.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1 — Subject line**
- Record: **[s390/mm]** **[Add]** missing `mm` check in
`do_secure_storage_access()`. “Add missing … check” is a correctness
fix, not a feature.
**Step 1.2 — Tags**
- Record:
- `Reported-by: sashiko-bot <sashiko-bot@kernel.org>` — static/AI
review, not syzbot
- `Closes: https://lore.kernel.org/all/20260717132345.539A11F00A3A@smt
p.kernel.org/`
- `Signed-off-by: Heiko Carstens <hca@linux.ibm.com>` (author; s390
architecture maintainer)
- `Reviewed-by: Christian Borntraeger <borntraeger@linux.ibm.com>`
(KVM/s390)
- `Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>` (s390 maintainer,
applied)
- No `Fixes:`, no `Cc: stable` (expected for this pipeline)
- Ignore any Sasha Levin SOB (none on the real commit)
**Step 1.3 — Body**
- Record:
- **Bug:** secure-storage exception handler did not check `current->mm
!= NULL`, unlike `do_exception()`.
- **Symptom:** kernel thread with `mm == NULL` can hit PGM 0x3d and
then `mmap_read_lock(NULL)`.
- **Failure mode:** NULL dereference in the exception handler.
- **Root cause:** user-fault path assumed `current->mm` was always
valid.
- **Version info:** none in the message. Function dates to 2020
(`084ea4d611a3d0`).
**Step 1.4 — Hidden bug fix?**
- Record: Not hidden. Explicit missing-check / NULL-deref fix. Not
cleanup disguised as a fix.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1 — Inventory**
- Record:
- File: `arch/s390/mm/fault.c` (+2 / −3)
- Function: `do_secure_storage_access()`
- Scope: single-file surgical fix
**Step 2.2 — Code flow**
- Record:
- **Before:** user-fault path did `if (faulthandler_disabled())` then
`mm = current->mm; mmap_read_lock(mm)`.
- **After:** `mm = current->mm` is taken first; path is `if
(faulthandler_disabled() || !mm) return
handle_fault_error_nolock(regs, 0);`
- Affected path: non-kernel-fault PGM 0x3d (user mode, or kernel
uaccess via secondary space).
- `handle_fault_error_nolock()` is already the no-lock fallback: user
SIGSEGV, or kernel `fixup_exception()` / oops.
**Step 2.3 — Bug mechanism**
- Record:
- **Category:** memory safety — NULL pointer dereference
- **Mechanism:** `mmap_read_lock(mm)` with `mm == NULL` (kernel
thread).
- **Fix:** same `!mm` guard as `do_exception()` at line 285.
**Step 2.4 — Fix quality**
- Record:
- Obviously correct: one extra condition, same pattern as the normal
fault handler.
- Minimal; no unrelated edits.
- Regression risk is very low: the new path only avoids locking a NULL
`mm`. For kernel uaccess, `handle_fault_error_nolock()` runs
exception fixups and returns `-EFAULT`, which is the intended
behavior (same as `do_exception()` and as `11709abccf93b` for atomic
context).
---
## PHASE 3: GIT HISTORY
**Step 3.1 — Blame**
- Record:
- `do_secure_storage_access()` introduced in `084ea4d611a3d0`
(2020-01-21): already did `mm = current->mm;
down_read(&mm->mmap_sem)` with no NULL check.
- `11709abccf93b` (2025-06-03) added `faulthandler_disabled()` after a
real qemu crash (`copy_page_from_iter_atomic` → PGM 0x3d while
`in_atomic`), but still assigned `mm = current->mm` afterward with
no `!mm` check.
- The `!mm` check on this path is `7cf227b2707ed` itself.
- `do_exception()` has had `faulthandler_disabled() || !mm` for years
(`a12d622ad541b5` / `70ffdb9393a72`).
**Step 3.2 — Fixes: tag**
- Record: No `Fixes:` tag. The incomplete guard is from `11709abccf93b`
(in this tree). The NULL-mm hole has existed since the handler was
added in 2020.
**Step 3.3 — File history**
- Record: This is v4 2/8 of “Reintroduce support for
DCACHE_WORD_ACCESS”. Neighbors:
- 1/8 VM_SPARSE (unrelated feature)
- 3/8 `lock_mm_and_find_vma()` (refactor; still needs `mm != NULL`)
- 4–7 more handler cleanups
- 8/8 DCACHE_WORD_ACCESS feature
- This patch is standalone. It does not depend on 1/8 or 8/8.
**Step 3.4 — Author**
- Record: Heiko Carstens is s390 architecture maintainer (`MAINTAINERS`:
S390 ARCHITECTURE). He wrote the 2025 atomic-context fix and this
follow-up. Borntraeger reviewed.
**Step 3.5 — Prerequisites**
- Record: None. It only adds `|| !mm` next to an existing
`faulthandler_disabled()` check. Applies standalone on this tree’s
`fault.c`.
---
## PHASE 4: MAILING LIST / EXTERNAL
**Step 4.1 — Original discussion**
- Record:
- `b4 dig -c 7cf227b2707ed` →
https://patch.msgid.link/20260720085834.898025-3-hca@linux.ibm.com
- `b4 dig -a`: v1/v2 were VM_SPARSE-only; v3 “Reintroduce
DCACHE_WORD_ACCESS”; v4 is the applied series. This mm-check first
appears as v4 2/8.
- Sashiko report (Closes:): **[High] Missing !mm check … kernel
threads faulting on user-space accesses … NULL pointer
dereference.** Example given: `copy_from_user()` onto a secure page
from a kernel thread, then `mmap_read_lock(NULL)`.
- Alexander Gordeev (S390 MM) called that sashiko note a **false
positive** on a related patch.
- Heiko still took the check: he wanted the handler to match
`do_exception()` and to address the sashiko reports. Borntraeger
gave `Reviewed-by` on this exact patch. No NAK.
**Step 4.2 — Recipients (`b4 dig -w`)**
- Record: Heiko, Gordeev, Schnelle, Gorbik, Borntraeger, Frank,
Imbrenda, Hildenbrand; `linux-s390@`, `kvm@`, `linux-kernel@`. Right
maintainers were on CC.
**Step 4.3 — Bug report**
- Record: Sashiko static finding, severity High. Not a user crash log
for the `!mm` case. The related 2025 bug *was* a real qemu oops on
this same handler. WebFetch/curl to lore returned 403; thread was
recovered with `b4 mbox`.
**Step 4.4 — Series**
- Record: Bundled with a feature series, but this hunk is an independent
NULL check. Other 2/8–7/8 patches are not required for this guard.
**Step 4.5 — Stable list**
- Record: **UNVERIFIED** — lore.kernel.org `/stable/` returned 403. No
stable discussion found in the saved v4 mbox (no `Cc: stable` in
review).
---
## PHASE 5: CODE SEMANTICS
**Step 5.1 — Functions**
- Record: `do_secure_storage_access()` only.
**Step 5.2 — Callers**
- Record: Trap table in `arch/s390/kernel/traps.c`: `[0x3d] =
do_secure_storage_access`. Hardware PGM 0x3d (secure storage access).
`COND_TRAP` weak-aliases to `default_trap_handler` if the symbol is
absent. Reached from `__do_pgm_check` → exception handler, not a
normal syscall helper.
**Step 5.3 — Callees**
- Record: After the check: `lock_mm_and_find_vma` / `mmap_read_lock`,
`folio_walk_*`, `arch_make_folio_accessible`. On the new path:
`handle_fault_error_nolock()` only (no lock, no folio walk).
**Step 5.4 — Reachability**
- Record: `is_kernel_fault()` is false for user mode, or kernel mode
with `teid.as == PSW_BITS_AS_SECONDARY` (uaccess). User processes have
an `mm`. Kernel threads have `current->mm == NULL` unless `use_mm()`.
- Reachable in principle: kernel-thread uaccess to a secure page without
`use_mm()`.
- Confirmed real traffic on this handler: `11709abccf93b` stack (qemu
`vfs_write` → `copy_page_from_iter_atomic` → PGM 0x3d). Those tasks
have an `mm`; they hit the atomic case, not `!mm`.
- Userspace-reachable for normal tasks: yes, but they have `mm`, so this
check does not change that path.
**Step 5.5 — Similar patterns**
- Record: `do_exception()` already uses `is_kernel_fault() ||
faulthandler_disabled() || !mm`. This commit brings the secure handler
in line. No other copy of this hole in `fault.c`.
---
## PHASE 6: LOCAL TREE
**Step 6.1 — Does the buggy code exist here?**
- Record: Yes. Tree is **7.3.0-rc4**. `do_secure_storage_access()` is in
`arch/s390/mm/fault.c` under `CONFIG_KVM`. Current source already has
`if (faulthandler_disabled() || !mm)` at line 451 (`git merge-base
--is-ancestor 7cf227b2707ed HEAD` succeeded). The unguarded
`mmap_read_lock(current->mm)` pattern was present from 2020 until this
commit (v7.3-rc1).
**Step 6.2 — Backport difficulty**
- Record: Clean apply on this tree. Later refactors
(`lock_mm_and_find_vma`, goto, `__context_unsafe`) still keep the
`!mm` check.
**Step 6.3 — Related fixes already here?**
- Record: `11709abccf93b` is here (atomic/`faulthandler_disabled` only).
No earlier `!mm` fix on this path. This commit is the `!mm` fix.
---
## PHASE 7: SUBSYSTEM
**Step 7.1**
- Record: **s390 MM / Secure Execution (Ultravisor) exception
handling**. Criticality: **IMPORTANT** on s390 with KVM/protected
virtualization; not core-universal. Not staging.
**Step 7.2**
- Record: Active. Multiple 2025–2026 fixes on this same function (atomic
handling, donated-memory fixups, folio_walk, vmalloc). Mature handler,
still getting real-bug fixes.
---
## PHASE 8: IMPACT AND RISK
**Step 8.1 — Who is affected**
- Record: s390 systems with `CONFIG_KVM` that take PGM 0x3d (Secure
Execution / protected virt). Not universal.
**Step 8.2 — Trigger**
- Record: Kernel thread (`current->mm == NULL`) takes a user/secondary-
space secure-storage exception (e.g. uaccess to a secure page).
Unprivileged users do not directly set `mm` to NULL. Likelihood is
lower than the 2025 atomic qemu case; Gordeev called the sashiko
report a false positive. Heiko still treated the check as required for
parity with `do_exception()`.
**Step 8.3 — Failure mode**
- Record: `mmap_read_lock(NULL)` → kernel oops. **Severity: HIGH** if
the path runs; **MEDIUM** if it is only defensive. Exception-handler
NULL deref is still a crash class of bug.
**Step 8.4 — Risk vs benefit**
- Record:
- **Benefit:** Medium–high. Prevents an oops on a path that already
crashed qemu in atomic context; completes the standard three-way
fault-handler guard.
- **Risk:** Very low. One condition, error-path only, same semantics
as `do_exception()`.
- **Ratio:** Favor including it.
---
## PHASE 9: SYNTHESIS
**Step 9.1 — Evidence**
**For:**
- Real NULL-deref if `mm` is NULL before `mmap_read_lock()`.
- Same guard `do_exception()` has used for years.
- Same function already needed a production fix (`11709abccf93b`) for
kernel uaccess to secure pages.
- 5-line, obviously correct change; `handle_fault_error_nolock()` is the
right fallback (exception fixup / `-EFAULT`).
- Written by the s390 arch maintainer; Reviewed-by Borntraeger.
- Sashiko rated the hole High and described the `copy_from_user` +
kernel-thread scenario.
- Code exists in this 7.3-rc4 tree; no missing prerequisites.
**Against:**
- Gordeev called the sashiko `!mm` note a false positive (may not be hit
by real kernel threads that use `use_mm()`).
- No live crash log for the `!mm` case (sashiko static analysis).
- s390 + KVM + Secure Execution only.
- Landed as 2/8 of a feature series (the hunk itself is independent).
**Unresolved:**
- Stable-list history (lore `/stable/` 403).
- No public reproducer of a kernel-thread `!mm` crash.
**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** — one condition, reviewed by
KVM/s390, applied by s390 maintainer.
2. Fixes a real bug that affects users? **PASS** — NULL `mm` then
`mmap_read_lock()` is a real oops; sibling path already crashed qemu.
Trigger may be uncommon, but the hole is real.
3. Important (crash/security/corruption/deadlock)? **PASS** — exception-
handler NULL deref / oops.
4. Small and contained? **PASS** — +2/−3 in one function.
5. No new features/APIs? **PASS**.
6. Applies to this tree? **PASS** — `fault.c` and the user-fault path
are present; patch is trivial here.
**Step 9.3 — Exceptions**
- Record: None (not a device ID, quirk, DT, build, or docs fix).
Qualifies as a normal crash-prevention fix.
**Step 9.4 — Decision**
This tree has the handler and had the unguarded `mm` use. The fix is a
one-line NULL check that matches `do_exception()` and closes a crash
path in the PGM 0x3d handler. Benefit outweighs risk.
---
### What the commit solves (for this tree)
`do_secure_storage_access()` handles s390 PGM 0x3d. On the
user/secondary-space path it took `mmap_read_lock(current->mm)` without
checking `mm`. Kernel threads have `mm == NULL`, so that is a NULL
deref. `do_exception()` already checks `faulthandler_disabled() || !mm`.
The 2025 commit added only the atomic half after a qemu oops; this
commit adds the missing `!mm` half. On this 7.3-rc4 tree the function
exists and the check is the right fix.
### Risk vs benefit
Very low risk, meaningful crash-prevention benefit on s390 KVM/Secure
Execution. No API or structural change.
---
Verification:
- [Phase 1] Parsed tags: Reported-by sashiko-bot; Closes lore msgid
`20260717132345.539A11F00A3A`; Reviewed-by Borntraeger; SOB Heiko +
Gorbik; no Fixes/Cc-stable
- [Phase 1] Body: kernel-thread `mm == NULL` vs normal exception handler
- [Phase 2] Diff: `arch/s390/mm/fault.c` +2/−3; `if
(faulthandler_disabled() || !mm)` then `handle_fault_error_nolock`
- [Phase 2] Confirmed `handle_fault_error_nolock()` does
fixup/SIGSEGV/oops and does not unlock `mm`
- [Phase 3] `git show 7cf227b2707ed`; ancestor of HEAD
- [Phase 3] Blame: function from `084ea4d611a3d0` (2020); `!mm` on
`do_exception` long-standing; `faulthandler_disabled` only from
`11709abccf93b` (2025)
- [Phase 3] `git show 11709abccf93b`: real qemu stack, sleeping on
mmap_lock in atomic PGM 0x3d
- [Phase 3] Series neighbors 79ef8d384fdfb…46a8c8d142ba8 do not replace
this check
- [Phase 4] `b4 dig -c 7cf227b2707ed`:
https://patch.msgid.link/20260720085834.898025-3-hca@linux.ibm.com
- [Phase 4] `b4 dig -a`: v4 is the applied series; this is 2/8
- [Phase 4] `b4 dig -w`: s390 + KVM maintainers on CC
- [Phase 4] `b4 mbox` of Closes: sashiko [High] NULL deref; Gordeev
“false positive”; Heiko still aligned with `do_exception()`
- [Phase 4] WebFetch/curl lore: 403; UNVERIFIED: stable@ discussion
- [Phase 5] `git grep do_secure_storage_access`: `traps.c` PGM 0x3d +
`entry.h`
- [Phase 5] Read `is_kernel_fault()`: user mode or secondary AS → user
path
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; `make
kernelversion` = 7.3.0-rc4
- [Phase 6] Current `fault.c:451` has `faulthandler_disabled() || !mm`
- [Phase 7] MAINTAINERS: Heiko = S390 ARCHITECTURE; Gordeev = S390 MM
- [Phase 8] Failure mode: NULL deref in exception handler; trigger
narrower than the 2025 atomic bug
- UNVERIFIED: whether a kernel thread without `use_mm()` has hit this on
a production machine; Gordeev doubted it
The fix is small, matches the existing fault-handler contract, and
prevents an oops on this tree’s s390 secure-storage path.
**YES**
arch/s390/mm/fault.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 069f72703a915..46e5748b5b1b3 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -404,9 +404,9 @@ void do_secure_storage_access(struct pt_regs *regs)
{
union teid teid = { .val = regs->int_parm_long };
unsigned long addr = get_fault_address(regs);
+ struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
struct folio_walk fw;
- struct mm_struct *mm;
struct folio *folio;
int rc;
@@ -448,9 +448,8 @@ void do_secure_storage_access(struct pt_regs *regs)
if (rc)
return handle_fault_error_nolock(regs, 0);
} else {
- if (faulthandler_disabled())
+ if (faulthandler_disabled() || !mm)
return handle_fault_error_nolock(regs, 0);
- mm = current->mm;
mmap_read_lock(mm);
vma = find_vma(mm, addr);
if (!vma)
--
2.53.0
^ permalink raw reply related [flat|nested] 6+ messages in thread