[PATCH AUTOSEL 6.18-6.1] scsi: core: Do not block on tag allocation in scsi_eh_lock_door()

[PATCH AUTOSEL 6.18-6.1] scsi: core: Do not block on tag allocation in scsi_eh_lock_door()

[Sasha Levin]

From: Zizhi Wo <wozizhi@huawei.com>

[ Upstream commit 732cb6bb37fd26863d5786522fb1997e7f5865b4 ]

scsi_eh_lock_door() is called from scsi_restart_operations() while the
host is still in the SHOST_RECOVERY state, i.e. before the host is
switched back to SHOST_RUNNING and scsi_run_host_queues() restarts the
queues. It allocates a request via scsi_alloc_request() with no flags,
so blk_mq_get_tag() may block waiting for a free sched tag when all tags
are already in use.

Those tags can be held by commands that were just requeued by
scsi_eh_flush_done_q() during error handling. Such commands cannot be
dispatched until the host leaves SHOST_RECOVERY and
scsi_run_host_queues() is called - which only happens *after*
scsi_eh_lock_door() returns.

This forms a circular dependency:

  - scsi_eh_lock_door(), running in the SCSI error handler thread, waits
    for a sched tag held by a requeued command;

  - the requeued command cannot complete and release its sched tag until
    the error handler thread leaves scsi_restart_operations() and restart
    the queues.

For devices with a single driver tag (e.g. USB storage) it is a
guaranteed deadlock and I/O that can never be submitted. This problem
has also been reproduced in our environment.

Locking the door is a best-effort operation, and scsi_eh_lock_door()
already returns silently when the request allocation fails. Pass
BLK_MQ_REQ_NOWAIT to scsi_alloc_request() so the allocation fails
instead of blocking when no tag is available. This breaks the circular
dependency and allows the error handler to finish restarting the queues,
after which the pending commands are dispatched normally.

Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://patch.msgid.link/20260723041238.1584632-1-wozizhi@huaweicloud.com
Signed-off-by: Martin K. Petersen (Oracle) <mkp@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

**Step 1.1 — Subject line**
- Record: `[scsi: core]` `[prevent/block]` — Do not block on tag
  allocation in `scsi_eh_lock_door()`.

**Step 1.2 — Tags**
- Record:
  - `Signed-off-by`: Zizhi Wo `<wozizhi@huawei.com>` (author)
  - `Reviewed-by`: Bart Van Assche `<bvanassche@acm.org>` (SCSI
    maintainer)
  - `Link`: https://patch.msgid.link/20260723041238.1584632-1-
    wozizhi@huaweicloud.com
  - `Signed-off-by`: Martin K. Petersen (Oracle) `<mkp@kernel.org>`
    (committer)
  - No `Fixes:`, `Reported-by:`, `Cc: stable@vger.kernel.org`, or
    `Tested-by:` tags.
  - Notable: maintainer review present; author reports reproduction in
    their environment.

**Step 1.3 — Body analysis**
- Record:
  - **Bug**: Circular dependency during SCSI error recovery —
    `scsi_eh_lock_door()` blocks in `blk_mq_get_tag()` while host is
    still in `SHOST_RECOVERY`, but tags are held by commands requeued by
    `scsi_eh_flush_done_q()` that cannot dispatch until
    `scsi_run_host_queues()` runs *after* `scsi_eh_lock_door()` returns.
  - **Symptom**: Guaranteed deadlock on single-tag devices (e.g. USB
    storage); I/O permanently stuck.
  - **Root cause**: Blocking tag allocation during recovery restart.
  - **Fix**: Pass `BLK_MQ_REQ_NOWAIT` so allocation fails fast; door-
    lock is already best-effort (silent return on failure).

**Step 1.4 — Hidden bug fix?**
- Record: No — this is an explicit deadlock fix, not disguised cleanup.

---

## Phase 2: Diff Analysis

**Step 2.1 — Inventory**
- Record: 1 file (`drivers/scsi/scsi_error.c`), 1 line changed (+1/-1).
  Function: `scsi_eh_lock_door()`. Scope: single-file surgical fix.

**Step 2.2 — Code flow change**
- Record:
  - **Before**: `scsi_alloc_request(..., 0)` → `blk_mq_get_tag()` may
    block indefinitely waiting for a sched tag.
  - **After**: `scsi_alloc_request(..., BLK_MQ_REQ_NOWAIT)` → returns
    `ERR_PTR(-EWOULDBLOCK)` immediately when no tag is available;
    existing `IS_ERR(req) return;` path handles it.

**Step 2.3 — Bug mechanism**
- Record: **Deadlock / lock ordering** — error-handler thread blocks on
  tag allocation while holding recovery state that prevents requeued
  commands from releasing tags. Category: synchronization deadlock in EH
  restart path.

**Step 2.4 — Fix quality**
- Record: Obviously correct. Minimal one-line change. Low regression
  risk — door locking is documented as best-effort; failure path already
  existed. Reviewed by SCSI maintainer.

---

## Phase 3: Git History Investigation

**Step 3.1 — Blame**
- Record: `scsi_eh_lock_door()` exists since kernel 2.6 (commit
  `1da177e`). Blocking allocation introduced in `68ec3b819a5d6` ("scsi:
  add a scsi_alloc_request helper", 2021-10-22, first in v5.16). Present
  in this tree.

**Step 3.2 — Fixes: tag**
- Record: N/A — no `Fixes:` tag. Buggy behavior introduced by
  `68ec3b819a5d6`, which is an ancestor of HEAD.

**Step 3.3 — Related file history**
- Record: Recent `scsi_error.c` changes in 6.18.y include EH wake
  reliability fixes (`c7a1509123720`, `219f009ebfd1e`) but nothing
  addressing this deadlock. Standalone fix, not part of a series.

**Step 3.4 — Author context**
- Record: Zizhi Wo is not a regular SCSI maintainer (other work in blk-
  throttle, xfs, tty). This is a targeted bug report/fix.

**Step 3.5 — Dependencies**
- Record: None. `BLK_MQ_REQ_NOWAIT`, `scsi_alloc_request()` flags
  parameter, and `IS_ERR` handling all exist in this tree. `git apply
  --check` on the patch succeeds cleanly.

---

## Phase 4: Mailing List and External Research

**Step 4.1 — Original discussion**
- Record: `b4 dig -c 732cb6bb37fd2` found v1 only at https://patch.msgid
  .link/20260723041238.1584632-1-wozizhi@huaweicloud.com. Bart Van
  Assche replied with `Reviewed-by`. No NAKs or objections in thread.

**Step 4.2 — Reviewers**
- Record: `b4 dig -w` shows CC to James Bottomley, Martin Petersen, Bart
  Van Assche, linux-scsi, linux-kernel. Appropriate maintainers
  included.

**Step 4.3 — Bug report**
- Record: Author states "reproduced in our environment." No
  syzbot/bugzilla link. Real-world reproduction claimed.

**Step 4.4 — Series context**
- Record: `b4 dig -a` shows single v1 patch — standalone, no series
  dependencies.

**Step 4.5 — Stable list discussion**
- Record: No stable-specific discussion found in mbox thread (no "Cc:
  stable" mentions). Absence is neutral per instructions.

---

## Phase 5: Code Semantic Analysis

**Step 5.1 — Key functions**
- Record: `scsi_eh_lock_door()`, called from
  `scsi_restart_operations()`, which is called from
  `scsi_error_handler()`.

**Step 5.2 — Callers**
- Record: `scsi_restart_operations()` ← `scsi_error_handler()` (EH
  kernel thread, process context). Triggered on SCSI I/O
  failures/timeouts — common path for all SCSI block devices.

**Step 5.3 — Callees**
- Record: `scsi_alloc_request()` → `blk_mq_alloc_request()` →
  `blk_mq_get_tag()`. On failure with NOWAIT: `ERR_PTR(-EWOULDBLOCK)`.
  On success: `blk_execute_rq_nowait()`.

**Step 5.4 — Reachability**
- Record: Trigger requires SCSI EH after device reset with door locked
  (`sdev->was_reset && sdev->locked`). Verified in
  `scsi_restart_operations()` at line 2200. Host stays in
  `SHOST_RECOVERY` until line 2215
  (`scsi_host_set_state(SHOST_RUNNING)`), which is *after*
  `scsi_eh_lock_door()`. Requeued commands from `scsi_eh_flush_done_q()`
  use `blk_mq_requeue_request(..., !scsi_host_in_recovery(...))` —
  during recovery, `kick_requeue_list` is false, so they cannot
  dispatch. Reachable from normal block I/O error paths.

**Step 5.5 — Similar patterns**
- Record: Historical USB SCSI EH deadlocks fixed in `7daf480483e60` and
  `c69e6f812bab0` (same file, same subsystem concern).
  `BLK_MQ_REQ_NOWAIT` already used elsewhere in SCSI (`sg.c` documents
  why it avoids NOWAIT for userspace). This is the correct use case for
  NOWAIT.

---

## Phase 6: Cross-Referencing Against Local Tree

**Step 6.1 — Buggy code exists?**
- Record: **YES.** Local tree is `v6.18.44` (`linux-6.18.y`). Current
  code at line 2160 still has `scsi_alloc_request(sdev->request_queue,
  REQ_OP_DRV_IN, 0)`. Fix commit `732cb6bb37fd2` is on mainline but
  **not** in this tree.

**Step 6.2 — Backport complications**
- Record: Clean apply confirmed via `git apply --check`. No conflicts
  expected.

**Step 6.3 — Related fixes already present?**
- Record: No — `git log -S"BLK_MQ_REQ_NOWAIT" --
  drivers/scsi/scsi_error.c` returns nothing. Fix not yet backported.

---

## Phase 7: Subsystem and Maintainer Context

**Step 7.1 — Subsystem criticality**
- Record: `drivers/scsi/scsi_error.c` — SCSI core error handling.
  **CORE/IMPORTANT** — affects all SCSI block storage (disks, USB
  storage, optical drives).

**Step 7.2 — Subsystem activity**
- Record: Actively maintained in 6.18.y with recent EH fixes
  (`c7a1509123720`, etc.).

---

## Phase 8: Impact and Risk Assessment

**Step 8.1 — Who is affected**
- Record: Users of SCSI block devices undergoing error recovery,
  especially USB storage (single tag) and optical media with door-lock
  after reset. Universal for SCSI-enabled kernels.

**Step 8.2 — Trigger conditions**
- Record: SCSI EH after reset on a device with locked door, with
  requeued commands occupying all queue tags. Reproducible per author;
  guaranteed on single-tag devices. Unprivileged users can trigger via
  normal I/O to removable media.

**Step 8.3 — Failure mode severity**
- Record: **CRITICAL** — permanent deadlock of EH thread; all I/O to
  affected device/host stuck indefinitely.

**Step 8.4 — Risk-benefit**
- Record: Benefit **very high** (prevents unrecoverable I/O hang). Risk
  **very low** (1-line change, existing failure path, best-effort
  operation). Strong positive ratio.

---

## Phase 9: Final Synthesis

**Evidence FOR backport:**
- Real, reproducible deadlock in SCSI EH restart path
- CRITICAL severity (permanent I/O hang)
- One-line, obviously correct fix
- Reviewed by SCSI maintainer (Bart Van Assche)
- Applies cleanly to 6.18.44
- Bug present since v5.16 (`scsi_alloc_request` migration)
- No dependencies or new APIs
- Historical precedent for USB/EH deadlock fixes in stable

**Evidence AGAINST backport:**
- None significant. Door-lock skip on tag exhaustion is acceptable
  (best-effort by design).

**Stable rules checklist:**
1. Obviously correct and tested? **PASS** — maintainer reviewed; author
   reproduced bug.
2. Fixes real bug? **PASS** — deadlock with concrete trigger.
3. Important issue? **PASS** — CRITICAL deadlock.
4. Small and contained? **PASS** — 1 line, 1 file.
5. No new features/APIs? **PASS**.
6. Can apply to local tree? **PASS** — clean apply verified.

**Exception category:** N/A (not a quirk/DT/build fix — standard bug
fix).

---

## Verification

- [Phase 1] `git show 732cb6bb37fd2`: parsed full commit message and
  tags
- [Phase 2] Diff: confirmed single-line `BLK_MQ_REQ_NOWAIT` change in
  `scsi_eh_lock_door()`
- [Phase 3] `git describe HEAD`: v6.18.44; `git blame` line 2160:
  blocking call from `68ec3b819a5d6` (v5.16)
- [Phase 3] `git merge-base --is-ancestor 68ec3b819a5d6 HEAD`: confirmed
  buggy code ancestor present
- [Phase 3] `git merge-base --is-ancestor 732cb6bb37fd2 HEAD`: fix NOT
  in tree
- [Phase 3] `git apply --check`: patch applies cleanly
- [Phase 4] `b4 dig -c 732cb6bb37fd2`: found lore thread
- [Phase 4] `b4 dig -w`: maintainers CC'd (Bottomley, Petersen, Van
  Assche)
- [Phase 4] `b4 dig -a`: single v1 patch, no series
- [Phase 4] mbox: Bart Van Assche `Reviewed-by`; no NAKs, no stable
  nomination
- [Phase 5] Read `scsi_restart_operations()`, `scsi_unjam_host()`,
  `scsi_eh_flush_done_q()`, `__scsi_queue_insert()`: confirmed recovery-
  state ordering and requeue behavior
- [Phase 5] Read `scsi_host_in_recovery()` in
  `include/scsi/scsi_host.h`: checks `SHOST_RECOVERY` etc.
- [Phase 5] Read `blk_mq_alloc_request()` / `blk_mq_get_tag()`: NOWAIT
  returns `ERR_PTR(-EWOULDBLOCK)` on no tag
- [Phase 6] Grep current tree line 2160: still uses flags `0` (buggy)
- [Phase 8] Confirmed `sdev->was_reset` set in
  `__scsi_report_device_reset()` during EH reset path

This commit fixes a real SCSI error-handler deadlock that can
permanently hang I/O — especially on single-tag USB storage — and the
fix is a minimal, reviewed, cleanly-applicable one-liner appropriate for
the 6.18.y stable tree.

**YES**

 drivers/scsi/scsi_error.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 13d46b6e0359e..857c2acc99645 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2157,7 +2157,7 @@ static void scsi_eh_lock_door(struct scsi_device *sdev)
 	struct scsi_cmnd *scmd;
 	struct request *req;
 
-	req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, 0);
+	req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, BLK_MQ_REQ_NOWAIT);
 	if (IS_ERR(req))
 		return;
 	scmd = blk_mq_rq_to_pdu(req);
-- 
2.53.0

[PATCH AUTOSEL 6.18-5.10] scsi: bfa: Reduce kernel stack usage in bfa_fcs_lport_fdmi_build_portattr_block()

[Sasha Levin]

From: Arnd Bergmann <arnd@arndb.de>

[ Upstream commit 57a6ed0b41677ccc5e28cc0976e495c1dfa33747 ]

bfa_fcs_fdmi_get_portattr() gets inlined into multiple places and has
two fairly large variables on the stack, to the point of causing a
warning in some randconfig builds:

drivers/scsi/bfa/bfa_fcs_lport.c:2198:1: error: stack frame size (1560) exceeds limit (1280) in 'bfa_fcs_lport_fdmi_build_portattr_block' [-Werror,-Wframe-larger-than]
 2198 | bfa_fcs_lport_fdmi_build_portattr_block(struct bfa_fcs_lport_fdmi_s *fdmi,
      | ^
drivers/scsi/bfa/bfa_fcs_lport.c:1856:1: error: stack frame size (1600) exceeds limit (1280) in 'bfa_fcs_lport_fdmi_build_rhba_pyld' [-Werror,-Wframe-larger-than]
 1856 | bfa_fcs_lport_fdmi_build_rhba_pyld(struct bfa_fcs_lport_fdmi_s *fdmi, u8 *pyld)
      | ^

Mark the inner function as noinline_for_stack to keep it separate from
the other variables and prevent multiple copies of the same variable to
get inlined here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://patch.msgid.link/20260611125601.3385418-1-arnd@kernel.org
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject Line
**Record:** `[scsi: bfa] [reduce] Reduce kernel stack usage in
bfa_fcs_lport_fdmi_build_portattr_block()` — prevents excessive stack
frame size from compiler inlining in the Brocade BFA FC driver's FDMI
attribute-building path.

### Step 1.2: Parse All Tags
**Record:**
- **Link:**
  https://patch.msgid.link/20260611125601.3385418-1-arnd@kernel.org
- **Signed-off-by:** Arnd Bergmann `<arnd@arndb.de>` (author)
- **Signed-off-by:** Martin K. Petersen `<martin.petersen@oracle.com>`
  (SCSI maintainer)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, or Cc:
  stable tags
- Notable: maintainer Signed-off-by; no syzbot or user bug reports

### Step 1.3: Commit Body Analysis
**Record:**
- **Bug:** `bfa_fcs_fdmi_get_portattr()` is inlined into callers that
  already hold large stack variables, pushing frame sizes to 1560/1600
  bytes.
- **Symptom:** Build failure with `-Werror,-Wframe-larger-than` (limit
  1280) in randconfig builds.
- **Affected functions:** `bfa_fcs_lport_fdmi_build_portattr_block`
  (1560 bytes) and `bfa_fcs_lport_fdmi_build_rhba_pyld` (1600 bytes).
- **Root cause:** Inlining duplicates `struct bfa_port_attr_s` and
  `struct bfa_lport_attr_s` locals from `get_portattr` into parent
  frames.
- **Fix:** Mark `bfa_fcs_fdmi_get_portattr()` as `noinline_for_stack` to
  keep its stack usage in a separate frame.
- No kernel version info in the message.

### Step 1.4: Hidden Bug Fix Detection
**Record:** Not a hidden runtime bug fix. This is an explicit **build
fix** for `-Wframe-larger-than` treated as error (`-Werror`). No crash,
corruption, or deadlock at runtime.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **Files:** `drivers/scsi/bfa/bfa_fcs_lport.c` — 1 line changed
  (attribute added to function declaration)
- **Functions modified:** `bfa_fcs_fdmi_get_portattr()` only
- **Scope:** Single-file, surgical (1-line change)

### Step 2.2: Code Flow Change
**Record:**
- **Before:** `static void bfa_fcs_fdmi_get_portattr(...)` — compiler
  may inline it into `bfa_fcs_lport_fdmi_build_portattr_block()`,
  `bfa_fcs_fdmi_get_hbaattr()` (called from
  `bfa_fcs_lport_fdmi_build_rhba_pyld()`), and other callers.
- **After:** `static noinline_for_stack void
  bfa_fcs_fdmi_get_portattr(...)` — function stays out-of-line; its
  `pport_attr` and `lport_attr` stack variables live in its own frame,
  not duplicated in callers.
- **Path affected:** FDMI attribute gathering during FC fabric
  registration (RHBA/RPRT/RPA CT payloads). Normal operation path, not
  error-only.

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Build failure / stack frame size (not a runtime memory-
  safety bug).
- **Mechanism:** Compiler inlining expands caller stack frames beyond
  `CONFIG_FRAME_WARN` limit. With `-Werror`, this becomes a hard compile
  error.

### Step 2.4: Fix Quality
**Record:**
- **Quality:** Obviously correct — standard kernel pattern for stack
  pressure (`noinline_for_stack` is defined as `noinline` in
  `include/linux/compiler_types.h`).
- **Regression risk:** Very low. Slight code-size/call-overhead cost on
  an infrequent FDMI registration path; no behavioral change.
- **No red flags:** No API changes, no locking changes, no data
  structure changes.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:**
- `bfa_fcs_fdmi_get_portattr()` introduced by Krishna Gudipati,
  2010-09-15 (commit `a36c61f9025b89`).
- `struct bfa_lport_attr_s lport_attr` added 2013-05-13 (commit
  `d7cbc3044f2b2`).
- Buggy inlining pattern has been present for many years; exposure
  depends on compiler inlining decisions and `CONFIG_FRAME_WARN`.

### Step 3.2: Fixes: Tag
**Record:** No Fixes: tag. N/A.

### Step 3.3: Related File History
**Record:**
- Prior related fix in same file: `a7a11b6cfec2c` (Mar 2021) — "Move a
  large struct from the stack onto the heap" for
  `bfa_fcs_lport_fdmi_build_rhba_pyld()` (1200-byte frame > 1024 limit).
  That fix is **present in this tree**.
- Recent changes: strscpy conversion, unused code removal, state machine
  type fixes — unrelated.
- **Standalone:** Yes — single one-line patch, not part of a series.

### Step 3.4: Author Context
**Record:** Arnd Bergmann is a prolific contributor of `-Wframe-larger-
than` build fixes across the tree. Martin K. Petersen is the SCSI
maintainer. Neither is the BFA driver author, but both are credible
reviewers for this class of fix.

### Step 3.5: Dependencies
**Record:** No dependencies. `noinline_for_stack` exists in this tree
(`include/linux/compiler_types.h:278`). Patch applies cleanly to current
`bfa_fcs_lport.c` at line 2630.

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
**Record:** Lore/patch.msgid.link fetch blocked by Anubis bot
protection. Could not retrieve thread. Commit message Link tag present
but content unverified.

### Step 4.2: Reviewers
**Record:** `b4 dig -c` failed (commit hash not in local tree).
Recipients unverified.

### Step 4.3: Bug Report
**Record:** No external bug report. Failure mode documented in commit
message (compiler error output from randconfig).

### Step 4.4: Related Patches
**Record:** Related prior fix `a7a11b6cfec2c` (heap allocation for HBA
attr struct) addresses the same class of problem in the same file and is
already in 6.18.y.

### Step 4.5: Stable List History
**Record:** Could not search lore (bot protection). No stable discussion
found.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `bfa_fcs_fdmi_get_portattr()` — builds port attribute
structure from HAL/driver info.

### Step 5.2: Callers
**Record:**
- `bfa_fcs_lport_fdmi_build_portattr_block()` — line 2212 (direct)
- `bfa_fcs_fdmi_get_hbaattr()` — line 2617 (indirect, via
  `bfa_fcs_lport_fdmi_build_rhba_pyld()` at line 1873)
- `bfa_fcs_lport_fdmi_build_portattr_block()` also called from
  `build_rprt_pyld()` and `build_rpa_pyld()`
- FDMI paths triggered from state machine during FC port online/fabric
  registration

### Step 5.3: Callees
**Record:** `bfa_fcport_get_attr()`, `fc_get_fc4type_bitmask()`,
`bfa_fcs_lport_get_*()` — attribute queries, no allocation in
`get_portattr` itself.

### Step 5.4: Reachability
**Record:** Reachable during FC HBA operation when FDMI registration
runs (port coming online on fabric). Not directly syscall-triggered, but
normal driver operation for Brocade FC hardware. Requires
`CONFIG_SCSI_BFA_FC`.

### Step 5.5: Similar Patterns
**Record:** Same file already uses heap allocation (`kzalloc`) for
`fcs_hba_attr` in `build_rhba_pyld()` (from `a7a11b6cfec2c`). This patch
uses the lighter-weight `noinline_for_stack` approach for
`get_portattr`.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
**Record:** **YES.** Local tree is **v6.18.44** (`stable/linux-6.18.y`).
At line 2630, function is still `static void
bfa_fcs_fdmi_get_portattr(...)` without `noinline_for_stack`. Large
stack variables (`pport_attr`, `lport_attr`) and callers
(`build_portattr_block`, `build_rhba_pyld`) all present. Fix commit is
**not** in this tree.

### Step 6.2: Backport Complications
**Record:** Clean apply expected — single-line attribute addition. No
conflicting recent changes in this area. File line numbers differ
slightly from commit message (2198→2198 area, 2630 for the function) but
context matches.

### Step 6.3: Related Fixes Already Present?
**Record:** Prior heap-based stack fix `a7a11b6cfec2c` is present. This
`noinline_for_stack` fix is **not** present — `git log -S
"noinline_for_stack"` returns nothing for this file.

---

## Phase 7: Subsystem and Maintainer Context

### Step 7.1: Subsystem
**Record:** `drivers/scsi/bfa/` — Brocade BFA Fibre Channel HBA driver.
**Criticality: PERIPHERAL** (niche PCI FC hardware, `CONFIG_SCSI_BFA_FC`
tristate module).

### Step 7.2: Activity
**Record:** Moderate maintenance activity (strscpy migration, dead code
removal, type fixes in 2024–2025). Mature, stable driver code with long
history.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** **Config-specific** — kernel builders with
`CONFIG_SCSI_BFA_FC=y/m` and `-Werror` (W=1/randconfig CI). Not runtime
users of already-built kernels.

### Step 8.2: Trigger Conditions
**Record:**
- `CONFIG_FRAME_WARN` default is **1280 on 32-bit** (`!64BIT`) and
  **2048 on 64-bit** (verified in `lib/Kconfig.debug:441-449`).
- Reported frame sizes: 1560/1600 bytes — **exceed 1280** (32-bit
  default) but **under 2048** (64-bit default).
- Build error requires `-Werror` treating the warning as error.
- **Practical trigger:** 32-bit kernel builds with default
  `FRAME_WARN=1280` and W=1, or any arch with `FRAME_WARN ≤ 1560` and
  W=1, with BFA enabled.
- On typical 64-bit stable builds with default `FRAME_WARN=2048`, this
  does **not** fail even with W=1.

### Step 8.3: Failure Mode Severity
**Record:** **Compilation error** — kernel fails to build. **Severity:
LOW-MEDIUM** for stable (blocks builds for a narrow config subset; no
runtime crash, security issue, or data corruption).

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** LOW — unblocks randconfig/W=1 builds for BFA on 32-bit
  (or low FRAME_WARN configs). No runtime user benefit.
- **Risk:** VERY LOW — one-line `noinline_for_stack`, zero behavioral
  change, standard kernel idiom.
- **Ratio:** Low benefit, very low risk. Qualifies as a build-fix
  exception but is not a high-priority stable item.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backporting:**
- Real build failure with `-Werror,-Wframe-larger-than` (documented
  compiler errors)
- Build-fix exception category per stable rules
- One-line, obviously correct, zero regression risk
- Same file had prior stack-frame fix (`a7a11b6cfec2c`) already in
  6.18.y
- Buggy code confirmed present in local 6.18.44 tree
- Applies cleanly

**AGAINST backporting:**
- **No runtime bug** — purely a compilation issue
- Affects only `CONFIG_SCSI_BFA_FC` (niche Brocade FC hardware)
- On 64-bit (majority of 6.18.y deployments), default `FRAME_WARN=2048`
  means frames of 1560/1600 do **not** trigger the error
- Primary impact is 32-bit + W=1 + BFA — very narrow audience
- No user reports, syzbot, or security implications
- Stable rule #3 ("important issue") is weakly satisfied — this is not
  crash/corruption/security

**Unresolved:**
- Lore discussion content (bot-blocked)
- Whether reviewers nominated for stable

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — standard
   `noinline_for_stack` idiom; maintainer SOB
2. Fixes a real bug affecting users? **PASS (narrow)** — real build
   failure, but only for specific build configs
3. Important issue? **BORDERLINE/FAIL** — build break, not
   crash/corruption/security; narrow config
4. Small and contained? **PASS** — 1 line
5. No new features or APIs? **PASS**
6. Can apply to local tree? **PASS** — clean apply to 6.18.44

### Step 9.3: Exception Category
**Record:** **BUILD FIX** — prevents compilation errors with `-Wframe-
larger-than` + `-Werror`.

### Step 9.4: Decision Rationale

This is a legitimate, surgical build fix that applies cleanly to
linux-6.18.y and follows a pattern already used in this same file.
However, the practical impact on 6.18.y is narrow: the documented frame
sizes (1560/1600) exceed the 32-bit default limit (1280) but not the
64-bit default (2048), so typical 64-bit stable builds are unaffected.
The failure only manifests in randconfig/W=1 CI with BFA enabled on
32-bit or low-FRAME_WARN configs — a peripheral driver on an uncommon
build path.

Per the instructions, build fixes are backport-eligible, and the fix is
trivially safe. The benefit is low but real for kernel builders hitting
this configuration. Given zero regression risk and the established
precedent of stack-frame fixes in this driver already being in stable,
this qualifies for backport under the build-fix exception.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body from user-provided commit
  message
- **[Phase 2]** Diff analysis: 1-line `noinline_for_stack` addition to
  `bfa_fcs_fdmi_get_portattr()`
- **[Phase 3]** `git describe HEAD`: v6.18.44-1-g2736c32da98b9
  (linux-6.18.y)
- **[Phase 3]** `git blame -L 2630,2640`: function from 2010,
  `lport_attr` from 2013
- **[Phase 3]** `git show a7a11b6cfec2c`: prior heap-based stack fix in
  same file, present in tree
- **[Phase 3]** `git log -S "noinline_for_stack"`: no match — fix not
  yet in tree
- **[Phase 4]** WebFetch/curl lore.kernel.org: blocked by Anubis —
  discussion unverified
- **[Phase 4]** `b4 dig -c 8c9d423129c07`: commit not in local tree,
  failed
- **[Phase 5]** `grep bfa_fcs_fdmi_get_portattr`: callers at lines 2212,
  2617 confirmed
- **[Phase 5]** Read call chain: FDMI state machine → send_rhba/rprt/rpa
  → build_*_pyld → get_portattr
- **[Phase 6]** Read `bfa_fcs_lport.c:2630`: `static void` without fix —
  buggy code present
- **[Phase 6]** `git log --oneline -20 --
  drivers/scsi/bfa/bfa_fcs_lport.c`: no conflicting changes
- **[Phase 7]** `drivers/scsi/Kconfig:1502`: `CONFIG_SCSI_BFA_FC`
  tristate, PCI FC
- **[Phase 8]** `lib/Kconfig.debug:441-449`: FRAME_WARN default 1280
  (!64BIT), 2048 (64BIT)
- **[Phase 8]** Struct size estimate: inlined frames ~1380+ bytes,
  consistent with reported 1560/1600
- **[Phase 8]** `include/linux/compiler_types.h:278`:
  `noinline_for_stack` defined as `noinline`

**YES**

 drivers/scsi/bfa/bfa_fcs_lport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/bfa/bfa_fcs_lport.c b/drivers/scsi/bfa/bfa_fcs_lport.c
index 9a85f417018f9..cabe408214b3f 100644
--- a/drivers/scsi/bfa/bfa_fcs_lport.c
+++ b/drivers/scsi/bfa/bfa_fcs_lport.c
@@ -2627,7 +2627,7 @@ bfa_fcs_fdmi_get_hbaattr(struct bfa_fcs_lport_fdmi_s *fdmi,
 
 }
 
-static void
+static noinline_for_stack void
 bfa_fcs_fdmi_get_portattr(struct bfa_fcs_lport_fdmi_s *fdmi,
 			  struct bfa_fcs_fdmi_port_attr_s *port_attr)
 {
-- 
2.53.0

[PATCH AUTOSEL 6.18-5.10] scsi: pm8001: Reject non-fatal dump when controller is crashed

[Sasha Levin]

From: Kumar Meiyappan <kumar.meiyappan@microchip.com>

[ Upstream commit aa3b8f56ef27ed72394a752820abdec4608b731c ]

pm80xx_get_non_fatal_dump() can be called even after the controller has
entered a fatal error state. In that case the forensic memory contents
are not safe to access for a non-fatal dump request, and attempting to
do so can trigger a call trace.

Check controller_fatal_error before reading the non-fatal dump buffer
and return -EINVAL when the controller is already in a crashed state.

This prevents non-fatal dump collection from running in an invalid
controller state.

Signed-off-by: Kumar Meiyappan <kumar.meiyappan@microchip.com>
Signed-off-by: Sagar Biradar <sagar.biradar@microchip.com>
Link: https://patch.msgid.link/20260416154650.415624-1-sagar.biradar@microchip.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject Line
Record: **[scsi: pm8001]** **[Reject]** — Reject non-fatal forensic dump
collection when the PMC-Sierra PM8001 controller is already in a fatal
error state.

### Step 1.2: Commit Message Tags
Record:
- **Fixes:** — not present (expected for manual review)
- **Reported-by:** — none
- **Tested-by:** — none
- **Reviewed-by:** — none
- **Acked-by:** — none
- **Link:** https://patch.msgid.link/20260416154650.415624-1-
  sagar.biradar@microchip.com
- **Cc: stable@** — present in original patch submission (verified via
  `b4 dig -w`)
- **Signed-off-by:** Kumar Meiyappan, Sagar Biradar, Martin K. Petersen
  (SCSI maintainer)

Notable: authors explicitly CC'd `stable@vger.kernel.org`; no syzbot or
user bug reports.

### Step 1.3: Commit Body Analysis
Record:
- **Bug:** `pm80xx_get_non_fatal_dump()` can run after the controller
  has entered fatal error state.
- **Symptom:** Attempting to read forensic memory in that state can
  trigger a kernel call trace.
- **Root cause:** Missing guard on `controller_fatal_error` before
  initiating non-fatal dump hardware access.
- **Fix approach:** Check `controller_fatal_error` and return `-EINVAL`
  early.
- **Version info:** none in commit message.

### Step 1.4: Hidden Bug Fix Detection
Record: **Yes, this is a straightforward bug fix** disguised as "reject"
rather than "fix", but it clearly prevents unsafe hardware access and
kernel call traces during error recovery.

---

## Phase 2: Diff Analysis

### Step 2.1: Change Inventory
Record:
- **Files:** `drivers/scsi/pm8001/pm80xx_hwi.c` (+7 / -0)
- **Function modified:** `pm80xx_get_non_fatal_dump()`
- **Scope:** Single-file, surgical fix

### Step 2.2: Code Flow Change
Record:
- **Before:** After mapping host forensic buffer pointer, function
  immediately increments `non_fatal_count` and performs MMIO
  writes/reads to the crashed controller (`pm8001_mw32`, `pm8001_mr32`,
  `pm8001_cw32`).
- **After:** If `controller_fatal_error` is true, log debug message and
  return `-EINVAL` before any controller interaction.
- **Path affected:** Sysfs read of `non_fatal_log` via
  `non_fatal_log_show()` → `pm80xx_get_non_fatal_dump()`.

### Step 2.3: Bug Mechanism
Record:
- **Category:** Logic/correctness fix — unsafe hardware access in
  invalid controller state.
- **Mechanism:** After fatal firmware error (`controller_fatal_error =
  true` set in interrupt handler at line 4084), forensic DMA/MMIO
  operations are unsafe; the function lacked the same state check used
  elsewhere in the driver.

### Step 2.4: Fix Quality
Record:
- **Quality:** Obviously correct; mirrors existing driver pattern
  (`pm8001_sas.c` task path, `pm80xx_chip_soft_rst()`).
- **Regression risk:** Very low — only rejects an operation that should
  never succeed on a dead controller.
- **Red flags:** None.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame / Introduction
Record:
- Buggy function exists in local tree at
  `drivers/scsi/pm8001/pm80xx_hwi.c:387-503`.
- Non-fatal dump sysfs feature originally added in `dba2cc03b9db`
  (2020-03-16, "scsi: pm80xx: sysfs attribute for non fatal dump").
- `controller_fatal_error` exists at lines 1445, 1650, 4084 in current
  tree.
- Local autosel tree has squashed history; per-file `git blame` is not
  reliable for introduction dating.

### Step 3.2: Fixes Tag
Record: **N/A** — no `Fixes:` tag present.

### Step 3.3: Related File History
Record:
- Sibling upstream commit `2a8fbcfb04aa9` ("scsi: pm8001: Reject
  firmware update in fatal error state") from same author/date — same
  pattern, different sysfs path; also **not** in current HEAD.
- Standalone v1 patch; not part of a multi-patch series.

### Step 3.4: Author Context
Record: Kumar Meiyappan / Sagar Biradar are Microchip pm8001 driver
authors. Martin K. Petersen (SCSI maintainer) committed the fix upstream
as `aa3b8f56ef27`.

### Step 3.5: Dependencies
Record: **Standalone.** No prerequisite commits required.
`controller_fatal_error` field and `pm80xx_get_non_fatal_dump()` both
exist in this tree. Patch applies at line ~403 with clean context
(verified against upstream diff and local file).

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
Record:
- **URL:** https://patch.msgid.link/20260416154650.415624-1-
  sagar.biradar@microchip.com
- **Series:** v1 only (no v2/v3)
- **Maintainer response:** Martin K. Petersen applied to 7.2/scsi-queue
  (May 22, 2026)
- **NAKs/concerns:** None found in thread
- **Stable nomination:** Authors CC'd `stable@vger.kernel.org` in
  original submission

### Step 4.2: Reviewers
Record: CC'd Martin K. Petersen, James Bottomley, Jack Wang, linux-scsi,
stable@, Brian King, Don Brace, and other Microchip engineers.

### Step 4.3: Bug Report
Record: No external bug report, syzbot, or stack trace in commit
message. Issue appears internally discovered by driver vendor during
fatal-error handling review.

### Step 4.4: Related Patches
Record: Related upstream fix `2a8fbcfb04aa9` for
`pm8001_store_update_fw()` — same failure class, separate commit. Not a
dependency for this fix.

### Step 4.5: Stable List History
Record: Patch was posted directly to stable@ (spinics stable archive).
No rejection or prior stable discussion found beyond the submission
itself.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
Record: `pm80xx_get_non_fatal_dump()` (modified), `non_fatal_log_show()`
(caller), fatal error ISR path setting `controller_fatal_error`.

### Step 5.2: Callers
Record:
- `non_fatal_log_show()` in `pm8001_ctl.c:588-591` directly returns
  result.
- Exposed as read-only sysfs host attribute `non_fatal_log` in
  `pm8001_host_attrs[]`.
- Called during diagnostic collection, typically by root/admin tooling
  after controller errors.

### Step 5.3: Callees
Record: `pm8001_mw32()`, `pm8001_mr32()`, `pm8001_cw32()` — MMIO
register access to potentially dead controller; forensic DMA buffer
reads from `FORENSIC_MEM`.

### Step 5.4: Reachability
Record:
- **Trigger chain:** Controller fatal firmware error → ISR sets
  `controller_fatal_error` → admin reads
  `/sys/class/scsi_host/hostN/non_fatal_log` → unsafe MMIO without
  guard.
- **Userspace reachable:** Yes, via sysfs read (requires appropriate
  permissions, typically root/CAP_SYS_ADMIN).

### Step 5.5: Similar Patterns
Record: Driver already guards fatal-error state in:
- `pm8001_sas.c:505` — task execution
- `pm80xx_hwi.c:1650` — soft reset MPI uninit
- `pm8001_sas.c:237-244` — phy control via `fatal_errors()`
- `pm8001_init.c:702-704` — NVMD read

This fix fills a gap in the same error-handling model.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
Record: **YES.** Local tree is **v6.18.43** (`git describe HEAD` →
`v6.18.43-1-gc7f0dac02d232`, `make kernelversion` → `6.18.43`).
`pm80xx_get_non_fatal_dump()` at lines 403-404 lacks
`controller_fatal_error` check. Upstream fix commit `aa3b8f56ef27` is
**NOT** an ancestor of HEAD.

### Step 6.2: Backport Complications
Record: **Clean apply expected.** Local file matches upstream pre-fix
context at the insertion point. No conflicting changes in that function
region.

### Step 6.3: Related Fixes Already Present?
Record: **No.** Neither `aa3b8f56ef27` (this fix) nor sibling
`2a8fbcfb04aa9` (firmware update guard) are in HEAD.
`non_fatal_log_show()` directly returns `ssize_t` from dump function, so
`-EINVAL` propagates correctly to sysfs (no need for separate error-code
fix `1b6f03b`).

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem Criticality
Record: **drivers/scsi/pm8001** — PERIPHERAL driver (Microchip/PMC-
Sierra SAS HBA, `CONFIG_SCSI_PM8001`). Important for deployments using
this hardware, but not core kernel.

### Step 7.2: Subsystem Activity
Record: pm8001 driver is mature; forensic dump sysfs has existed since
2020. Recent upstream activity includes fatal-error handling hardening
from Microchip (April 2026).

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
Record: **Driver-specific** — systems with PM8001/PM80xx SAS HBAs and
`CONFIG_SCSI_PM8001` enabled.

### Step 8.2: Trigger Conditions
Record:
- Controller must first enter fatal firmware error state (already a
  serious failure).
- Then sysfs read of `non_fatal_log` (diagnostic path during triage).
- **Likelihood:** Uncommon but realistic during failure investigation —
  exactly when admins collect dumps.
- **Privilege:** Typically root/admin.

### Step 8.3: Failure Mode Severity
Record:
- **Failure mode:** Kernel call trace from unsafe MMIO/DMA on dead
  controller.
- **Severity:** **MEDIUM-HIGH** — secondary kernel instability during
  already-critical failure recovery; not a normal-I/O-path crash, but
  can produce oops/warnings and complicate diagnostics.

### Step 8.4: Risk-Benefit
Record:
- **Benefit:** Prevents additional kernel call traces during HBA failure
  recovery; aligns driver error handling.
- **Risk:** Very low — 7-line guard, no API changes, no behavior change
  on healthy controllers.
- **Ratio:** Favorable for stable backport.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Real bug with concrete trigger (fatal error + sysfs non_fatal dump
  read)
- Prevents kernel call trace / unsafe hardware access
- Small (7 lines), obviously correct, standalone
- Buggy code confirmed present in v6.18.43 tree
- Consistent with existing `controller_fatal_error` checks in same
  driver
- Authors CC'd stable@; SCSI maintainer merged upstream
- Clean apply expected

**AGAINST backport:**
- Narrow hardware scope (PM8001 SAS HBAs only)
- Only affects diagnostic sysfs path after controller already failed
- No syzbot report or public user bug report
- "Call trace" severity not fully documented with stack trace

**Unresolved:**
- Exact call trace type (WARN vs oops) not documented in commit message
- Introduction date of `controller_fatal_error` not determinable from
  squashed local git history (but field is present in tree)

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — logic is clear; maintainer-
   applied; no Tested-by but low complexity.
2. Fixes a real bug? **PASS** — unsafe operation on crashed controller.
3. Important issue? **PASS** — kernel call trace during failure recovery
   (medium-high severity).
4. Small and contained? **PASS** — 7 lines, 1 file, 1 function.
5. No new features/APIs? **PASS** — guard only.
6. Can apply to local tree? **PASS** — code exists, patch applies
   cleanly.

### Step 9.3: Exception Categories
Record: **None** — standard driver bug fix.

### Step 9.4: Decision Rationale
This is a small, vendor-submitted fix for a real gap in fatal-error
handling. The buggy code is present in the local **6.18.43** tree, the
fix is minimal and follows established driver patterns, and it prevents
additional kernel instability during exactly the scenario where
administrators are collecting diagnostic data from a failed HBA. The
risk is negligible and the authors explicitly nominated it for stable.

---

## Verification

- [Phase 1] Parsed subject, tags; confirmed stable@ CC in original
  submission via `b4 dig -w`
- [Phase 1] No Fixes:/Reported-by/syzbot tags
- [Phase 2] Read full `pm80xx_get_non_fatal_dump()` (lines 387-503);
  confirmed MMIO access after missing guard
- [Phase 2] Diff is +7 lines adding `controller_fatal_error` check
  returning `-EINVAL`
- [Phase 3] `git describe HEAD` → v6.18.43; `make kernelversion` →
  6.18.43
- [Phase 3] `git merge-base --is-ancestor aa3b8f56ef27 HEAD` →
  NOT_IN_HEAD (fix not yet applied)
- [Phase 3] `git show aa3b8f56ef27^` at lines 400-415 confirms buggy
  pre-fix code
- [Phase 3] `dba2cc03b9db` identified as 2020 introduction of non_fatal
  dump sysfs
- [Phase 3] Sibling commit `2a8fbcfb04aa9` found (fw update guard); also
  NOT_IN_HEAD
- [Phase 4] `b4 dig -c aa3b8f56ef27` → lore URL found
- [Phase 4] `b4 dig -a` → v1 only, no later revisions
- [Phase 4] `b4 dig -w` → stable@, Martin Petersen, James Bottomley CC'd
- [Phase 4] `b4 dig -m /tmp/pm8001_thread.mbox` → Martin Petersen
  "Applied to 7.2/scsi-queue"
- [Phase 5] `grep pm80xx_get_non_fatal_dump` → caller
  `non_fatal_log_show()` in pm8001_ctl.c:591
- [Phase 5] `grep controller_fatal_error` → used in pm80xx_hwi.c,
  pm8001_ctl.c, pm8001_sas.c, pm8001_sas.h
- [Phase 5] Fatal error set at pm80xx_hwi.c:4084 in interrupt handler
- [Phase 6] Buggy code confirmed at pm80xx_hwi.c:403-404 without guard
- [Phase 6] `non_fatal_log_show()` directly returns ssize_t — `-EINVAL`
  propagation works
- [Phase 8] CONFIG_SCSI_PM8001 confirmed in drivers/scsi/Kconfig:1494
- [UNVERIFIED] Exact kernel version when `controller_fatal_error` was
  introduced (squashed local history)
- [UNVERIFIED] Full stack trace / oops type from original bug report
  (not in commit message)

**YES**The background check finished: `aa3b8f56ef27` is **not** in this
tree (`v6.18.43`), and the buggy code is still present in
`pm80xx_get_non_fatal_dump()`.

The backport analysis stands: **YES** — this is a small, low-risk guard
that prevents kernel call traces when reading `non_fatal_log` on a
controller already in fatal error state.

 drivers/scsi/pm8001/pm80xx_hwi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 31960b72c1e92..1ce8226fcd292 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -401,6 +401,13 @@ ssize_t pm80xx_get_non_fatal_dump(struct device *cdev,
 	char *buf_copy = buf;
 
 	temp = (u32 *)pm8001_ha->memoryMap.region[FORENSIC_MEM].virt_ptr;
+
+	if (pm8001_ha->controller_fatal_error) {
+		pm8001_dbg(pm8001_ha, FAIL,
+			   "non-fatal dump not available in fatal error state\n");
+		return -EINVAL;
+	}
+
 	if (++pm8001_ha->non_fatal_count == 1) {
 		if (pm8001_ha->chip_id == chip_8001) {
 			snprintf(pm8001_ha->forensic_info.data_buf.direct_data,
-- 
2.53.0

Re: [PATCH AUTOSEL 6.18-5.10] scsi: pm8001: Reject non-fatal dump when controller is crashed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Returning -EINVAL from pm80xx_get_non_fatal_dump() is improperly handled by its caller, which truncates the negative error code to a large positive u32 integer, leading to severe sysfs corruption and kernel memory leak.

Pre-existing issues:
- [High] Preexisting buffer overflow in pm80xx_get_non_fatal_dump due to incorrect snprintf size parameter and trusting unvalidated hardware register values for loop bounds.
- [High] The sysfs callbacks for fatal and non-fatal dump collection access and modify shared driver state and hardware registers without holding any locks.
--

commit 88fed450e7290e833f0fc22f3e8782d89c4fea56
Author: Kumar Meiyappan <kumar.meiyappan@microchip.com>

scsi: pm8001: Reject non-fatal dump when controller is crashed

pm80xx_get_non_fatal_dump() can be called even after the controller has
entered a fatal error state. In that case the forensic memory contents
are not safe to access for a non-fatal dump request, and attempting to
do so can trigger a call trace.

Check controller_fatal_error before reading the non-fatal dump buffer
and return -EINVAL when the controller is already in a crashed state.

Link: https://patch.msgid.link/20260416154650.415624-1-sagar.biradar@microchip.com

> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 31960b72c1e92..1ce8226fcd292 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -401,6 +401,13 @@ ssize_t pm80xx_get_non_fatal_dump(struct device *cdev,
>  	char *buf_copy = buf;
>  
>  	temp = (u32 *)pm8001_ha->memoryMap.region[FORENSIC_MEM].virt_ptr;
> +
> +	if (pm8001_ha->controller_fatal_error) {
> +		pm8001_dbg(pm8001_ha, FAIL,
> +			   "non-fatal dump not available in fatal error state\n");
> +		return -EINVAL;

[Severity: High]
Does returning -EINVAL here cause a massive sysfs memory leak?

The caller of pm80xx_get_non_fatal_dump() in drivers/scsi/pm8001/pm8001_ctl.c
is non_fatal_log_show(), which stores the return value in a u32:

drivers/scsi/pm8001/pm8001_ctl.c:non_fatal_log_show() {
	u32 count;

	count = pm80xx_get_non_fatal_dump(cdev, attr, buf);
	return count;
}

When pm80xx_get_non_fatal_dump() returns -EINVAL, count becomes a large
positive integer (4294967274). This is returned to the sysfs core as a ssize_t.
Because the sysfs core sees a huge length, it caps it to PAGE_SIZE and copies
uninitialized kernel memory to userspace.

> +	}
> +
>  	if (++pm8001_ha->non_fatal_count == 1) {

[Severity: High]
This isn't a bug introduced by this patch, but does this sysfs read callback 
access shared driver state and hardware registers concurrently without holding
any locks?

Since sysfs read callbacks are not serialized, multiple threads reading the
non_fatal_log sysfs file concurrently can increment pm8001_ha->non_fatal_count
and execute multi-step MMIO sequences simultaneously.

This also seems to affect pm80xx_get_fatal_dump() which modifies hardware
registers without serialization:

drivers/scsi/pm8001/pm80xx_hwi.c:pm80xx_get_fatal_dump() {
	...
	pm8001_cw32(pm8001_ha, 0, MEMBASE_II_SHIFT_REGISTER,
			pm8001_ha->fatal_forensic_shift_offset);
	...
}

Could concurrent reads overwrite each other's hardware doorbell requests and
corrupt the forensic DMA setup?


[Severity: High]
This is a pre-existing issue, but is it possible for snprintf to write
out-of-bounds later in this function?

drivers/scsi/pm8001/pm80xx_hwi.c:pm80xx_get_non_fatal_dump() {
	...
	accum_len = pm8001_mr32(nonfatal_table_address,
			MPI_FATAL_EDUMP_TABLE_ACCUM_LEN);
	output_length = accum_len -
		pm8001_ha->forensic_preserved_accumulated_transfer;

	for (index = 0; index < output_length/4; index++)
		buf_copy += snprintf(buf_copy, PAGE_SIZE,
				"%08x ", *(temp+index));
	...
}

The loop dictates the number of iterations using accum_len, which is read
directly from the hardware register MPI_FATAL_EDUMP_TABLE_ACCUM_LEN.

If the hardware provides a large value, snprintf is statically passed PAGE_SIZE
on every iteration instead of the actual remaining buffer space
(PAGE_SIZE - (buf_copy - buf)). This allows snprintf to write past the end of
the single-page sysfs buffer, leading to a kernel heap buffer overflow.

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

[PATCH AUTOSEL 6.18-5.10] scsi: pm8001: Reject firmware update in fatal error state

[Sasha Levin]

From: Kumar Meiyappan <kumar.meiyappan@microchip.com>

[ Upstream commit 2a8fbcfb04aa9db189bfa3842d4f586aecd0e631 ]

pm8001_store_update_fw() allows a firmware update request even when the
controller has already entered a fatal error state.

Firmware update is not valid once the controller is in that state, and
attempting it can lead to a call trace. Reject the request early by
checking controller_fatal_error, set the firmware status to
FAIL_PARAMETERS, and return -EINVAL.

Signed-off-by: Kumar Meiyappan <kumar.meiyappan@microchip.com>
Signed-off-by: Sagar Biradar <sagar.biradar@microchip.com>
Link: https://patch.msgid.link/20260416153757.414896-1-sagar.biradar@microchip.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `scsi: pm8001: Reject firmware update in
fatal error state`

**Local tree:** Linux **6.18.43** (`stable/linux-6.18.y`, `HEAD`
detached at `c7f0dac02d232`)
**Upstream commit:** `2a8fbcfb04aa9` (candidate on `autosel`:
`dc825300273da`)
**Fix status in this tree:** **Not present** — patch applies cleanly.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject line
**Record:** `[scsi: pm8001]` `[Reject]` — Reject firmware update when
the controller is already in a fatal error state.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Kumar Meiyappan, Sagar Biradar, Martin K. Petersen
  (subsystem maintainer)
- **Link:** https://patch.msgid.link/20260416153757.414896-1-
  sagar.biradar@microchip.com
- **No** Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Cc: stable (in
  commit message; stable was CC'd on the mailing list submission)
- Notable: Martin K. Petersen (SCSI maintainer) signed off and applied
  upstream.

### Step 1.3: Body analysis
**Record:**
- **Bug:** `pm8001_store_update_fw()` accepts firmware-update sysfs
  writes even when `controller_fatal_error` is already true.
- **Symptom:** Attempting a firmware update in that state can produce a
  **kernel call trace**.
- **Fix:** Early check of `controller_fatal_error`, set `fw_status =
  FAIL_PARAMETERS`, return `-EINVAL`.
- **Root cause:** Missing guard in the firmware-update sysfs path; other
  paths already check this flag.

### Step 1.4: Hidden bug fix?
**Record:** No — this is an explicit bug fix, not disguised cleanup.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
**Record:**
- **File:** `drivers/scsi/pm8001/pm8001_ctl.c` (+8 lines)
- **Function:** `pm8001_store_update_fw()`
- **Scope:** Single-file, surgical fix.

### Step 2.2: Code flow change
**Record:**
- **Before:** After parsing `buf` into command and filename, code
  proceeds to flash-command lookup and `request_firmware()` /
  `pm8001_update_flash()` even if the controller is in fatal error.
- **After:** After parameter parsing, if `controller_fatal_error` is
  true, log, set status, return `-EINVAL` via existing `out:` cleanup.
- **Path affected:** Sysfs write to `update_fw` (admin-only,
  error/recovery path after hardware failure).

### Step 2.3: Bug mechanism
**Record:**
- **Category:** Logic/correctness — missing state guard on an admin
  sysfs operation.
- **Mechanism:** Firmware flash IOCTLs
  (`PM8001_CHIP_DISP->fw_flash_update_req()`) assume a live controller.
  After fatal firmware error, hardware/firmware is not in a valid state;
  proceeding causes a kernel call trace. The fix mirrors the existing
  I/O rejection in `pm8001_task_exec()`.

### Step 2.4: Fix quality
**Record:**
- Obviously correct: uses existing `controller_fatal_error` flag and
  established error-handling pattern (`goto out`).
- Minimal, no API changes.
- **Regression risk:** Very low — only rejects an operation that is
  invalid by definition when the controller is crashed.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
**Record:** `controller_fatal_error` field and
`pm8001_store_update_fw()` exist in v6.18; the field is present back to
at least **v6.0**. The omission in the firmware-update path is long-
standing, not a regression from a recent commit.

### Step 3.2: Fixes: tag
**Record:** N/A — no Fixes: tag.

### Step 3.3: Related file history
**Record:**
- Sibling fix: `scsi: pm8001: Reject non-fatal dump when controller is
  crashed` (`aa3b8f56ef27e` / autosel `3b6992159c00d`) — same pattern,
  different sysfs path; also **not** in this tree.
- `4851c39aae3a9` ("scsi: pm80xx: Add fatal error checks", May 2023)
  added fatal-error checks to I/O paths in `pm8001_sas.c`, but not to
  sysfs firmware update.
- Standalone 1/1 patch; no series dependency.

### Step 3.4: Author context
**Record:** Kumar Meiyappan and Sagar Biradar are Microchip driver
authors; Martin K. Petersen is the SCSI maintainer who applied the patch
upstream.

### Step 3.5: Dependencies
**Record:** No prerequisites. `controller_fatal_error` field, sysfs
attribute, and fatal-error setting in `pm80xx_hwi.c` all exist in this
tree. Patch applies cleanly (`git apply --check` passed).

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original discussion
**Record:**
- **URL:** https://patch.msgid.link/20260416153757.414896-1-
  sagar.biradar@microchip.com
- **Series:** v1 only (no further revisions).
- **Reviewer feedback:** Martin K. Petersen applied to scsi-
  staging/scsi-queue ("Applied... thanks!").
- **No NAKs** in thread.
- **Stable nomination:** `stable@vger.kernel.org` was CC'd on the
  original submission.

### Step 4.2: Reviewers
**Record:** CC'd: Martin K. Petersen, James Bottomley, Jack Wang, linux-
scsi, stable@vger.kernel.org, Microchip team. Appropriate maintainers
included.

### Step 4.3: Bug report
**Record:** No external bug tracker or syzbot report. Bug identified
internally by driver authors based on invalid-operation behavior after
controller crash.

### Step 4.4: Related patches
**Record:** Companion fix for `pm80xx_get_non_fatal_dump()` is separate
and addresses the same class of bug; not required for this patch to
function.

### Step 4.5: Stable list history
**Record:** Patch was submitted with stable CC; no separate stable-list
discussion found beyond that.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key functions
**Record:** `pm8001_store_update_fw()` (modified); related:
`pm8001_update_flash()`, `pm8001_set_nvmd()`,
`controller_fatal_error_show()`.

### Step 5.2: Callers
**Record:** `pm8001_store_update_fw` is the sysfs store handler for
`DEVICE_ATTR(update_fw, S_IRUGO|S_IWUSR|S_IWGRP, ...)`. Invoked when
root (CAP_SYS_ADMIN) writes to `/sys/class/sas_host/hostN/update_fw`.

### Step 5.3: Callees
**Record:** Without the guard, calls `request_firmware()`, then
`pm8001_update_flash()` → `PM8001_CHIP_DISP->fw_flash_update_req()` →
hardware interaction with a crashed controller.

### Step 5.4: Reachability
**Record:**
- Requires `CONFIG_SCSI_PM8001` and Microchip pm8001/pm80xx hardware.
- Requires `CAP_SYS_ADMIN` (checked at line 803).
- Trigger: controller fatal firmware error **then** admin attempts
  firmware update via sysfs.
- Realistic in production recovery scenarios after a controller crash.

### Step 5.5: Similar patterns
**Record:** Existing guard in `pm8001_sas.c`:

```505:508:drivers/scsi/pm8001/pm8001_sas.c
        if (pm8001_ha->controller_fatal_error) {
                ts->resp = SAS_TASK_UNDELIVERED;
                task->task_done(task);
                return 0;
```

The fix brings the firmware-update sysfs path in line with I/O rejection
behavior.

---

## PHASE 6: CROSS-REFERENCING AGAINST LOCAL TREE

### Step 6.1: Buggy code exists?
**Record:** **Yes.** In 6.18.43, `pm8001_store_update_fw()` at lines
819–837 proceeds without checking `controller_fatal_error`. The flag
infrastructure is fully present (field, sysfs readout, set on fatal
error in `pm80xx_hwi.c`).

### Step 6.2: Backport complications
**Record:** **Clean apply** — verified with `git format-patch` + `git
apply --check`. No conflicts expected.

### Step 6.3: Related fixes already present?
**Record:** **No.** Neither this fix (`2a8fbcfb04aa9`) nor the sibling
non-fatal-dump fix is in `HEAD` or `v6.18`.

---

## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT

### Step 7.1: Subsystem criticality
**Record:** **drivers/scsi/pm8001** — IMPORTANT but hardware-specific
(Microchip/PMC SAS HBAs). Not core kernel, but affects production
storage servers using these controllers.

### Step 7.2: Subsystem activity
**Record:** pm8001 driver is mature; recent fixes target fatal-error
edge cases in sysfs/admin paths.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who is affected
**Record:** Users of `CONFIG_SCSI_PM8001` hardware who experience a
controller fatal error and then attempt firmware update via sysfs.

### Step 8.2: Trigger conditions
**Record:**
- Controller must enter fatal error state (firmware/hardware failure).
- Admin must write to `update_fw` sysfs attribute.
- **Unprivileged users cannot trigger** (CAP_SYS_ADMIN required).
- Not an every-boot path, but a realistic post-failure recovery attempt.

### Step 8.3: Failure mode severity
**Record:** Kernel **call trace** when invalid firmware flash is
attempted on a dead controller. Severity: **HIGH** (kernel instability
on admin operation); not a security issue from unprivileged users, but
prevents further kernel problems during disaster recovery.

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Prevents kernel call trace during admin recovery on
  crashed controllers; aligns sysfs behavior with I/O path and exposed
  `controller_fatal_error` attribute.
- **Risk:** Very low — 8-line guard on an invalid code path.
- **Ratio:** Strong benefit, minimal risk.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence summary

**FOR backport:**
- Fixes a real bug with kernel call trace on a reachable admin path.
- Small, obviously correct, applies cleanly to 6.18.43.
- Uses existing infrastructure; no new APIs or features.
- SCSI maintainer reviewed and applied upstream.
- Stable was CC'd on submission.
- Long-standing omission (field present since at least v6.0); affects
  this stable tree.
- Consistent with existing `controller_fatal_error` checks in I/O path.

**AGAINST backport:**
- Narrow audience (pm8001 hardware only).
- Requires admin + prior controller crash.
- No syzbot or user bug report with stack trace in commit message.

**Unresolved:** Exact call-trace stack not provided in commit message
(author states it "can lead to a call trace" without paste). Mechanism
is plausible from code analysis.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — simple guard; maintainer
   applied upstream.
2. Fixes a real bug? **PASS** — invalid operation on crashed controller.
3. Important issue? **PASS** — kernel call trace / instability (HIGH,
   admin recovery path).
4. Small and contained? **PASS** — 8 lines, 1 file.
5. No new features or APIs? **PASS**.
6. Can apply to local tree? **PASS** — verified clean apply; all
   prerequisites present.

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

### Step 9.4: Decision rationale

For Linux **6.18.y**, this fix addresses a real gap: after a controller
fatal error, the driver already rejects I/O and exposes the error state
via sysfs, but still allows firmware update attempts that can provoke a
kernel call trace. The patch is minimal, self-contained, maintainer-
reviewed, and applies cleanly. The trigger is narrow (admin + hardware
failure), but the consequence (kernel call trace during disaster
recovery) meets stable backport criteria.

---

## Verification

- **[Phase 1]** Parsed commit message and tags from `dc825300273da` /
  user-provided diff.
- **[Phase 2]** Read `pm8001_ctl.c` lines 791–865; analyzed diff (+8
  lines in `pm8001_store_update_fw`).
- **[Phase 3]** `git log all-next --grep="pm8001"` found upstream
  `2a8fbcfb04aa9`; `git merge-base --is-ancestor` confirmed fix NOT in
  `HEAD` or `v6.18`.
- **[Phase 3]** Verified `controller_fatal_error` present in v6.0–v6.18
  via `git show $tag:...pm8001_sas.h`.
- **[Phase 3]** Found sibling fix `3b6992159c00d` / `aa3b8f56ef27e`
  (same pattern, not in tree).
- **[Phase 4]** `b4 dig -c dc825300273da`: lore URL found; `-a`: v1
  only; `-w`: stable@vger.kernel.org CC'd.
- **[Phase 4]** Downloaded mbox; confirmed Martin Petersen applied
  patch, no NAKs.
- **[Phase 5]** `grep` for `update_fw`, `controller_fatal_error`,
  `fw_flash_update_req` call chain.
- **[Phase 5]** Read `pm8001_sas.c:505-508` existing fatal-error guard.
- **[Phase 6]** `git describe HEAD`: v6.18.43; confirmed buggy code at
  lines 819–837 without guard.
- **[Phase 6]** `git format-patch -1 dc825300273da | git apply --check`:
  **clean apply**.
- **[Phase 6]** `git merge-base --is-ancestor dc825300273da HEAD`: fix
  NOT in HEAD.
- **[Phase 8]** Confirmed `CAP_SYS_ADMIN` check at line 803;
  `CONFIG_SCSI_PM8001` in `drivers/scsi/Kconfig`.
- **UNVERIFIED:** Exact kernel stack trace from the failure (not in
  commit message or mbox).

**YES**

 drivers/scsi/pm8001/pm8001_ctl.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
index c10854ec44c7b..a27f3287748ee 100644
--- a/drivers/scsi/pm8001/pm8001_ctl.c
+++ b/drivers/scsi/pm8001/pm8001_ctl.c
@@ -823,6 +823,14 @@ static ssize_t pm8001_store_update_fw(struct device *cdev,
 		goto out;
 	}
 
+	if (pm8001_ha->controller_fatal_error) {
+		pm8001_dbg(pm8001_ha, FAIL,
+			   "controller in fatal error state, firmware update rejected\n");
+		pm8001_ha->fw_status = FAIL_PARAMETERS;
+		ret = -EINVAL;
+		goto out;
+	}
+
 	for (i = 0; flash_command_table[i].code != FLASH_CMD_NONE; i++) {
 		if (!memcmp(flash_command_table[i].command,
 				 cmd_ptr, strlen(cmd_ptr))) {
-- 
2.53.0

Re: [PATCH AUTOSEL 6.18-5.10] scsi: pm8001: Reject firmware update in fatal error state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] TOCTOU race in fw_status check allows concurrent firmware updates, leading to a double free and Use-After-Free (UAF) of pm8001_ha->fw_image.
--

commit 0afa89048afde066ccd69e2cb178118dd117d047
Author: Kumar Meiyappan <kumar.meiyappan@microchip.com>

scsi: pm8001: Reject firmware update in fatal error state

pm8001_store_update_fw() allows a firmware update request even when the
controller has already entered a fatal error state. This commit rejects the
request early by checking controller_fatal_error to avoid a call trace.

Link: https://patch.msgid.link/20260416153757.414896-1-sagar.biradar@microchip.com

> diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
> index cbfda8c04e956..bb38b2d63acb8 100644
> --- a/drivers/scsi/pm8001/pm8001_ctl.c
> +++ b/drivers/scsi/pm8001/pm8001_ctl.c
> @@ -826,6 +826,14 @@ static ssize_t pm8001_store_update_fw(struct device *cdev,
>  		goto out;
>  	}
>  
> +	if (pm8001_ha->controller_fatal_error) {
> +		pm8001_dbg(pm8001_ha, FAIL,
> +			   "controller in fatal error state, firmware update rejected\n");
> +		pm8001_ha->fw_status = FAIL_PARAMETERS;
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +

[Severity: High]
This isn't a bug introduced by this patch, but could this sysfs write handler
be subject to a TOCTOU race condition if multiple threads with CAP_SYS_ADMIN
privileges write to the update_fw sysfs attribute concurrently?

Because kernfs does not serialize concurrent writes to the same sysfs file,
the check earlier in pm8001_store_update_fw():

	if (pm8001_ha->fw_status == FLASH_IN_PROGRESS)
		return -EINPROGRESS;
	pm8001_ha->fw_status = FLASH_IN_PROGRESS;

lacks lock protection. If two threads pass this check simultaneously, they will
both proceed and later call request_firmware() on the same pointer:

	ret = request_firmware(&pm8001_ha->fw_image,
			       filename_ptr,
			       pm8001_ha->dev);

This would leak the first firmware allocation. Furthermore, both threads will
eventually call:

	release_firmware(pm8001_ha->fw_image);

Does this sequence result in a double free of pm8001_ha->fw_image?

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