* [PATCH AUTOSEL 6.18-6.12] pNFS: honor clora_changed when recalling a layout
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:47 ` Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18] NFSv4/flexfiles: report cancelled I/O as a layout error Sasha Levin
` (2 subsequent siblings)
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
To: patches, stable
Cc: Benjamin Coddington, Benjamin Coddington, Trond Myklebust,
Sasha Levin, trondmy, anna, linux-nfs, linux-kernel
From: Benjamin Coddington <ben.coddington@hammerspace.com>
[ Upstream commit aceaa5991bdf744e9c3fa1e5ad73405b1801eaab ]
When the metadata server recalls a layout with clora_changed FALSE, the
layout is not changing and the client may complete its modified writes to
the storage devices before returning the layout (RFC 8881, Section
20.3.3). Only when clora_changed is TRUE -- the server is restriping, or
a storage device has failed -- should the client stop writing to the
storage devices and redirect through the metadata server.
Since commit b739a5bd9d9f ("NFSv4/flexfiles: Cancel I/O if the layout is
recalled or revoked") the client cancels in-flight I/O on every recall,
regardless of clora_changed. For an unchanged recall this abandons
writes whose data may already have reached the storage device; such a
write can then land after the LAYOUTRETURN, which the server sees as a
write without a layout.
Pass the recall's clora_changed value through
pnfs_mark_matching_lsegs_return() and only cancel in-flight I/O when the
layout is actually changing. When it is not, the existing deferred
return path waits for the in-flight writes to drain before sending the
LAYOUTRETURN. Other callers, which are tearing down or returning the
layout for their own reasons, continue to cancel as before.
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111** (`pending-6.12`). The buggy cancel-on-
every-recall path is present here; the fix is not. It is a small
protocol/data-integrity correction and applies cleanly.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[pNFS] [honor] honor `clora_changed` when recalling a layout**
Subsystem: NFS pNFS client. Verb “honor” is a correctness/protocol fix,
not a feature add.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Signed-off-by:** Benjamin Coddington `<bcodding@hammerspace.com>`
(author; Hammerspace)
- **Signed-off-by:** Trond Myklebust `<trond.myklebust@hammerspace.com>`
(NFS client maintainer)
- **No Fixes:** (expected; body names `b739a5bd9d9f`)
- **No Cc: stable** (expected)
- **No Reported-by / Tested-by / Reviewed-by / Link / Acked-by**
Mainline SHA (verified on `origin/master`):
`aceaa5991bdf744e9c3fa1e5ad73405b1801eaab`
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** Since `b739a5bd9d9f`, CB_LAYOUTRECALL always cancels in-
flight DS I/O, including when `clora_changed` is FALSE.
- **RFC 8881 §20.3.3:** FALSE means the layout is not changing; the
client may finish modified writes to storage before LAYOUTRETURN. TRUE
means restriping/device failure; cancel and go through the MDS.
- **Failure mode:** Cancelled writes may already have reached the DS,
then complete after LAYOUTRETURN → server sees a write with no layout.
- **Fix:** Pass `clora_changed` into
`pnfs_mark_matching_lsegs_return()`; cancel only when the layout is
changing. Unchanged recalls use the existing drain-then-return path.
Other callers still cancel.
- **Version info:** Bug since the 2022 cancel-I/O commit (`v6.1-rc1`).
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not a disguised cleanup.** It is an explicit
protocol/correctness fix. The extra `bool cancel_io` on other call sites
preserves existing teardown behavior.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- `fs/nfs/callback_proc.c`: +2 / −1 (`initiate_file_draining`)
- `fs/nfs/pnfs.c`: +13 / −9 (declaration, all callers, cancel gate)
- `fs/nfs/pnfs.h`: +1 / −1 (prototype)
- **Net: +16 / −11**, three files. Surgical, single-subsystem.
Functions: `initiate_file_draining`, `nfs4_layout_refresh_old_stateid`,
`pnfs_mark_layout_stateid_return`, `pnfs_layout_io_set_failed`,
`_pnfs_return_layout`, `pnfs_layout_process`,
`pnfs_mark_matching_lsegs_return`, `pnfs_mark_layout_for_return`,
`pnfs_layout_return_unused_byserver`.
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Recall path (`initiate_file_draining`):** passes
`args->cbl_layoutchanged` instead of always cancelling.
- **`pnfs_mark_matching_lsegs_return`:** `pnfs_lseg_cancel_io()` only if
`cancel_io`.
- **All other callers:** pass `true` → same as today.
- **Unchanged recall with I/O in flight:** still marks lsegs for return,
returns `-EBUSY`, callback still `NFS4_OK`; deferred return waits for
refs to drop (existing `pnfs_put_lseg` / LAYOUTRETURN path).
`nfs_commit_inode()` after unlock is unchanged.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Logic / protocol correctness]** Unconditional I/O cancel on
every recall. That races completed-on-wire writes vs LAYOUTRETURN. The
fix restores RFC drain-before-return when `clora_changed` is false.
Not a leak, UAF, lock, or new API.
**Step 2.4: ASSESS THE FIX QUALITY**
Record: **Obviously correct and minimal.** Only the CB_LAYOUTRECALL file
path changes behavior. `cbl_layoutchanged` is already decoded
(`callback_xdr.c`). Risk: unchanged recalls return the layout more
slowly if DS I/O is stuck (RFC-intended; server may recall again). Tear-
down still cancels. `pnfs_mark_matching_lsegs_invalid()` still always
cancels (revoke). No public API change.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record: Unconditional `pnfs_lseg_cancel_io()` at `fs/nfs/pnfs.c:2678` is
from **`b739a5bd9d9f`** (“NFSv4/flexfiles: Cancel I/O if the layout is
recalled or revoked”), 2022-10-05. `git describe --contains`:
**`v6.1-rc1`**. Present for the whole 6.12 lifetime.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Named commit **`b739a5bd9d9f` is an ancestor of
HEAD** (`git merge-base --is-ancestor` exit 0). That commit added cancel
in both `pnfs_mark_matching_lsegs_return` and
`pnfs_mark_matching_lsegs_invalid`; this patch only gates the return
path.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Recent 6.12 pNFS work (`layout segment leak`, `EBUSY check`,
DRAIN wakeups) does not replace this fix. On `origin/master` this is
**v2 2/3** of “pNFS/flexfiles: honor clora_changed and report cancelled
I/O”:
- 1/3 `ba0f097418c7d` — tracepoint only; **not required**
- 2/3 **this commit**
- 3/3 `4a013b0e881e0` — report cancelled I/O as LAYOUTERROR; **not
required** for this behavior
Standalone for the recall-cancel bug.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Benjamin Coddington is a regular NFS/pNFS contributor
(Hammerspace). Trond applied it. Anna’s v1 comments (kerneldoc,
`FILE_INODE`) were addressed in v2.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Needs existing `cbl_layoutchanged`, `pnfs_lseg_cancel_io()`, and
`cancel_io` on flexfiles — all in this tree. No new helpers. **Applies
standalone.**
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- Lore/msgid: https://patch.msgid.link/05e93230d2f58d9d8b23b083d8d89b21c
b9f5184.1782388900.git.bcodding@hammerspace.com
- **v1 → v2**; applied version is v2.
- Cover: client has decoded `cbl_layoutchanged` but ignored it since
`b739a5bd9d9f`; cancel can leave a DS write incomplete with no write
layout.
- Saved mbox: cover + 1/3 + 2/3 + 3/3 only; **no NAK, no extra review
mail in that match**.
- Trond’s `nfs-for-7.3-1` pull listed it as *“Avoid cancelling in-flight
I/O during a layout recall if the server doesn't require it”* under
**Other bugfixes** (not the Cc:stable subset). That is tagging, not a
NAK.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc Trond Myklebust, Anna Schumaker, `linux-
nfs@vger.kernel.org`. Maintainer SOB on the commit.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot/bugzilla. Vendor/protocol analysis (Hammerspace), not
a crash repro.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: 1/3 observability, 3/3 error reporting when cancel still
happens. Neither is a functional prerequisite. Follow-ups on `pnfs.c`
after this SHA (`2035820177749` EBUSY, `ee5a386cfe60f` lseg leak) are
independent.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** — `lore.kernel.org/stable/?q=clora_changed`
returned 403. No evidence this was rejected for stable.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: KEY FUNCTIONS**
Record: listed in 2.1. Behavioral change is
**`pnfs_mark_matching_lsegs_return` + `initiate_file_draining`**.
**Step 5.2: TRACE CALLERS**
Record:
- File recall: `nfs4_callback_layoutrecall` → `do_callback_layoutrecall`
→ `initiate_file_draining` (MDS CB_LAYOUTRECALL, RETURN_FILE).
- Only flexfiles implements `cancel_io` (`ff_layout_cancel_io` →
`rpc_cancel_tasks` / `rpc_clnt_disconnect`). Other layout types: no-op
today.
- Remaining callers keep `cancel_io=true` (return/teardown/failure/new
stateid).
**Step 5.3: TRACE CALLEES**
Record: `pnfs_match_lseg_recall`, `mark_lseg_invalid`,
`pnfs_lseg_cancel_io` → layout-driver `cancel_io`. Flexfiles cancels
matching RPCs with `-EAGAIN` and disconnects the DS client.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable in production: MDS sends CB_LAYOUTRECALL while the
client has DS writes in flight. Common for pNFS flexfiles (layout recall
on close, space management, etc.). Not a rare debug path. Unprivileged
write traffic plus server recall.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Second cancel site in `pnfs_mark_matching_lsegs_invalid()` is
left unconditional (correct for invalidate/revoke). `cbl_layoutchanged`
is stored but otherwise unused in this tree — this patch is the first
user for I/O policy.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `pending-6.12` / **6.12.111**. `pnfs_lseg_cancel_io()`
is unconditional in `pnfs_mark_matching_lsegs_return()`.
`cbl_layoutchanged` is decoded. `git merge-base --is-ancestor
aceaa5991bdf7 HEAD` → **not an ancestor**. Bug since **v6.1-rc1**.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: `git show aceaa5991bdf7 | git apply --check` **succeeded** (hunk
offsets only). **Clean apply.** All current callers are updated by the
patch.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No `honor clora_changed` commit. No alternate fix. 6.12 already
has later unrelated pNFS fixes (`2ef79f5aa107a` EBUSY, `7c5b0e813efd9`
lseg leak).
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: SUBSYSTEM AND CRITICALITY**
Record: **fs/nfs pNFS client**. **IMPORTANT** for NFSv4.1 pNFS
(especially flexfiles). `PNFS_FLEXFILE_LAYOUT` **defaults to `NFS_V4`**.
Not core mm/VFS, but production clustered NFS.
**Step 7.2: SUBSYSTEM ACTIVITY**
Record: Actively maintained; this is a targeted fix in mature recall
code, not churn in new code.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Record: **Config/driver-specific:** NFSv4.1 pNFS flexfiles clients under
CB_LAYOUTRECALL with in-flight DS writes. Enterprise/scale-out NFS (e.g.
Hammerspace). Filesystems/block/files layouts without `cancel_io` are
unchanged.
**Step 8.2: TRIGGER CONDITIONS**
Record: Server file layout recall with `clora_changed=FALSE` while DS
writes are in flight. Common operational recall, not only device
failure. Userspace writes plus MDS callback; timing-dependent.
**Step 8.3: FAILURE MODE SEVERITY**
Record: **Write-after-LAYOUTRETURN / write with no layout** → protocol
violation, possible rejected/unaccounted DS write, MDS/DS inconsistency.
Cover letter: incomplete DS write with no write layout. **Severity:
HIGH** (data integrity), not a panic.
**Step 8.4: RISK-BENEFIT RATIO**
Record:
- **Benefit: high** for pNFS flexfiles — stops abandoning in-flight
writes on unchanged recalls.
- **Risk: low** — ~16 lines, one gated call, other paths unchanged, RFC-
aligned, maintainer-acked, applies cleanly.
- **Ratio: favorable.**
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real protocol/data-integrity bug in this 6.12 tree
- Introduced v6.1, still present
- Small, obviously correct, RFC 8881 §20.3.3
- Maintainer SOB; described as a bugfix in nfs-for-7.3-1
- Clean apply; no prerequisites
- Flexfiles enabled by default with NFS v4
**AGAINST:**
- No crash/syzbot; pNFS-specific
- Series 2/3 (siblings not required)
- Not in the pull’s “Stable fixes” list (no Cc: stable)
- Unchanged recalls return the layout more slowly if DS I/O is stuck
(intended)
**UNRESOLVED:** Stable-list archives (HTTP 403); no public review
replies beyond v1→v2.
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — RFC behavior, maintainer
SOB, mainline
2. Real bug affecting users? **PASS** — writes after LAYOUTRETURN on
flexfiles
3. Important (crash/security/corruption/deadlock)? **PASS** —
data/protocol integrity (HIGH)
4. Small and contained? **PASS** — +16/−11, three NFS files
5. No new features/APIs? **PASS** — uses existing decoded flag
6. Can apply to this tree? **PASS** — `git apply --check` clean
**Step 9.3: EXCEPTION CATEGORIES**
Record: None (not a device ID/quirk/DT/build/doc-only patch). Qualifies
as a normal important bugfix.
**Step 9.4: DECISION**
Backport to this 6.12 tree. The cancel-always path is here; the fix is
small and restores RFC drain-before-return for unchanged recalls, which
otherwise can complete DS writes after LAYOUTRETURN.
---
### What the commit solves for 6.12 users
On pNFS flexfiles, every layout recall currently cancels in-flight DS
I/O (`ff_layout_cancel_io`). For `clora_changed=FALSE` that is wrong:
those writes may already be on the DS and then finish after
LAYOUTRETURN, which the server treats as I/O without a layout. The patch
only stops cancelling in that recall case and lets the existing deferred
LAYOUTRETURN wait for I/O to drain.
### Risk vs benefit
Benefit is data/protocol correctness for a default NFSv4.1 layout type.
Risk is low: teardown still cancels; only the already-decoded recall
flag changes the file-recall path; the patch applies without rework.
---
### Verification
- **[Phase 1]** Parsed tags: two SOBs (Coddington, Myklebust); no
Fixes/Cc:stable; mainline `aceaa5991bdf7`
- **[Phase 2]** Diff: 16 insertions / 11 deletions; cancel gated; recall
passes `cbl_layoutchanged`; other callers pass `true`
- **[Phase 3]** Blame: cancel line from `b739a5bd9d9f` (`v6.1-rc1`);
ancestor of HEAD
- **[Phase 3]** `git merge-base --is-ancestor aceaa5991bdf7 HEAD` → not
in this tree
- **[Phase 4]** `b4 dig -c aceaa5991bdf7`: v2 2/3, msgid above; v1→v2
(Anna: kerneldoc/FILE_INODE)
- **[Phase 4]** `b4 dig -w`: Trond, Anna, linux-nfs
- **[Phase 4]** `b4 dig -m`: series only, no NAK
- **[Phase 4]** Trond pull nfs-for-7.3-1 lists this under Other bugfixes
- **[Phase 5]** Call chain: CB_LAYOUTRECALL → `initiate_file_draining`;
cancel only implemented in flexfiles
- **[Phase 5]** `cbl_layoutchanged` decoded in `callback_xdr.c`, unused
for I/O policy in this tree
- **[Phase 6]** `git describe HEAD`: `p-6.12-3-gc504cf0baa3d8`; `make
kernelversion`: 6.12.111
- **[Phase 6]** `git apply --check` of `aceaa5991bdf7`: success
- **[Phase 6]** 1/3 and 3/3 not required
- **[Phase 7]** `PNFS_FLEXFILE_LAYOUT` default `NFS_V4`
- **[Phase 8]** Failure: write without layout / incomplete DS write;
HIGH integrity, not panic
- **UNVERIFIED:** lore.kernel.org/stable search (403)
**YES**
fs/nfs/callback_proc.c | 3 ++-
fs/nfs/pnfs.c | 22 +++++++++++++---------
fs/nfs/pnfs.h | 2 +-
3 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 4f41986b875e8..72c25b8864fe5 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -288,7 +288,8 @@ static u32 initiate_file_draining(struct nfs_client *clp,
pnfs_set_layout_stateid(lo, &args->cbl_stateid, NULL, true);
switch (pnfs_mark_matching_lsegs_return(lo, &free_me_list,
&args->cbl_range,
- be32_to_cpu(args->cbl_stateid.seqid))) {
+ be32_to_cpu(args->cbl_stateid.seqid),
+ args->cbl_layoutchanged)) {
case 0:
case -EBUSY:
/* There are layout segments that need to be returned */
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index cf0dcf9e35d75..e2c723e21cb72 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -432,7 +432,8 @@ bool nfs4_layout_refresh_old_stateid(nfs4_stateid *dst,
goto out;
}
/* Try to update the seqid to the most recent */
- err = pnfs_mark_matching_lsegs_return(lo, &head, &range, 0);
+ err = pnfs_mark_matching_lsegs_return(lo, &head, &range, 0,
+ true);
if (err != -EBUSY) {
dst->seqid = lo->plh_stateid.seqid;
*dst_range = range;
@@ -486,7 +487,7 @@ static int pnfs_mark_layout_stateid_return(struct pnfs_layout_hdr *lo,
.length = NFS4_MAX_UINT64,
};
- return pnfs_mark_matching_lsegs_return(lo, lseg_list, &range, seq);
+ return pnfs_mark_matching_lsegs_return(lo, lseg_list, &range, seq, true);
}
static int
@@ -524,7 +525,7 @@ pnfs_layout_io_set_failed(struct pnfs_layout_hdr *lo, u32 iomode)
spin_lock(&inode->i_lock);
pnfs_layout_set_fail_bit(lo, pnfs_iomode_to_fail_bit(iomode));
- pnfs_mark_matching_lsegs_return(lo, &head, &range, 0);
+ pnfs_mark_matching_lsegs_return(lo, &head, &range, 0, true);
spin_unlock(&inode->i_lock);
pnfs_free_lseg_list(&head);
dprintk("%s Setting layout IOMODE_%s fail bit\n", __func__,
@@ -1461,7 +1462,7 @@ _pnfs_return_layout(struct inode *ino)
}
valid_layout = pnfs_layout_is_valid(lo);
pnfs_clear_layoutcommit(ino, &tmp_list);
- pnfs_mark_matching_lsegs_return(lo, &tmp_list, &range, 0);
+ pnfs_mark_matching_lsegs_return(lo, &tmp_list, &range, 0, true);
/* Don't send a LAYOUTRETURN if list was initially empty */
@@ -2603,7 +2604,7 @@ pnfs_layout_process(struct nfs4_layoutget *lgp)
.iomode = IOMODE_ANY,
.length = NFS4_MAX_UINT64,
};
- pnfs_mark_matching_lsegs_return(lo, &free_me, &range, 0);
+ pnfs_mark_matching_lsegs_return(lo, &free_me, &range, 0, true);
goto out_forget;
} else {
/* We have a completely new layout */
@@ -2635,6 +2636,7 @@ pnfs_layout_process(struct nfs4_layoutget *lgp)
* @tmp_list: list header to be used with pnfs_free_lseg_list()
* @return_range: describe layout segment ranges to be returned
* @seq: stateid seqid to match
+ * @cancel_io: signal io be cancelled
*
* This function is mainly intended for use by layoutrecall. It attempts
* to free the layout segment immediately, or else to mark it for return
@@ -2649,7 +2651,7 @@ int
pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
struct list_head *tmp_list,
const struct pnfs_layout_range *return_range,
- u32 seq)
+ u32 seq, bool cancel_io)
{
struct pnfs_layout_segment *lseg, *next;
struct nfs_server *server = NFS_SERVER(lo->plh_inode);
@@ -2675,7 +2677,8 @@ pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
continue;
remaining++;
set_bit(NFS_LSEG_LAYOUTRETURN, &lseg->pls_flags);
- pnfs_lseg_cancel_io(server, lseg);
+ if (cancel_io)
+ pnfs_lseg_cancel_io(server, lseg);
}
if (remaining) {
@@ -2710,7 +2713,8 @@ pnfs_mark_layout_for_return(struct inode *inode,
* segments at hand when sending layoutreturn. See pnfs_put_lseg()
* for how it works.
*/
- if (pnfs_mark_matching_lsegs_return(lo, &lo->plh_return_segs, range, 0) != -EBUSY) {
+ if (pnfs_mark_matching_lsegs_return(lo, &lo->plh_return_segs, range, 0,
+ true) != -EBUSY) {
const struct cred *cred;
nfs4_stateid stateid;
enum pnfs_iomode iomode;
@@ -2825,7 +2829,7 @@ static int pnfs_layout_return_unused_byserver(struct nfs_server *server,
pnfs_get_layout_hdr(lo);
pnfs_set_plh_return_info(lo, range->iomode, 0);
if (pnfs_mark_matching_lsegs_return(lo, &lo->plh_return_segs,
- range, 0) != 0 ||
+ range, 0, true) != 0 ||
!pnfs_prepare_layoutreturn(lo, &stateid, &cred, &iomode)) {
spin_unlock(&inode->i_lock);
rcu_read_unlock();
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 7b3ba2e64e6f3..14d2a91250f24 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -301,7 +301,7 @@ int pnfs_mark_matching_lsegs_invalid(struct pnfs_layout_hdr *lo,
int pnfs_mark_matching_lsegs_return(struct pnfs_layout_hdr *lo,
struct list_head *tmp_list,
const struct pnfs_layout_range *recall_range,
- u32 seq);
+ u32 seq, bool cancel_io);
int pnfs_mark_layout_stateid_invalid(struct pnfs_layout_hdr *lo,
struct list_head *lseg_list);
bool pnfs_roc(struct inode *ino, struct nfs4_layoutreturn_args *args,
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.18] NFSv4/flexfiles: report cancelled I/O as a layout error
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.12] pNFS: honor clora_changed when recalling a layout Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.12] NFS: Verify symlink inode before caching target Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] NFSv4: pin the superblock for active state owners Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: Benjamin Coddington, Benjamin Coddington, Trond Myklebust,
Sasha Levin, trondmy, anna, linux-nfs, linux-kernel
From: Benjamin Coddington <ben.coddington@hammerspace.com>
[ Upstream commit 4a013b0e881e0466cc2c6a5518d2f6cda3a4f19f ]
When a layout is recalled or revoked the client cancels its in-flight I/O
so the layout can be returned. The metadata server needs to learn that
this I/O to the storage device did not complete, so that it can reconcile
the affected mirror instance (or, if none remains, take other action).
The cancellation completed with -EAGAIN, which ff_layout_io_track_ds_error()
does not recognise: it fell through the switch and recorded nothing, so no
error was reported to the server.
-EAGAIN is overloaded in the RPC layer, so rather than key the reporting on
it, cancel the I/O with -ECANCELED and map that to NFS4ERR_NXIO in
ff_layout_io_track_ds_error() -- the status the client already reports for
the transport errors that leave an in-flight write incomplete. The
cancelled I/O is then reported to the server via LAYOUTERROR / LAYOUTRETURN.
Unlike a genuine transport error, though, we aborted the I/O ourselves and
have no evidence the device is at fault, so once the error is recorded we
skip marking the device unreachable and forcing a further layout return.
The retry disposition is unchanged from the original -EAGAIN cancellation:
both NFS4ERR_NXIO and -ECANCELED are no-ops in ff_layout_async_handle_error(),
which still resets the I/O to pNFS (or the MDS), so it is re-driven as before.
Signed-off-by: Benjamin Coddington <bcodding@hammerspace.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111** (`pending-6.12`). The buggy cancel path
is present here; the candidate is not.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[NFSv4/flexfiles] [report] report cancelled I/O as a layout
error`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- Signed-off-by: Benjamin Coddington `<bcodding@hammerspace.com>`
(author)
- Signed-off-by: Trond Myklebust `<trond.myklebust@hammerspace.com>`
(NFS maintainer)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, Link:,
or Cc: stable (absence expected)
- Author in git: Benjamin Coddington `<ben.coddington@hammerspace.com>`
- Upstream SHA (on `all-next`, not in this tree):
`4a013b0e881e0466cc2c6a5518d2f6cda3a4f19f` (2026-06-25)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- Bug: on layout recall/revoke the client cancels in-flight DS I/O so
the layout can be returned. Cancellation used `-EAGAIN`, which
`ff_layout_io_track_ds_error()` does not recognize, so it records
nothing and the MDS never learns the I/O did not complete.
- Symptom: MDS cannot reconcile the affected flexfiles mirror (or take
other action if no instance remains). Incomplete writes can sit on a
DS with no write layout held.
- Root cause: `-EAGAIN` is overloaded in the RPC layer and is not in the
tracked-error switch (default: return).
- Fix: cancel with `-ECANCELED`, map that to `NFS4ERR_NXIO` (same status
already used for transport errors that leave a write incomplete),
report via LAYOUTERROR / LAYOUTRETURN, but skip marking the device
unreachable and skip forcing another layout return.
- Retry path: unchanged; both `NFS4ERR_NXIO` and `-ECANCELED` fall
through `ff_layout_async_handle_error()` to RESET_TO_PNFS /
RESET_TO_MDS.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not disguised. This is an explicit protocol/correctness fix:
cancelled I/O was silently dropped instead of being reported to the MDS.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `fs/nfs/flexfilelayout/flexfilelayout.c` (+22 / -1)
- Functions: `ff_layout_io_track_ds_error()`, `ff_layout_cancel_io()`
- Scope: single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- Hunk 1 (switch): `-ECANCELED` now maps to `NFS4ERR_NXIO` and continues
into `ff_layout_track_ds_error()` instead of hitting `default:
return`.
- Hunk 2: after recording the error, `if (error == -ECANCELED) goto
out;` skips `ff_layout_mark_ds_unreachable()` and
`pnfs_error_mark_layout_for_return()`.
- Hunk 3: `rpc_cancel_tasks(clnt, -EAGAIN, ...)` becomes
`rpc_cancel_tasks(clnt, -ECANCELED, ...)`.
- Prepare paths (`ff_layout_*_prepare_common`) still `rpc_exit(task,
-EAGAIN)` for invalid lsegs. That is I/O that was never sent; it is
correctly left alone.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- Category: logic / protocol correctness (missing error reporting), with
data-integrity impact on flexfiles mirrors.
- Mechanism: cancelled in-flight RPC completed with an unrecognized
errno, so no `nfs4_ff_layout_ds_err` was queued and LAYOUTERROR /
LAYOUTRETURN carried no I/O error.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct: use a dedicated errno, record it as the same NXIO
the client already reports for incomplete writes, and do not condemn
the device for a client-initiated abort.
- Minimal; no API or structural change.
- Regression risk: low. `nfs_pgio_result()` treats any non-zero
`rw_done()` return the same, so `-ECANCELED` vs `-EAGAIN` does not
change the resend path. Verified in
`ff_layout_async_handle_error_v4()`: `NFS4ERR_NXIO` is a no-op in the
op_status switch; `-ECANCELED` is not a connection error, then
RESET_TO_PNFS/MDS. v3 already deleted the deviceid on `-EAGAIN`
(default) and still does so on `-ECANCELED`.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Error switch introduced in `dd52128afdde` (Trond, 2015-08-20) — never
handled `-EAGAIN`.
- `ff_layout_cancel_io()` and the `-EAGAIN` cancel introduced in
`b739a5bd9d9f18` ("NFSv4/flexfiles: Cancel I/O if the layout is
recalled or revoked", 2022-10-05, first in v6.1-rc1).
- That cancel commit is an ancestor of HEAD.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No Fixes: tag. The introducing commit is `b739a5bd9d9f18`,
present in this tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Same pattern already in this tree: `1630224189cc4` (ENETDOWN),
`dd7d7ee3ba2a7` (EINVAL), `7836d75467e9d` (RDMA) — all add cases to
this switch so the MDS is told.
- This commit is patch 3/3 of "pNFS/flexfiles: honor clora_changed and
report cancelled I/O".
- Patch 2 (`aceaa5991bdf7` "honor clora_changed when recalling a
layout") is **not** in this tree. It only changes *when* cancel runs
(skip cancel if `clora_changed` is false). This commit does not call
any new helper from patch 2.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Benjamin Coddington (Hammerspace) is a regular NFS/pNFS
contributor. Trond (NFS maintainer, also Hammerspace) applied it.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- Semantic fix is standalone: change the cancel errno and handle it in
the existing switch.
- Clean apply is **not** standalone against this tree. Mainline added
`dss_id` via later striped-layout commits (`8a8e0f5566edc`,
`a1491919c8805`, `20b1d75fb840c`, …). Those are **not** in HEAD. This
tree only has `d1830d0740d72` (data-structure support) and still uses
`mirror->dss[0]` / no `dss_id` argument.
- `git apply --check` of the mainline diff: hunk 1 applied (offset
-177); hunk 2 failed on the `dss_id` argument. A 6.12 backport is a
small context edit, not a rewrite.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 4a013b0e881e0` → https://patch.msgid.link/b5ed2050a2fa573ad
9a2bcf7769f23397e480f2a.1782388900.git.bcodding@hammerspace.com
- Series: v1 (2026-06-24) then v2 (2026-06-25). Applied version is v2
3/3.
- Cover letter: RFC 8881 §20.3.3; since `b739a5bd9d9f` the client
cancels DS I/O on every recall; when it *does* cancel (changed recall
or revoke), report `NFS4ERR_NXIO` so the MDS can reconcile.
- v2 changes were only for patches 1–2 (Anna: kerneldoc, `FILE_INODE` /
`inode->i_ino`). Patch 3 was unchanged.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record:
- Recipients: Trond Myklebust, Anna Schumaker, linux-nfs@vger.kernel.org
- Anna reviewed v1 1/3 and 2/3 only (tracepoint inode id; kerneldoc for
`cancel_io`). No NAK. No comments on 3/3.
- Trond applied the series (Signed-off-by).
- No reviewer Cc: stable.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot, bugzilla, or external user report. The description is
a vendor/protocol failure (Hammerspace flexfiles MDS cannot reconcile
after cancel). WebFetch to lore returned 403; discussion was recovered
via `b4 dig -m`.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record:
- 1/3: tracepoint only — not required for this fix.
- 2/3: stop cancelling on unchanged recalls — complementary RFC
behavior, not a compile/runtime dependency of 3/3. Without it, 6.12
still cancels on every recall, so this fix would send NXIO on more
recalls than post-series mainline. That is more conservative for the
MDS (reconcile), not a crash path.
- Other cancel callers (`pnfs_mark_matching_lsegs_invalid`,
teardown/return with cancel) remain even after 2/3.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: No stable-list discussion found (lore.kernel.org/stable search
via WebFetch was 403). UNVERIFIED: whether anyone already requested or
rejected this for stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `ff_layout_io_track_ds_error()`, `ff_layout_cancel_io()`.
**Step 5.2: TRACE CALLERS**
Record:
- `ff_layout_io_track_ds_error()`: `ff_layout_read_done_cb()`,
`ff_layout_write_done_cb()`, `ff_layout_commit_done_cb()` — DS I/O
completion.
- `ff_layout_cancel_io()`: registered as
`pnfs_layoutdriver_type.cancel_io`.
- `pnfs_lseg_cancel_io()` → `pnfs_mark_matching_lsegs_invalid()` (line
744) and `pnfs_mark_matching_lsegs_return()` (line 2678).
- Those run on CB_LAYOUTRECALL, layout return, revoke, and teardown.
**Step 5.3: TRACE CALLEES**
Record: `ff_layout_track_ds_error()` queues `nfs4_ff_layout_ds_err`;
`ff_layout_send_layouterror()` / `ff_layout_encode_ioerr()` send
LAYOUTERROR / LAYOUTRETURN. `rpc_cancel_tasks()` →
`rpc_task_try_cancel()`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable from normal pNFS flexfiles file I/O plus MDS
CB_LAYOUTRECALL. Not a raw syscall crash path; it is a common path for
any flexfiles mount under layout recall/return.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same switch already maps transport failures to `NFS4ERR_NXIO`.
This is the same class of fix, for a client-initiated cancel that
previously fell on the floor.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes.
- Tree: `p-6.12-3-gc504cf0baa3d8`, `VERSION=6 PATCHLEVEL=12
SUBLEVEL=111`.
- `git merge-base --is-ancestor 4a013b0e881e0 HEAD` → not an ancestor
(fix not present).
- `ff_layout_cancel_io()` still uses `-EAGAIN` (line 2196).
- `ff_layout_io_track_ds_error()` has no `-ECANCELED` case; default
returns without recording (lines 1369–1370).
- Bug introduced in v6.1 (`b739a5bd9d9f18`); present for the whole 6.12
lifetime.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Minor conflicts. Need to drop `dss_id` from
`ff_layout_track_ds_error()` / `ff_layout_mark_ds_unreachable()` and
keep the single `dss[0]` cancel loop. Not a semantic rewrite.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: No equivalent cancel-reporting fix. Related NXIO mappings
(ENETDOWN, EINVAL, EPROTO/ENODEV) are already here and do not cover
cancel.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: `fs/nfs/flexfilelayout` — NFSv4.1 pNFS flexfiles client.
**IMPORTANT** (not core VFS; production for flexfiles MDS deployments).
`CONFIG_PNFS_FLEXFILE_LAYOUT` defaults to `NFS_V4`.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Active. Recent 6.12 backports include NO_IO_THRU_MDS, zero-FH
rejection, NFS-level error handling, ENETDOWN reporting.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Flexfiles pNFS clients (`CONFIG_PNFS_FLEXFILE_LAYOUT`). Not
universal; real production users (Hammerspace and other flexfiles MDS).
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Layout recall, revoke, or return while DS read/write/commit RPCs
are in flight. Common on flexfiles (recall on close, lease recovery,
restriping). Not an unprivileged local exploit; it is a networked MDS
callback plus in-flight I/O.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: MDS is not told that in-flight writes did not complete, so it
may not reconcile a mirror. Later reads can see a stale or incomplete
replica. **HIGH** (data integrity / stale data), not a kernel oops.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- Benefit: HIGH for flexfiles — MDS can reconcile after cancel/revoke.
- Risk: LOW — +22 lines, contained, retry path unchanged. Extra NXIO
reports on 6.12’s still-unconditional cancel are conservative, not a
hang/crash.
- Ratio: benefit outweighs risk.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real bug: cancelled DS I/O is invisible to the MDS.
- Data-integrity impact on flexfiles mirrors.
- Code is in this 6.12 tree since v6.1.
- Small, obviously correct, maintainer-applied.
- Same reporting pattern already backported here (ENETDOWN, EINVAL,
RDMA).
- Retry disposition verified unchanged.
AGAINST:
- Patch 3/3 of a series; 2/3 (narrower cancel) is not here.
- Mainline diff needs a small `dss_id`-free backport.
- No crash/UAF/CVE; flexfiles-specific.
- No Fixes:/Cc: stable/user report (expected for this queue).
UNRESOLVED:
- Stable-list history (lore/stable WebFetch 403).
- How every flexfiles MDS implementation reacts to extra NXIO on
unchanged recalls (without patch 2). Conservative, not a kernel
regression.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — mapped like existing
transport errors; applied by the NFS maintainer.
2. Fixes a real bug that affects users? **PASS** — MDS cannot reconcile
after cancel.
3. Important issue? **PASS** — data integrity / stale flexfiles mirrors
(HIGH, not oops).
4. Small and contained? **PASS** — +22/-1, one file.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — needs a trivial context
backport (no `dss_id`); buggy code is here.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: None (not a device ID, quirk, DT, build, or docs fix). Qualifies
as a normal important bugfix.
**Step 9.4: MAKE YOUR DECISION**
This should be backported to this 6.12 tree. The cancel path exists, the
MDS is not told, and the fix is small. Adapt the hunks to the `dss[0]` /
no-`dss_id` signatures; do not pull the striped-layout series.
What it solves for stable users: after layout recall/revoke, in-flight
flexfiles I/O is cancelled so the layout can return, but 6.12 records
nothing. The MDS then cannot reconcile the mirror. Mapping cancel to
`NFS4ERR_NXIO` and sending LAYOUTERROR / LAYOUTRETURN closes that hole
without marking the device dead or changing retry.
Risk vs benefit: low risk, high benefit on flexfiles. Companion patch 2
(honor `clora_changed`) is a separate RFC behavior change and is not
required for this reporting fix to work.
Verification:
- [Phase 1] Tags: only author + Trond SOBs; no Fixes/Reported-by/Cc
stable
- [Phase 1] Upstream SHA `4a013b0e881e0` on all-next; `git merge-base
--is-ancestor` → not in HEAD
- [Phase 2] Diff: +22/-1 in one file; `-EAGAIN` → `-ECANCELED`; new NXIO
case; skip unreachable/return
- [Phase 2] `NFS4ERR_NXIO` and `NFSERR_NXIO` both equal 6
- [Phase 2] `nfs_pgio_result()`: non-zero `rw_done()` skips
`nfs_set_pgio_error` for both errnos
- [Phase 2] v4/v3 async handlers: retry disposition matches the commit
text
- [Phase 3] Blame: switch from `dd52128afdde` (2015); cancel from
`b739a5bd9d9f18` (v6.1), ancestor of HEAD
- [Phase 3] Similar backports already here: `1630224189cc4`,
`dd7d7ee3ba2a7`, `7836d75467e9d`
- [Phase 3] Patch 2 `aceaa5991bdf7` is not an ancestor of HEAD
- [Phase 4] `b4 dig -c 4a013b0e881e0`: v2 3/3 at the msgid.link above
- [Phase 4] `b4 dig -a`: v1 then v2; 3/3 unchanged
- [Phase 4] `b4 dig -w`: Trond, Anna, linux-nfs
- [Phase 4] Anna v1 review: nits on 1/3 and 2/3 only; no NAK of 3/3
- [Phase 4] Cover letter recovered from `/tmp/ff-cancel-thread.mbox`
- [Phase 5] Callers: read/write/commit done_cb; cancel via
`pnfs_lseg_cancel_io` from recall/return/invalidate
- [Phase 6] `git describe HEAD` → `p-6.12-3-gc504cf0baa3d8`; `make
kernelversion` → 6.12.111
- [Phase 6] Local cancel still `-EAGAIN` at flexfilelayout.c:2196; no
`-ECANCELED` in `fs/nfs`
- [Phase 6] `git apply --check` failed on `dss_id` hunk; striped error-
path commits not in HEAD
- [Phase 8] Failure mode: unreported incomplete DS I/O →
unreconciliation / stale mirror, HIGH
- UNVERIFIED: stable@ discussion (lore 403)
**YES**
fs/nfs/flexfilelayout/flexfilelayout.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 8099bfc4fe2f8..c708724bb53f2 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1546,6 +1546,17 @@ static void ff_layout_io_track_ds_error(struct pnfs_layout_segment *lseg,
case -EACCES:
*op_status = status = NFS4ERR_ACCESS;
break;
+ case -ECANCELED:
+ /*
+ * In-flight I/O we cancelled to return a recalled or
+ * revoked layout. Report it as a failure to reach the
+ * device (NFS4ERR_NXIO), like the transport errors
+ * above, so the server can reconcile the affected mirror
+ * instance. We aborted the I/O ourselves rather than
+ * observe the device fail, so don't condemn it below.
+ */
+ *op_status = status = NFS4ERR_NXIO;
+ break;
default:
return;
}
@@ -1556,6 +1567,15 @@ static void ff_layout_io_track_ds_error(struct pnfs_layout_segment *lseg,
mirror, dss_id, offset, length, status, opnum,
nfs_io_gfp_mask());
+ /*
+ * I/O we cancelled ourselves to return a recalled or revoked layout
+ * is reported above so the server can reconcile the mirror, but we
+ * have no evidence the device is at fault: don't mark it unreachable
+ * or force a return.
+ */
+ if (error == -ECANCELED)
+ goto out;
+
switch (status) {
case NFS4ERR_DELAY:
case NFS4ERR_GRACE:
@@ -1575,6 +1595,7 @@ static void ff_layout_io_track_ds_error(struct pnfs_layout_segment *lseg,
lseg);
}
+out:
dprintk("%s: err %d op %d status %u\n", __func__, err, opnum, status);
}
@@ -2465,7 +2486,7 @@ static void ff_layout_cancel_io(struct pnfs_layout_segment *lseg)
clnt = ds_clp->cl_rpcclient;
if (!clnt)
continue;
- if (!rpc_cancel_tasks(clnt, -EAGAIN,
+ if (!rpc_cancel_tasks(clnt, -ECANCELED,
ff_layout_match_io, lseg))
continue;
rpc_clnt_disconnect(clnt);
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.18-6.12] NFS: Verify symlink inode before caching target
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.12] pNFS: honor clora_changed when recalling a layout Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18] NFSv4/flexfiles: report cancelled I/O as a layout error Sasha Levin
@ 2026-10-19 13:48 ` Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] NFSv4: pin the superblock for active state owners Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-19 13:48 UTC (permalink / raw)
To: patches, stable
Cc: ZhangGuoDong, Jackie Liu, Trond Myklebust, Sasha Levin, trondmy,
anna, linux-nfs, linux-kernel
From: ZhangGuoDong <zhangguodong@kylinos.cn>
[ Upstream commit 3265f1998ae9a9282a8a6ca95467d9572d6ebb82 ]
nfs_symlink() copies the symlink target into a folio before issuing the
SYMLINK RPC. After a successful reply, it caches that folio in the
instantiated inode mapping and assumes that the dentry now names a
symlink.
If the dentry is instantiated with a non-symlink inode, the raw symlink
target folio can be inserted into the wrong mapping. When that inode is
a directory, reclaim or unmount later calls nfs_readdir_clear_array()
through nfs_dir_aops and interprets the symlink target as a readdir
cache array, which can lead to invalid kfree() calls.
A vmcore from a 4.19-based kernel showed the crash when reclaiming a
directory mapping on unmount:
Stack trace:
nfs_readdir_clear_array+0x4d/0x70 [nfs]
page_cache_free_page.isra.35+0x1a/0x90
delete_from_page_cache_batch+0x1cf/0x2c0
truncate_inode_pages_range+0x24d/0x910
[...]
nfs_evict_inode+0x15/0x30 [nfs]
evict+0x115/0x2b0
dispose_list+0x48/0x60
evict_inodes+0x16c/0x1b0
generic_shutdown_super+0x3f/0x120
nfs_kill_super+0x1b/0x40 [nfs]
deactivate_locked_super+0x3f/0x70
cleanup_mnt+0x3b/0x80
The current code still has the same unchecked cache insertion pattern,
so it may be susceptible to the same failure mode.
Verify that the instantiated inode is a symlink before caching the
target folio. If the type is wrong, drop the suspect dentry and skip
the cache insertion while preserving the successful SYMLINK result.
Co-developed-by: Jackie Liu <liuyun01@kylinos.cn>
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.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: **[NFS]** **[Verify]** Verify the instantiated inode is a
symlink before inserting the SYMLINK target folio into the page cache.
**Step 1.2: Tags**
Record:
- Co-developed-by: Jackie Liu \<liuyun01@kylinos.cn\>
- Signed-off-by: Jackie Liu \<liuyun01@kylinos.cn\>
- Signed-off-by: ZhangGuoDong \<zhangguodong@kylinos.cn\>
- Signed-off-by: Trond Myklebust \<trond.myklebust@hammerspace.com\>
(NFS client maintainer)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, Link:,
or Cc: stable (absence is expected for this review)
- Mainline SHA: `3265f1998ae9a9282a8a6ca95467d9572d6ebb82` (authored
2026-07-03, committed by Trond 2026-08-17, merged to Linus in `nfs-
for-7.3-1`)
**Step 1.3: Body**
Record: After a successful SYMLINK RPC, `nfs_symlink()` inserts the pre-
copied target folio into `d_inode(dentry)->i_mapping` without checking
that the dentry is actually a symlink. If that inode is a directory,
later reclaim/unmount runs `nfs_readdir_clear_array()` via
`nfs_dir_aops.free_folio` and treats the symlink text as a readdir cache
array, leading to invalid `kfree()`. A 4.19 vmcore showed exactly that
stack on unmount. The author states the same unchecked insertion still
exists. Failure mode: invalid free / memory corruption / crash during
unmount or mapping reclaim.
**Step 1.4: Hidden bug fix?**
Record: Not hidden. This is an explicit crash/corruption fix. The
“verify before caching” wording is a missing type check, not cleanup.
## Phase 2: Diff analysis
**Step 2.1: Inventory**
Record: One file, `fs/nfs/dir.c`. **+6 / −0**. Function:
`nfs_symlink()`. Single-file surgical fix.
**Step 2.2: Control-flow change**
Record: After `NFS_PROTO(dir)->symlink()` returns 0, the old code always
set the dentry verifier and tried
`filemap_add_folio(d_inode(dentry)->i_mapping, folio, 0, …)`. After the
change, `unlikely(!d_is_symlink(dentry))` causes `d_drop(dentry)`,
`folio_put(folio)`, and `return 0` (RPC still succeeded). The success
path is unchanged for real symlinks.
**Step 2.3: Bug mechanism**
Record: **Memory safety / type confusion.** A folio of symlink text is
inserted into a directory mapping whose `a_ops->free_folio` is
`nfs_readdir_clear_array()`. That function maps the folio as `struct
nfs_cache_array` and `kfree()`s `array[i].name` for `i < array->size`.
Path-string bytes as `size`/pointers yield invalid `kfree()`. Also
avoids a possible NULL deref if `d_inode(dentry)` is NULL (negative
dentry after splice).
**Step 2.4: Fix quality**
Record: Obviously correct and minimal. `d_is_symlink()` is the right VFS
helper. Skipping cache insert is already treated as non-fatal (“READLINK
will get the missing page from the server”). `d_drop()` avoids leaving a
wrong-type dentry hashed with a verifier. Regression risk is low: the
common success path is unchanged; the new path only runs on a type
mismatch.
## Phase 3: Git history
**Step 3.1: Blame**
Record: Unchecked cache insertion comes from `873101b337767` (“NFS: copy
symlinks into page cache before sending NFS SYMLINK request”,
2006-09-22). That commit is an ancestor of this tree (`git merge-base
--is-ancestor` exit 0). Folio conversion is `f003a717ae908` (2023); same
pattern, `add_to_page_cache` → `filemap_add_folio`. Directory
`free_folio = nfs_readdir_clear_array` is present at
`fs/nfs/dir.c:71-73`.
**Step 3.2: Fixes: tag**
Record: None. Introducer is `873101b337767`, present here since 2.6-era
NFS.
**Step 3.3: File history**
Record: Recent `fs/nfs/dir.c` work is readdir/verifier/`atomic_open`
(including `df214a74b5f2a` ENXIO on O_CREAT of an existing symlink).
Those are related NFS type-confusion issues but not prerequisites. This
patch is standalone.
**Step 3.4: Author**
Record: ZhangGuoDong / Jackie Liu (Kylin). Not subsystem maintainers.
Trond (maintainer) signed off and put it in `nfs-for-7.3-1`. Same author
also has `pnfs/blocklayout` leak fixes in this tree.
**Step 3.5: Dependencies**
Record: Needs `d_is_symlink()` (in `include/linux/dcache.h` here) and
folio-based `nfs_symlink()` (present via `f003a717ae908`). No other
commits required. Insertion context around `nfs_set_verifier()` is
unique in this file.
## Phase 4: Mailing list / external
**Step 4.1: Original submission**
Record: `b4 dig -c 3265f1998ae9a9282a8a6ca95467d9572d6ebb82` →
https://patch.msgid.link/20260703065448.210255-1-zhang.guodong@linux.dev
v1 only (2026-07-03). Saved mbox has the patch and **no replies**. Trond
applied it as-is (same +6 hunk). Lore HTML was 403 (Anubis); discussion
content was recovered via b4.
**Step 4.2: Recipients (`b4 dig -w`)**
Record: Trond Myklebust, Anna Schumaker, linux-nfs@vger.kernel.org,
ZhangGuoDong, Jackie Liu. Right maintainers/list.
**Step 4.3: Bug report**
Record: No public bugzilla/syzbot link. Evidence is a 4.19 vmcore stack
in the commit message (unmount → `nfs_readdir_clear_array`).
**Step 4.4: Series**
Record: Standalone `[PATCH]`, not `n/m`. Same 7.3 NFS pull also has
NFSv3 symlink/`atomic_open` fixes; those are separate.
**Step 4.5: Stable list**
Record: Lore stable search 403. Trond’s 7.3 pull listed this under
**“Other bugfixes”**, not “Stable fixes”. That is categorization, not a
NAK. No stable-thread rejection found.
## Phase 5: Semantic / call graph
**Step 5.1: Functions**
Record: `nfs_symlink()` only.
**Step 5.2: Callers**
Record: `nfs_dir_inode_operations.symlink`,
`nfs3_dir_inode_operations.symlink`, `nfs4_dir_inode_operations.symlink`
all point at `nfs_symlink`. `vfs_symlink()` → `dir->i_op->symlink()`
from `do_symlinkat` / `symlink(2)`.
**Step 5.3: Callees**
Record: `NFS_PROTO(dir)->symlink` (`nfs_proc_symlink` /
`nfs3_proc_symlink` / `nfs4_proc_symlink`) instantiate via
`nfs_instantiate` / `nfs_add_or_obtain` → `nfs_fhget` +
`d_splice_alias`. Then `filemap_add_folio`. Directory inodes get
`i_data.a_ops = &nfs_dir_aops` in `fs/nfs/inode.c` (`S_ISDIR` branch).
**Step 5.4: Reachability**
Record: Yes — unprivileged `symlink(2)` on an NFS mount. Crash is later
(reclaim/unmount), not at syscall return.
**Step 5.5: Similar patterns**
Record: Same class as `nfs_atomic_open_v23()` not checking `d_is_reg()`
after CREATE (fixed in this tree by `df214a74b5f2a`). NFS create paths
can attach a different type than the client assumed.
## Phase 6: This tree (pending-6.12 / 6.12.111)
**Step 6.1: Buggy code present?**
Record: **Yes.** `nfs_symlink()` at `fs/nfs/dir.c:2612-2665` has the
unchecked `filemap_add_folio(d_inode(dentry)->i_mapping, …)` after RPC
success. `nfs_dir_aops.free_folio = nfs_readdir_clear_array` still
`kfree`s names from folio contents. `3265f1998ae9` is **not** an
ancestor of HEAD.
**Step 6.2: Backport difficulty**
Record: **Clean apply.** Hunk context is the `return error;` /
`nfs_set_verifier()` sequence, identical here. Later mainline `%llu` for
`i_ino` is outside this hunk.
**Step 6.3: Related fixes already here?**
Record: No `d_is_symlink()` guard in `nfs_symlink()`. `df214a74b5f2a` is
a different type-check (open/CREATE), not this cache-insert bug.
## Phase 7: Subsystem
**Step 7.1:** NFS client (`fs/nfs/`). **IMPORTANT** — common networked
filesystem; not core mm/VFS, but widely deployed
(enterprise/Android/embedded).
**Step 7.2:** Actively maintained; this is a long-standing pattern, not
a brand-new subsystem.
## Phase 8: Impact and risk
**Step 8.1:** NFS client users who create symlinks (`symlink(2)`). All
of NFSv2/v3/v4 client.
**Step 8.2:** Trigger is **not** every `symlink()`. It needs the post-
RPC dentry to not be a symlink: NFSv2 LOOKUP-after-SYMLINK race, missing
fh/attrs then LOOKUP, server wrong type, or fh alias onto a directory
inode. Unprivileged users can call `symlink(2)`; the crash is delayed to
reclaim/unmount.
**Step 8.3:** Failure mode: invalid `kfree` / memory corruption / oops
on unmount. **CRITICAL**.
**Step 8.4:** Benefit **high** (prevents unmount crash/corruption). Risk
**very low** (6 lines, success path unchanged, cache skip already
defined as optional). Ratio strongly favors backport.
## Phase 9: Synthesis
**For:**
- Real crash with vmcore stack (`nfs_readdir_clear_array` on unmount).
- Same insertion vs directory `free_folio` pattern is in this 6.12 tree.
- Bug dates to 2006 (`873101b337767`).
- 6-line type check; NFS maintainer signed off.
- Userspace-reachable via `symlink(2)`.
- Also avoids NULL `d_inode` if instantiation left a non-
symlink/negative dentry.
- Applies cleanly; no new APIs.
**Against:**
- Trigger is uncommon (race/server/fh mismatch), not every symlink.
- No Fixes:/Cc: stable (expected; not a NO reason).
- Trond grouped it as “other bugfixes” in the 7.3 pull, not “stable
fixes”.
- Mailing list had no public review comments (silent apply).
- Crash evidence is 4.19; current kernels are folio-based but the type-
confusion is unchanged.
**Unresolved:** Exact 4.19 race vs this tree is not re-reproduced here.
The local code still matches the described mechanism.
**Stable rules:**
1. Obviously correct and tested? **PASS** — type check + drop/skip
cache; maintainer applied v1 unchanged.
2. Real user-visible bug? **PASS** — production vmcore.
3. Important (crash/corruption)? **PASS** — invalid `kfree` on unmount.
4. Small/contained? **PASS** — +6 lines, one function.
5. No new features/APIs? **PASS**.
6. Applies to this tree? **PASS**.
**Exception category:** None needed; it is a normal bugfix.
This tree is **6.12.111** (`pending-6.12`). The buggy path is here, the
fix is surgical, and the failure is a crash with memory corruption. It
should be backported.
Verification:
- [Phase 1] Tags parsed from provided message and GitHub `3265f1998ae9`;
no Fixes/Cc:stable; Trond SOB confirmed.
- [Phase 2] Diff is +6 in `nfs_symlink()` only; compared to local
`fs/nfs/dir.c:2612-2665`.
- [Phase 3] `git blame` on `nfs_symlink`; introducer `873101b337767` is
ancestor; `git merge-base --is-ancestor 3265f1998ae9 HEAD` → not in
this tree.
- [Phase 3] `nfs_dir_aops.free_folio = nfs_readdir_clear_array` at lines
71-73; `nfs_readdir_clear_array` kfree loop at 221-231; directory
`a_ops` set in `inode.c` ~501-505.
- [Phase 4] `b4 dig -c 3265f1998ae9` → lore msgid
`20260703065448.210255-1-zhang.guodong@linux.dev`; `-a` v1 only; `-w`
Trond+Anna+linux-nfs; mbox has no replies.
- [Phase 4] Openwall 7.3 pull lists this under other bugfixes:
https://lists.openwall.net/linux-kernel/2026/08/26/1997
- [Phase 4] Lore HTML 403; used b4 mbox instead.
- [Phase 5] Grep: v2/v3/v4 dir `i_op->symlink = nfs_symlink`;
`vfs_symlink` in `fs/namei.c`.
- [Phase 6] `make kernelversion` → 6.12.111; `git describe HEAD` →
`p-6.12-3-gc504cf0baa3d8`; insertion context unique; `d_is_symlink`
exists.
- [Phase 8] Failure mode from vmcore + local `nfs_readdir_clear_array`
code: invalid kfree, CRITICAL.
- UNVERIFIED: Could not re-run the 4.19 workload; mechanism match is
from code, not a new repro.
**YES**
fs/nfs/dir.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 38b0834e3ebf1..0d8ac9a7448af 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2632,6 +2632,12 @@ int nfs_symlink(struct mnt_idmap *idmap, struct inode *dir,
return error;
}
+ if (unlikely(!d_is_symlink(dentry))) {
+ d_drop(dentry);
+ folio_put(folio);
+ return 0;
+ }
+
nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
/*
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH AUTOSEL 6.18-6.12] NFSv4: pin the superblock for active state owners
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.12] NFS: Verify symlink inode before caching target Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Jia Zhu, Trond Myklebust, Sasha Levin, trondmy, anna, linux-nfs,
linux-kernel
From: Jia Zhu <zhujia.zj@bytedance.com>
[ Upstream commit 8cb1ce7aa0e8ac30e55f5bccfb80125f2e43e84a ]
NFSv4 open state can outlive the file and dentry that created it. This
was observed in production when NFSv4 state recovery, such as after a
server reboot or lease expiration, raced with unmount.
The race requires recovery to hold an open state reference while the last
open file is closed and the filesystem is unmounted, allowing the
superblock's active reference to drop to zero between
refcount_inc(&state->count) and nfs4_put_open_state():
state manager umount
nfs4_run_state_manager()
nfs4_do_reclaim()
nfs4_reclaim_open_state()
refcount_inc(&state->count)
...
close last file
generic_shutdown_super()
"Busy inodes after unmount"
nfs_free_server()
nfs4_put_open_state()
iput(inode)
evict()
nfs_clear_inode()
nfs_zap_acl_cache()
The "VFS: Busy inodes after unmount" warning is the visible symptom of
that lifetime mismatch: superblock teardown proceeds even though the NFS
open state still pins an inode. After umount has freed the server, the
state manager can then run nfs4_put_open_state() for the last open-state
reference. The resulting iput(inode) can evict an NFS inode with freed
server data, causing crashes at nfs_zap_acl_cache(). This can be
reproduced by delaying the reclaim path before nfs4_put_open_state(),
then closing the last file and unmounting the NFS mount.
Pin the superblock while a state owner is active, and drop the pin when
the owner becomes idle again, so the NFS server stays alive until all
open state associated with the owner has been released.
Assisted-by: Codex:GPT-5
Signed-off-by: Jia Zhu <zhujia.zj@bytedance.com>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This is a real NFSv4 recovery-vs-unmount use-after-free that exists in
this 6.12 tree. The fix is small, uses the existing `nfs_sb_active`
pattern, and should be backported.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[NFSv4] [pin] pin the superblock for active state owners**
Subsystem prefix: `NFSv4`. Action verb: **pin** (lifetime/refcount fix,
same class as “ensure”/“prevent”). Claimed intent: keep the NFS
superblock alive while a state owner is active so umount cannot free
`nfs_server` under recovery.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Verified on `8cb1ce7aa0e8ac30e55f5bccfb80125f2e43e84a`:
- **Assisted-by:** Codex:GPT-5
- **Signed-off-by:** Jia Zhu \<zhujia.zj@bytedance.com\> (author)
- **Signed-off-by:** Trond Myklebust \<trond.myklebust@hammerspace.com\>
(NFS client maintainer; committer)
- **No Fixes:**
- **No Reported-by / Tested-by / Reviewed-by / Acked-by**
- **No Link:**
- **No Cc: stable@vger.kernel.org** (expected for this review pipeline;
not a negative)
Notable: production report is in the body (“observed in production”),
not a `Reported-by` tag. Maintainer SOB is a strong quality signal.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** NFSv4 open state can outlive the file/dentry. State recovery
(`nfs4_reclaim_open_state`) holds `refcount_inc(&state->count)` across
a window where the last file can be closed and the mount unmounted.
- **Symptom:** `VFS: Busy inodes after unmount`, then crash in
`nfs_zap_acl_cache()` after `nfs_free_server()`.
- **Failure mode:** umount tears down the superblock/`nfs_server` while
recovery still pins an inode via open state. Later
`nfs4_put_open_state()` → `iput()` → `evict()` → `nfs_clear_inode()` →
`nfs_zap_acl_cache()` → `NFS_PROTO(inode)` dereferences freed
`nfs_server`.
- **Version info:** none in the message. Race is in long-standing NFSv4
reclaim/umount code.
- **Root cause:** open-state lifetime is not tied to `s_active` /
`nfs_server` lifetime.
- **Fix:** `nfs_sb_active()` while a state owner is counted;
`nfs_sb_deactive()` when `so_count` hits 0.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** Explicit production crash/UAF with a race
diagram and reproduction (delay reclaim, close last file, umount). Not
cleanup/refactor.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `fs/nfs/nfs4state.c` only (`+33/-7`, 40 lines)
- **Functions:**
- **Added:** `nfs4_get_state_owner_active_locked()`
- **Modified:** `nfs4_find_state_owner_locked()`,
`nfs4_insert_state_owner_locked()`, `nfs4_alloc_state_owner()`,
`nfs4_get_state_owner()`, `nfs4_put_state_owner()`
- **Scope:** single-file surgical lifetime fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
1. **Idle-owner revival (find/insert, `cl_lock` held):** before: always
`list_del_init(so_lru)` + `atomic_inc(so_count)`. After: if `so_count
== 0`, pin SB via `nfs_sb_active()`; if pin fails (umount already
dropped `s_active`), return NULL instead of reviving.
2. **New owner alloc:** after `kzalloc`, call
`nfs_sb_active(server->super)`; fail allocation if umount is
underway.
3. **Duplicate insert:** if insert returns an existing owner, free the
new one **and** drop the extra pin taken in alloc.
4. **Last put:** save `sb` before dropping `so_count`; on 0, put on LRU,
drop `cl_lock`, then `nfs_sb_deactive(sb)` (may call
`deactivate_super()`).
Pin is 0/1 per owner: taken on 0→1, dropped on 1→0. Extra `so_count`
bumps (reclaim, close, opendata) do not take extra pins.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Use-after-free / lifetime race] [superblock and `nfs_server`
freed while NFSv4 open state still holds an inode]**
Verified crash path in this tree:
- `nfs4_reclaim_open_state()` does `refcount_inc(&state->count)` then
later `nfs4_put_open_state()` (`fs/nfs/nfs4state.c`).
- `nfs4_put_open_state()` does `iput(inode)` then
`nfs4_put_state_owner()`.
- `nfs_kill_super()` calls `nfs_free_server()` even if inodes remain.
- `nfs_clear_inode()` → `nfs_zap_acl_cache()` → `NFS_PROTO(inode)` →
`NFS_SERVER(inode)->nfs_client->rpc_ops` (`include/linux/nfs_fs.h`),
which is UAF if `s_fs_info` was already freed.
Category: **race + UAF**. Same established NFS pattern as
delegation/unlink/`opendata` SB pins.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** high. Pin/unpin is balanced (alloc pin, duplicate-insert
deactive, last-put deactive). `nfs_sb_active()` is atomics-only, so it
is safe under `cl_lock`. `nfs_sb_deactive()` runs after dropping
`cl_lock` because `deactivate_super()` can sleep.
- **Regression risk:** low–medium. Umount now waits until owners go idle
(intended; matches other NFS pins). New OPEN during umount can fail if
`s_active==0` (correct). Imbalance would hang umount or leak
`s_active`; the get/put pairing is straightforward.
- **Red flags:** none for APIs/features. AI-assisted, but maintainer-
committed with unchanged v1.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Idle LRU + `atomic_inc(so_count)` in find/insert: `0aaaf5c424c7ff`
(“NFS: Cache state owners after files are closed”, Chuck Lever, 2011)
— **is an ancestor of HEAD**.
- `nfs4_put_state_owner` LRU move: `7bf97bc27308cf` (2012).
- `nfs4_alloc_state_owner`: structure since `d1e284d50a1506` (2012);
`kzalloc(sizeof(*sp))` still used in **this** 6.12 tree.
- Reclaim extra open-state ref: long-standing
`nfs4_reclaim_open_state()`.
Bug is in code present for many years, not post-6.12.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Introducer is the 2011 LRU caching plus
reclaim holding open-state refs without an SB pin. Both exist in this
tree.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: recent `fs/nfs/nfs4state.c` work is other recovery/hang fixes
(`71e07bb1556c7`, `a848c29e34861`, `8f6a7c9467eaf`). No prerequisite
refactor for this pin. **Standalone**, not `patch X/Y`.
Parent of `8cb1ce7aa0e8` is `932a8cf6abb2b` (unrelated sunrpc pipefs
UAF). Not required here.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Jia Zhu has no other commits on `fs/nfs/` in this tree. Trond
(maintainer) committed it. Related historical pin: `9f0f8e12c48e4`
“NFSv4: Pin the superblock while we're returning the delegation” (2015)
**is** in this tree.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **None.** Needs existing `nfs_sb_active`/`nfs_sb_deactive`
(`fs/nfs/super.c`; present). `kzalloc_obj` in the mainline hunk is
**not** used in this tree’s `nfs4_alloc_state_owner` (still
`kzalloc(sizeof(*sp))`). Backport keeps `kzalloc` and adds the
`nfs_sb_active` check. `kzalloc_obj` the macro exists in 6.12
`include/linux/slab.h` but is not a dependency of the fix.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 8cb1ce7aa0e8` → https://patch.msgid.link/20260707133623.230
78-1-zhujia.zj@bytedance.com
- `b4 dig -a`: **v1 only**, 2026-07-07, Jia Zhu. Applied version **is**
v1.
- `b4 mbox`: **1 message, no replies, no NAK, no v2.**
- Merged via Trond’s NFS pull for Linux 7.3 (`tags/nfs-for-7.3-1`), into
Linus as part of `73e3f0710014`. SHA `8cb1ce7aa0e8`, 2026-08-17.
- Pull letter lists it under Jia Zhu and describes “NFSv4: Fix another
state manager race with umount” under **Other bugfixes** (not the
“Stable fixes” bullet list). That is description of this race, not a
second patch in the shortlog.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To Trond Myklebust, Anna Schumaker; Cc linux-nfs,
linux-kernel. No on-list review comments. Maintainer applied it (~6
weeks later).
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: no syzbot/bugzilla. Author states **production** observation and
a concrete repro (delay reclaim, close last file, umount). Verified
reclaim/`put_open_state`/`nfs_zap_acl_cache` code matches the diagram.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: standalone. Same 7.3 pull also has `cf616096a0f3` “NFS: Pin the
`struct nfs_server` during a FREE_STATEID call” — **different bug**,
**not** an ancestor of this HEAD, **not** required for this patch.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search returned **403**. **UNVERIFIED**
whether it was discussed on stable@. No evidence it was rejected.
Absence of Cc: stable is expected.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: KEY FUNCTIONS**
Record: `nfs4_get_state_owner_active_locked` (new),
`nfs4_find_state_owner_locked`, `nfs4_insert_state_owner_locked`,
`nfs4_alloc_state_owner`, `nfs4_get_state_owner`,
`nfs4_put_state_owner`. Related unchanged: `nfs4_reclaim_open_state`,
`nfs4_do_reclaim`, `nfs4_put_open_state`.
**Step 5.2: TRACE CALLERS**
Record:
- `nfs4_get_state_owner()`: `nfs4_do_open()` (`nfs4proc.c`),
`nfs4file.c` (copy/open context). Userspace OPEN on NFSv4.
- `nfs4_put_state_owner()`: `nfs4_put_open_state`, `__nfs4_close`,
`nfs4_do_reclaim`, opendata free, several `nfs4proc.c` paths.
Process/workqueue context (`iput`, RPC completion).
`nfs_sb_deactive()` → `deactivate_super()` is sleepable; those paths
already sleep.
- `nfs4_do_reclaim()` uses `atomic_inc_not_zero(&sp->so_count)`
(already-active owners only). With this patch those owners already
hold the SB pin from the original get.
**Step 5.3: TRACE CALLEES**
Record: `nfs_sb_active` / `nfs_sb_deactive` (`super.c`): first pin keeps
an extra `s_active`; last `nfs_sb_deactive` calls `deactivate_super()`.
Same helpers as unlink, pnfs, delegation, callback, opendata.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: reachable from NFSv4 OPEN + lease recovery + umount. State
manager is a kthread (`nfs4_run_state_manager`). Unprivileged users with
an NFSv4 mount can close the last file and umount (typically admin for
umount). Recovery after server reboot/lease expiry is a **common**
enterprise path. **Yes, user-triggerable on any NFSv4 client.**
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `nfs_sb_active` already used in `unlink.c`, `pnfs.c`,
`nfs4proc.c` (opendata), `inode.c`, `delegation.c`, `callback_proc.c`.
`9f0f8e12c48e4` is the same class of fix. This extends that pattern to
state owners.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** Tree is **6.12.111** (`pending-6.12`,
`p-6.12-3-gc504cf0baa3d8`).
`git merge-base --is-ancestor 8cb1ce7aa0e8 HEAD` → **not an ancestor**.
Local `nfs4_find_state_owner_locked` still does unpinned `atomic_inc`;
`nfs4_put_state_owner` has no `nfs_sb_deactive`; reclaim still
`refcount_inc` + `nfs4_put_open_state`. Bug is here.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **minor context only.** Same function bodies except alloc uses
`kzalloc(sizeof(*sp), gfp_flags)` here vs `kzalloc_obj(*sp, gfp_flags)`
upstream. Rest should apply with fuzz or a one-line context tweak. No
structural conflict.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: no `nfs4_get_state_owner_active_locked`. Delegation SB pin is
present; it does not cover this reclaim/open-state race. No alternate
fix found.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: SUBSYSTEM AND CRITICALITY**
Record: **[fs/nfs NFSv4 client] [IMPORTANT]**
Not core mm/VFS, but NFSv4 is widely used (enterprise NAS, diskless,
containers). Crash is in client umount/recovery.
**Step 7.2: SUBSYSTEM ACTIVITY**
Record: mature, still actively fixed. State-owner LRU dates to 2011;
this is a long-standing lifetime hole, not a new-subsystem issue.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Record: **NFSv4 client users** (`CONFIG_NFS_V4`). Trigger: recovery
(reboot/lease expiry) racing last close + umount. Production report from
ByteDance.
**Step 8.2: TRIGGER CONDITIONS**
Record: needs NFSv4 state recovery overlapping last-file close and
umount. Timing-dependent but **reproduced** (delay before
`nfs4_put_open_state`) and seen in production. Umount is typically
privileged; recovery is not a syzbot syscall gadget, but the crash is
real.
**Step 8.3: FAILURE MODE SEVERITY**
Record: **UAF → kernel oops/crash after “Busy inodes after unmount”.**
`NFS_PROTO()` loads `nfs_client` from freed `nfs_server`. **Severity:
HIGH/CRITICAL.**
**Step 8.4: RISK-BENEFIT RATIO**
Record:
- **Benefit:** high — prevents production NFSv4 umount crash/UAF.
- **Risk:** low–medium — ~33 lines, one file, established helpers.
Umount waits for idle owners (correct). Pin imbalance would hang
umount; pairing is clear.
- **Ratio:** benefit outweighs risk.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Production crash/UAF with a clear race diagram and repro.
- Buggy code is in this 6.12 tree; fix is not.
- Small, one-file, uses existing `nfs_sb_active`/`nfs_sb_deactive`.
- NFS maintainer (Trond) committed it; merged to mainline for 7.3.
- Same class as prior NFS SB-pin fixes already in this tree.
- Reclaim path verified: extra open-state ref without SB pin;
`nfs_kill_super` frees server with busy inodes.
**AGAINST:**
- No Fixes:/Cc: stable (expected; not a reason to reject).
- No on-list review replies; AI-assisted. Mitigated by maintainer SOB.
- Pull letter put it under “Other bugfixes” not “Stable fixes” (likely
because it lacked Cc: stable).
- Umount may wait longer while owners drain — intended, same as other
NFS pins.
- Tiny backport context (`kzalloc` vs `kzalloc_obj`).
**UNRESOLVED:**
- lore.kernel.org/stable returned 403; no confirmed stable@ thread.
- No public `Tested-by` besides the author’s production/repro claim
(unverified independently, but code path matches).
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — pin/unpin is balanced;
production + explicit repro; maintainer applied v1 unchanged.
2. Fixes a real bug that affects users? **PASS** — production UAF/crash.
3. Important (crash/security/corruption/deadlock)? **PASS** — crash/UAF
on umount during recovery. **HIGH/CRITICAL**.
4. Small and contained? **PASS** — +33/-7, one file.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — same functions; minor
`kzalloc` context only.
**Step 9.3: EXCEPTION CATEGORIES**
Record: none of device-ID/quirk/DT/build/docs. Qualifies as a normal
important bugfix.
**Step 9.4: DECISION**
Backport to this 6.12.y tree. The lifetime hole is here, the failure is
a real crash, and the fix is the same SB-pin pattern NFS already uses.
---
### What this solves for stable users
On NFSv4, lease recovery can hold an open-state ref while the last file
is closed and the mount is unmounted. VFS then warns `Busy inodes after
unmount`, `nfs_free_server()` frees `nfs_server`, and the later `iput`
in `nfs4_put_open_state()` oopses in `nfs_zap_acl_cache()` via
`NFS_PROTO()`. Pinning the superblock for the life of an active state
owner keeps `nfs_server` alive until that state is dropped.
### Risk vs benefit
Benefit is high (production oops). Risk is low: ~30 lines, existing
helpers, umount waits instead of tearing down under live state. Backport
may need to keep `kzalloc(sizeof(*sp), gfp_flags)` instead of
`kzalloc_obj`.
---
### Verification
- **[Phase 1]** `git describe HEAD` / Makefile: **6.12.111**, branch
`pending-6.12`. Tags on `8cb1ce7aa0e8`: Assisted-by Codex, SOB Jia Zhu
+ Trond; no Fixes/Cc stable/Reported-by.
- **[Phase 2]** Diff: +33/-7 in `fs/nfs/nfs4state.c`. Local
reclaim/`put_open_state`/`nfs_clear_inode`/`NFS_PROTO` match the crash
story.
- **[Phase 2]** `nfs_sb_active`/`nfs_sb_deactive` in `fs/nfs/super.c`
confirmed (atomics; last deactive → `deactivate_super`).
- **[Phase 3]** `git blame` on find/put: LRU caching `0aaaf5c424c7ff`
(2011) **is** ancestor. `git merge-base --is-ancestor 8cb1ce7aa0e8
HEAD` → **false**.
- **[Phase 3]** Parent `932a8cf6abb2b` is unrelated sunrpc UAF; not a
prerequisite.
- **[Phase 4]** `b4 dig -c 8cb1ce7aa0e8`: lore msgid
`20260707133623.23078-1-zhujia.zj@bytedance.com`. `-a`: v1 only. `-w`:
Trond + Anna + linux-nfs. `b4 mbox`: 1 message, no replies.
- **[Phase 4]** LKML GIT PULL 2026-08-26: Jia Zhu commit listed; merged
to Linus (`73e3f0710014`).
- **[Phase 4]** lore.kernel.org and lore stable search: **403** —
UNVERIFIED stable@ discussion.
- **[Phase 5]** Callers: `nfs4_do_open`, `nfs4file.c`, reclaim,
opendata, close. `nfs4_do_reclaim` uses `atomic_inc_not_zero` (active
owners only → pin already held).
- **[Phase 6]** Local find/insert/put still unpinned. Alloc still
`kzalloc(sizeof(*sp))` vs upstream `kzalloc_obj`.
- **[Phase 7]** `CONFIG_NFS_V4` exists; NFSv4 client is important for
LTS.
- **[Phase 8]** Failure: UAF/oops after busy-inodes warning. Severity
HIGH/CRITICAL.
- **UNVERIFIED:** independent tester beyond author; stable@ archive
contents.
**YES**
fs/nfs/nfs4state.c | 40 +++++++++++++++++++++++++++++++++-------
1 file changed, 33 insertions(+), 7 deletions(-)
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 01179f7de3225..6147d8c0ee111 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -403,6 +403,26 @@ const struct cred *nfs4_get_clid_cred(struct nfs_client *clp)
return cred;
}
+static bool
+nfs4_get_state_owner_active_locked(struct nfs4_state_owner *sp)
+{
+ struct nfs_server *server = sp->so_server;
+
+ /*
+ * A counted state owner may dereference so_server until the final
+ * nfs4_put_state_owner(). Pin the superblock when reviving an idle
+ * owner so umount cannot free the server underneath it.
+ */
+ if (atomic_read(&sp->so_count) == 0) {
+ if (!nfs_sb_active(server->super))
+ return false;
+ if (!list_empty(&sp->so_lru))
+ list_del_init(&sp->so_lru);
+ }
+ atomic_inc(&sp->so_count);
+ return true;
+}
+
static struct nfs4_state_owner *
nfs4_find_state_owner_locked(struct nfs_server *server, const struct cred *cred)
{
@@ -421,9 +441,8 @@ nfs4_find_state_owner_locked(struct nfs_server *server, const struct cred *cred)
else if (cmp > 0)
p = &parent->rb_right;
else {
- if (!list_empty(&sp->so_lru))
- list_del_init(&sp->so_lru);
- atomic_inc(&sp->so_count);
+ if (!nfs4_get_state_owner_active_locked(sp))
+ return NULL;
return sp;
}
}
@@ -449,9 +468,8 @@ nfs4_insert_state_owner_locked(struct nfs4_state_owner *new)
else if (cmp > 0)
p = &parent->rb_right;
else {
- if (!list_empty(&sp->so_lru))
- list_del_init(&sp->so_lru);
- atomic_inc(&sp->so_count);
+ if (!nfs4_get_state_owner_active_locked(sp))
+ return NULL;
return sp;
}
}
@@ -501,6 +519,10 @@ nfs4_alloc_state_owner(struct nfs_server *server,
sp = kzalloc(sizeof(*sp), gfp_flags);
if (!sp)
return NULL;
+ if (!nfs_sb_active(server->super)) {
+ kfree(sp);
+ return NULL;
+ }
sp->so_seqid.owner_id = atomic64_inc_return(&server->owner_ctr);
sp->so_server = server;
sp->so_cred = get_cred(cred);
@@ -586,8 +608,10 @@ struct nfs4_state_owner *nfs4_get_state_owner(struct nfs_server *server,
spin_lock(&clp->cl_lock);
sp = nfs4_insert_state_owner_locked(new);
spin_unlock(&clp->cl_lock);
- if (sp != new)
+ if (sp != new) {
nfs4_free_state_owner(new);
+ nfs_sb_deactive(server->super);
+ }
out:
nfs4_gc_state_owners(server);
return sp;
@@ -609,6 +633,7 @@ void nfs4_put_state_owner(struct nfs4_state_owner *sp)
{
struct nfs_server *server = sp->so_server;
struct nfs_client *clp = server->nfs_client;
+ struct super_block *sb = server->super;
if (!atomic_dec_and_lock(&sp->so_count, &clp->cl_lock))
return;
@@ -616,6 +641,7 @@ void nfs4_put_state_owner(struct nfs4_state_owner *sp)
sp->so_expires = jiffies;
list_add_tail(&sp->so_lru, &server->state_owners_lru);
spin_unlock(&clp->cl_lock);
+ nfs_sb_deactive(sb);
}
/**
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-19 14:01 UTC | newest]
Thread overview: 4+ 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:47 ` [PATCH AUTOSEL 6.18-6.12] pNFS: honor clora_changed when recalling a layout Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18] NFSv4/flexfiles: report cancelled I/O as a layout error Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.12] NFS: Verify symlink inode before caching target Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-6.12] NFSv4: pin the superblock for active state owners Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox