Linux s390 Architecture development
 help / color / mirror / Atom feed
* [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

* Re: [PATCH AUTOSEL 6.18-5.10] s390/dasd: Snapshot intrc before freeing the request block
  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 14:54   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-19 14:54 UTC (permalink / raw)
  To: Sasha Levin
  Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-284-sashal@kernel.org?part=1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH AUTOSEL 6.18] s390/mm: Fix handling of vmalloc area in do_secure_storage_access()
  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 15:08   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-19 15:08 UTC (permalink / raw)
  To: Sasha Levin
  Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-404-sashal@kernel.org?part=1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH AUTOSEL 6.18] s390/mm: Add missing mm check to do_secure_storage_access()
  2026-10-19 13:53 ` [PATCH AUTOSEL 6.18] s390/mm: Add missing mm check to do_secure_storage_access() Sasha Levin
@ 2026-10-19 15:26   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-19 15:26 UTC (permalink / raw)
  To: Sasha Levin
  Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger

> 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>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-512-sashal@kernel.org?part=1


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-19 15:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [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 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 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
2026-10-19 15:26   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox