* [PATCH AUTOSEL 6.18] hfs: validate catalog CNIDs before instantiating inodes
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.12] Revert "namei: don't reveal overmounted entries in refwalk" Sasha Levin
` (2 subsequent siblings)
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: David Maximiliano Hermitte, syzbot+97e301b4b82ae803d21b,
George Anthony Vernon, Tetsuo Handa, Viacheslav Dubeyko,
Sasha Levin, frank.li, linux-fsdevel, linux-kernel
From: David Maximiliano Hermitte <davemadmaxxx@gmail.com>
[ Upstream commit 53c3c138d672765c26884c55728307a65c634328 ]
hfs_cat_find_brec() first resolves a catalog thread record by CNID and
then looks up the corresponding catalog record by parent/name. On a
corrupted filesystem image, the second lookup may find a record whose
CNID does not match the CNID that was requested.
Validate the record found by the second lookup before returning it.
Read the already-found record with hfs_bnode_read(), require the exact
fixed size for file and directory records, reject other record types,
and verify that the stored CNID matches the requested CNID.
Also validate reserved CNIDs in hfs_read_inode() before populating the
inode, propagate hfs_read_inode() failures from the resource-fork lookup
path, and reject bad resource-fork and root inodes.
Keep hfs_write_inode() unchanged, so corrupted catalog records are
rejected before reaching its existing reserved-CNID BUG() path.
Reported-by: syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b
Cc: George Anthony Vernon <contact@gvernon.com>
Cc: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: David Maximiliano Hermitte <davemadmaxxx@gmail.com>
Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/r/20260708215636.73815-1-davemadmaxxx@gmail.com
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local checkout is **7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The
commit is `53c3c138d6727` and is already in this mainline history (first
contained in `v7.3-rc1`). The evaluation below is whether that fix is
suitable to take into stable from this tree: HFS is present, the `BUG()`
path is ancient, and the patch is self-contained.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[hfs] [validate] Validate catalog CNIDs before instantiating
inodes** — “validate” here is a correctness/safety check, not a feature.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Reported-by:** syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
(fuzzer, reproducible)
- **Closes:**
https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b
- **Cc:** George Anthony Vernon, Tetsuo Handa
- **Signed-off-by:** David Maximiliano Hermitte (author); Viacheslav
Dubeyko (HFS maintainer, twice)
- **Reviewed-by:** Viacheslav Dubeyko
- **Link:** https://lore.kernel.org/r/20260708215636.73815-1-
davemadmaxxx@gmail.com
- **No Fixes:** tag (expected)
- **No Cc: stable** (expected; not a negative)
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `hfs_cat_find_brec()` resolves a thread by CNID, then looks
up the catalog record by parent/name. On a corrupted image that second
lookup can return a record whose stored CNID does not match the
requested CNID.
- **Symptom/failure mode:** A corrupted catalog record is turned into an
inode with a reserved CNID (`i_ino < HFS_FIRSTUSER_CNID` and not
root/extents/catalog). Later writeback hits `BUG()` in
`hfs_write_inode()`.
- **Version info:** none in the message; syzbot first crash is
2022-11-25.
- **Root cause (author):** missing validation of the second catalog
lookup, plus missing reserved-CNID checks at inode instantiation;
resource-fork lookup ignored `hfs_read_inode()` failures.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. Explicit syzbot crash fix: reject bad
catalog records so they never reach the reserved-CNID `BUG()`.
---
## PHASE 2: DIFF ANALYSIS - LINE BY LINE
**Step 2.1: INVENTORY THE CHANGES**
Record:
- `fs/hfs/catalog.c`: +40 / -1 — add `hfs_cat_validate_found_cnid()`,
call it from `hfs_cat_find_brec()`
- `fs/hfs/hfs_fs.h`: +19 — add inline `hfs_is_valid_cnid()`
- `fs/hfs/inode.c`: +12 / -1 — CNID checks in `hfs_read_inode()`;
propagate errors and `is_bad_inode()` in `hfs_file_lookup()`
- `fs/hfs/super.c`: +5 — reject bad root inode in `hfs_fill_super()`
- **Total:** +76 / -2 across 4 HFS files
- **Scope:** single-subsystem surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **`hfs_cat_find_brec`:** Before: thread lookup, then `return
hfs_brec_find(fd)` with no check that the found record’s CNID matches.
After: on success, read the found record, require exact file/dir size,
reject other types, require `found_cnid == cnid`, else `-EIO`.
- **`hfs_read_inode`:** Before: populate inode from any file/dir record,
including reserved CNIDs 0,1,5–15. After: `hfs_is_valid_cnid()`;
invalid → `-EIO` before `i_ino` is set.
- **`hfs_file_lookup` (rsrc):** Before: ignored `hfs_read_inode()`
return, then `mark_inode_dirty()`. After: propagate errors;
`is_bad_inode()` → `-EIO` + `iput`.
- **`hfs_fill_super`:** After `hfs_iget()`, `is_bad_inode(root)` →
`iput` and fail mount instead of `d_make_root()` on a bad inode.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** memory-safety / logic — corrupted on-disk metadata →
kernel `BUG()` (oops, invalid opcode).
- **Mechanism:** Reserved CNIDs instantiated as normal inodes; writeback
`hfs_write_inode()` does `BUG()` in the default branch of
`inode->i_ino < HFS_FIRSTUSER_CNID`. Also bounds/type checks on
catalog record length.
- **Not:** leak, UAF, or locking fix.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- Obviously correct relative to HFS CNID rules: user CNIDs ≥ 16;
reserved 2=root dir, 3=extents file, 4=catalog file; others invalid
for catalog-instantiated inodes.
- `iget5_locked` `set()` returning non-zero → `inode_insert5()` returns
NULL; `hfs_iget()` already treats NULL as failure.
- New `hfs_read_inode()` code does not sleep (safe under
`inode_hash_lock`).
- Maintainer asked to leave `hfs_write_inode()` unchanged; defense is at
instantiation. Remaining `BUG()` is only if some other path still
creates a reserved-CNID inode.
- **Regression risk:** low. Could reject a theoretically weird volume
with catalog entries using CNIDs 1 or 5–15; that is invalid HFS and
those inodes would `BUG()` on writeback anyway.
- No public API / data-structure change.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record: `hfs_write_inode()` reserved-CNID `BUG()` and
`hfs_cat_find_brec()` date to **1da177e4c3f41 (Linux-2.6.12-rc2,
2005)**. Long-standing, not a newly introduced bug.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Buggy behavior is original HFS catalog/inode
code, not a recent regression.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: Nearby HFS work (`hfs_correct_next_unused_CNID`, MDB locking) is
independent. This commit is standalone, not `patch X/Y`. After it, later
`fs/hfs/{inode,catalog,super,hfs_fs.h}` commits are unrelated (MDB,
delayed work, ENOSPC). No follow-up fix to this patch.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Only this HFS commit from David Maximiliano Hermitte. Weight
comes from **Reviewed-by / Signed-off-by Viacheslav Dubeyko** (HFS
maintainer). Earlier related attempts: George Vernon, Tetsuo Handa
(syzbot thread).
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Uses existing `hfs_bnode_read()`, `hfs_brec_find()`,
`is_bad_inode()`, `HFS_*_CNID`, `hfs_read_inode()` already returning
`int` since 2.6.12. **Can apply standalone.**
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig:**
https://patch.msgid.link/20260708215636.73815-1-davemadmaxxx@gmail.com
- **Revisions:** v1 (2026-07-02) → v3 RESEND → v4 → **v5 (applied)**
- Maintainer review drove exact size checks, signed `rec_len`, keep
`hfs_write_inode()` unchanged, preserve `hfs_file_lookup()` error
codes
- v5: Slava: “Looks good. Thanks a lot for the fix.” + Reviewed-by
- Tetsuo: not a NAK; points at sashiko notes on *pre-existing* issues
- Author revalidated with HFS build and QEMU syzbot repro
- **No NAKs** of the committed approach
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (b4 dig -w): Viacheslav Dubeyko, Tetsuo Handa, George Vernon,
Jori Koolstra, linux-fsdevel, syzbot. Maintainer reviewed and merged.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record (syzbot, live page):
- Title: **kernel BUG in hfs_write_inode**
- First crash: **2022-11-25**; last listed ~35d before the page
snapshot; **243695 crashes**; C and syz reproducers
- Stack: `hfs_write_inode` ← writeback `wb_workfn` (kworker), invalid
opcode at `BUG()`
- Security assessment: DenialOfService yes, FilesystemTrigger yes,
**Unprivileged: no** (needs a mountable image)
- Duplicate: “invalid opcode in hfs_write_inode”
- Fix commit listed as `53c3c138d672`
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Single-patch series (v1–v5 of the same change). Earlier
alternate approaches (replace `BUG()` with `-EIO`/`-EFSCORRUPTED`,
broader `BUG()` removal) were **not** what merged; this validation-at-
instantiation patch is standalone.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** — lore.kernel.org fetches returned 403. No `Cc:
stable` in the fsdevel thread. That absence is not a reason to reject.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `hfs_cat_validate_found_cnid` (new), `hfs_cat_find_brec`,
`hfs_is_valid_cnid` (new inline), `hfs_read_inode`, `hfs_file_lookup`,
`hfs_fill_super`. `hfs_write_inode` unchanged.
**Step 5.2: TRACE CALLERS**
Record:
- `hfs_cat_find_brec()` — only `hfs_fill_super()` (root at mount)
- `hfs_read_inode()` — `iget5_locked` set callback from `hfs_iget()`
(`hfs_lookup` / `hfs_fill_super`) and direct call from
`hfs_file_lookup()` (rsrc)
- `hfs_write_inode()` — `s_op->write_inode` from writeback (crash site)
**Step 5.3: TRACE CALLEES**
Record: `hfs_bnode_read`, `hfs_brec_find`, `be32_to_cpu`,
`is_bad_inode`, `iput`. No new allocations/locks.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Reachable from **mounting an HFS image** and from **lookup /
rsrc** on a mounted volume. Syzbot C repro mounts a crafted image;
writeback then `BUG()`s. Userspace-triggerable if the caller can mount
HFS (`CONFIG_HFS_FS`).
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: HFS+ already special-cases reserved vs user CNIDs in
`hfsplus_iget()` (`fs/hfsplus/super.c`). Classic HFS lacked that for
catalog-instantiated inodes. Same class of bug, HFS-only fix.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: Yes. `BUG()` still in `hfs_write_inode()` at `fs/hfs/inode.c`
(lines 479–491), present since 2.6.12. Validation from this commit is
already applied here (`hfs_is_valid_cnid` in `hfs_fs.h`,
`hfs_cat_validate_found_cnid` in `catalog.c`). Introduced: original HFS
port, not post-7.3.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Expected apply: clean / minor context** on trees that still
have `hfs_cat_find_brec`, `hfs_read_inode`, `hfs_file_lookup`,
`hfs_fill_super`. New code does not depend on `inode_set_ctime_to_ts`,
`set_default_d_op`, or CNID-correction helpers. Constants live in
`include/linux/hfs_common.h` here; older trees may still have them in
`fs/hfs/hfs.h` — only a context/include issue, not a logic dependency.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This is the fix (`53c3c138d6727`). No earlier equivalent in-
tree. Not in `v7.2`.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **fs/hfs** — **PERIPHERAL** (legacy Mac HFS, `CONFIG_HFS_FS`,
not the VFS core). Still a real on-disk filesystem; a `BUG()` on
corrupted images is a kernel crash, not a cosmetic warning.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained under Viacheslav Dubeyko (`hfs-v7.3-tag1`
and other recent HFS tags). Mature code with ongoing syzbot/hardening.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Config/driver-specific:** `CONFIG_HFS_FS`. Anyone who can
mount HFS (including fuzzers, forensic mounts, old Mac volumes). Not
universal.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Mount / use a **corrupted HFS image**. Syzbot has a C repro;
240k+ crashes over ~4 years. Not unprivileged without mount capability.
Timing-independent once the bad inode is instantiated and dirtied
(`hfs_file_lookup` does `mark_inode_dirty` on rsrc).
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **`BUG()` / invalid opcode in writeback kworker → oops.**
**Severity: CRITICAL** (kernel crash). DoS, not a demonstrated
privilege-escalation.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for HFS users and any tree that enables HFS — stops
a years-old reproducible `BUG()`.
- **Risk:** low — +76 lines, HFS-only, reviewed, tested with QEMU repro;
failure mode on bad images becomes `-EIO` / failed mount instead of
oops.
- **Ratio:** strongly favors backport.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Syzbot kernel `BUG()` since 2022, C repro, hundreds of thousands of
crashes
- Crash in writeback (`wb_workfn` → `hfs_write_inode`)
- Small, contained HFS-only change (+76/-2)
- Maintainer Reviewed-by after v1–v5
- Bug in code since 2.6.12; no missing prerequisites in this tree
- Matches stable rules: real crash, obviously correct validation, no new
API
**AGAINST:**
- HFS is a legacy/peripheral FS
- Needs ability to mount HFS (not unprivileged syscall-only)
- `hfs_write_inode()` `BUG()` remains as a backstop if another path
instantiates a reserved CNID
- No `Cc: stable` (expected for this review queue)
**UNRESOLVED:**
- lore.kernel.org/stable search (HTTP 403)
- Whether every older stable series has identical hunk context
(`hfs_common.h` vs `hfs.h`)
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — maintainer review, QEMU
repro, syzbot
2. Fixes a real bug that affects users? **PASS** — reproducible `BUG()`
on corrupted HFS
3. Important issue? **PASS** — kernel oops (CRITICAL)
4. Small and contained? **PASS** — 4 files, +76/-2, HFS only
5. No new features or APIs? **PASS** — validation only
6. Can apply to the local tree? **PASS** — code is present; patch is
self-contained (already merged here as of 7.3-rc1)
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not ID/quirk/DT/docs. Ordinary **crash fix** on filesystem
corruption.
**Step 9.4: MAKE YOUR DECISION**
This is a real, long-standing, syzbot-reproduced kernel `BUG()` on
corrupted HFS catalog data. The fix is small, reviewed, and stops bad
CNIDs from becoming inodes that writeback will crash on. That is
appropriate for stable.
---
### What the commit solves and why it matters
On a corrupted HFS catalog, a thread lookup by CNID can be followed by a
parent/name lookup that returns a **different** record. That record can
carry a reserved CNID (0, 1, 5–15, …). Instantiating it and dirtying it
(notably the resource-fork path, which previously ignored
`hfs_read_inode()` errors) causes **writeback to `BUG()`** in
`hfs_write_inode()`. Stable kernels that still ship HFS will oops the
same way; rejecting the record with `-EIO` is the right user-visible
behavior.
### Stable rules / risk
Surgical, HFS-only, no new APIs, maintainer-acked, fuzzer-proven.
Residual risk is failing mount/lookup on already-invalid volumes that
used to oops instead.
### Concerns
Older trees may need a trivial context tweak if CNID macros are still in
`fs/hfs/hfs.h`. No semantic prerequisites. The remaining `BUG()` in
`hfs_write_inode()` is intentional per the maintainer.
### Verification
- **[Phase 1]** Tags: syzbot Reported-by, Closes syzkaller URL,
Reviewed-by Dubeyko, lore Link; no Fixes/Cc-stable
- **[Phase 2]** Diff: +76/-2 in catalog.c, hfs_fs.h, inode.c, super.c;
validation + error propagation
- **[Phase 3]** `git describe HEAD` → v7.3-rc4-27-gf0100363d8c37; `make
kernelversion` → 7.3.0-rc4
- **[Phase 3]** Commit `53c3c138d6727`; `git merge-base --is-ancestor
53c3c138d6727 HEAD` → yes
- **[Phase 3]** `git merge-base --is-ancestor 53c3c138d6727 v7.2` → not
in v7.2; `git describe --contains` → v7.3-rc1~222^2~9
- **[Phase 3]** Blame: `BUG()` in `hfs_write_inode` from 1da177e4c3f41
(2.6.12-rc2)
- **[Phase 3]** `hfs_read_inode` already `static int` since 2.6.12; pre-
fix `hfs_file_lookup` discarded its return value
- **[Phase 4]** `b4 dig -c 53c3c138d6727` → msgid
20260708215636.73815-1; v1/v3/v4/v5; v5 applied
- **[Phase 4]** `b4 dig -w`: Dubeyko, Handa, fsdevel, syzbot
- **[Phase 4]** `b4 dig -m`: Slava “Looks good” + Reviewed-by; asked to
leave `hfs_write_inode` unchanged
- **[Phase 4]** WebFetch syzbot: kernel BUG in hfs_write_inode,
2022-11-25, C repro, 243695 crashes
- **[Phase 4]** lore.kernel.org WebFetch: 403 — stable-list search
UNVERIFIED
- **[Phase 5]** Callers: `hfs_cat_find_brec` only from `hfs_fill_super`;
`hfs_iget` from dir lookup + mount; rsrc via `hfs_file_lookup`
- **[Phase 5]** `inode_insert5()` treats failed `set()` as NULL inode
(verified `fs/inode.c`)
- **[Phase 6]** Local files already contain the new helpers; `BUG()`
path still present
- **[Phase 8]** Failure mode: oops in kworker writeback, severity
CRITICAL
- **UNVERIFIED:** stable@ discussion (lore 403); exact hunk context on
every LTS series
**YES**
fs/hfs/catalog.c | 41 ++++++++++++++++++++++++++++++++++++++++-
fs/hfs/hfs_fs.h | 19 +++++++++++++++++++
fs/hfs/inode.c | 13 ++++++++++++-
fs/hfs/super.c | 5 +++++
4 files changed, 76 insertions(+), 2 deletions(-)
diff --git a/fs/hfs/catalog.c b/fs/hfs/catalog.c
index ccdbbffaaf7c1..f3fd84cffed5f 100644
--- a/fs/hfs/catalog.c
+++ b/fs/hfs/catalog.c
@@ -182,6 +182,40 @@ int hfs_cat_keycmp(const btree_key *key1, const btree_key *key2)
key2->cat.CName.name, key2->cat.CName.len);
}
+static int hfs_cat_validate_found_cnid(struct hfs_find_data *fd, u32 cnid)
+{
+ hfs_cat_rec rec;
+ u32 found_cnid;
+ int rec_len;
+
+ rec_len = fd->entrylength;
+ if (rec_len <= 0 || (size_t)rec_len > sizeof(rec))
+ return -EIO;
+
+ memset(&rec, 0, sizeof(rec));
+ hfs_bnode_read(fd->bnode, &rec, fd->entryoffset, rec_len);
+
+ switch (rec.type) {
+ case HFS_CDR_FIL:
+ if ((size_t)rec_len != sizeof(struct hfs_cat_file))
+ return -EIO;
+ found_cnid = be32_to_cpu(rec.file.FlNum);
+ break;
+ case HFS_CDR_DIR:
+ if ((size_t)rec_len != sizeof(struct hfs_cat_dir))
+ return -EIO;
+ found_cnid = be32_to_cpu(rec.dir.DirID);
+ break;
+ default:
+ return -EIO;
+ }
+
+ if (found_cnid != cnid)
+ return -EIO;
+
+ return 0;
+}
+
/* Try to get a catalog entry for given catalog id */
// move to read_super???
int hfs_cat_find_brec(struct super_block *sb, u32 cnid,
@@ -208,7 +242,12 @@ int hfs_cat_find_brec(struct super_block *sb, u32 cnid,
return -EIO;
}
memcpy(fd->search_key->cat.CName.name, rec.thread.CName.name, len);
- return hfs_brec_find(fd);
+
+ res = hfs_brec_find(fd);
+ if (res)
+ return res;
+
+ return hfs_cat_validate_found_cnid(fd, cnid);
}
static inline
diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index 86b5ae62247be..724b28bb8c6c4 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -156,6 +156,25 @@ extern int hfs_cat_move(u32 cnid, struct inode *src_dir,
extern void hfs_cat_build_key(struct super_block *sb, btree_key *key,
u32 parent, const struct qstr *name);
+/*
+ * Validate the CNID of a catalog record.
+ */
+static inline bool hfs_is_valid_cnid(u32 cnid, u8 type)
+{
+ if (likely(cnid >= HFS_FIRSTUSER_CNID))
+ return true;
+
+ switch (cnid) {
+ case HFS_ROOT_CNID:
+ return type == HFS_CDR_DIR;
+ case HFS_EXT_CNID:
+ case HFS_CAT_CNID:
+ return type == HFS_CDR_FIL;
+ default:
+ return false;
+ }
+}
+
/* dir.c */
extern const struct file_operations hfs_dir_operations;
extern const struct inode_operations hfs_dir_inode_operations;
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index f1796e4b07e83..e815e9a8ac6f2 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -365,6 +365,9 @@ static int hfs_read_inode(struct inode *inode, void *data)
rec = idata->rec;
switch (rec->type) {
case HFS_CDR_FIL:
+ if (!hfs_is_valid_cnid(be32_to_cpu(rec->file.FlNum), rec->type))
+ return -EIO;
+
if (!HFS_IS_RSRC(inode)) {
hfs_inode_read_fork(inode, rec->file.ExtRec, rec->file.LgLen,
rec->file.PyLen, be16_to_cpu(rec->file.ClpSize));
@@ -386,6 +389,9 @@ static int hfs_read_inode(struct inode *inode, void *data)
inode->i_mapping->a_ops = &hfs_aops;
break;
case HFS_CDR_DIR:
+ if (!hfs_is_valid_cnid(be32_to_cpu(rec->dir.DirID), rec->type))
+ return -EIO;
+
inode->i_ino = be32_to_cpu(rec->dir.DirID);
inode->i_size = be16_to_cpu(rec->dir.Val) + 2;
HFS_I(inode)->fs_blocks = 0;
@@ -565,13 +571,18 @@ static struct dentry *hfs_file_lookup(struct inode *dir, struct dentry *dentry,
res = hfs_brec_read(&fd, &rec, sizeof(rec));
if (!res) {
struct hfs_iget_data idata = { NULL, &rec };
- hfs_read_inode(inode, &idata);
+ res = hfs_read_inode(inode, &idata);
}
hfs_find_exit(&fd);
if (res) {
iput(inode);
return ERR_PTR(res);
}
+
+ if (is_bad_inode(inode)) {
+ iput(inode);
+ return ERR_PTR(-EIO);
+ }
HFS_I(inode)->rsrc_inode = dir;
HFS_I(dir)->rsrc_inode = inode;
igrab(dir);
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 97546d6b41f47..933aa04480899 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -372,6 +372,11 @@ static int hfs_fill_super(struct super_block *sb, struct fs_context *fc)
if (!root_inode)
goto bail_no_root;
+ if (is_bad_inode(root_inode)) {
+ iput(root_inode);
+ goto bail_no_root;
+ }
+
set_default_d_op(sb, &hfs_dentry_operations);
res = -ENOMEM;
sb->s_root = d_make_root(root_inode);
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH AUTOSEL 6.18-6.12] Revert "namei: don't reveal overmounted entries in refwalk"
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18] hfs: validate catalog CNIDs before instantiating inodes Sasha Levin
@ 2026-10-19 13:46 ` Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18] fs/namespace: notify pollers of legacy propagation changes Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] hfsplus: validate extent record length before writing it back Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-19 13:46 UTC (permalink / raw)
To: patches, stable
Cc: Christian Brauner, Sasha Levin, viro, linux-fsdevel, linux-kernel
From: Christian Brauner <brauner@kernel.org>
[ Upstream commit 214897acc8c2e5d6b63189d23ccaed6fa2af45a9 ]
This reverts commit 8189ab688f5a2c745dc90b63374da02f213d6994.
This bug class is bigger than can be fixed with this simple patch.
Back out of it and give us some time to fix this properly.
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This revert should go into this 6.18.57 tree. The incomplete
`unlink_may_reveal()` check is already in v6.18.57, the VFS maintainer
pulled it back as an rc8 fix, and leaving it here regresses common
unlink/open races.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
Step 1.1: PARSE THE SUBJECT LINE
Record: `[vfs/namei] [Revert] undo "don't reveal overmounted entries in
refwalk"`
Step 1.2: PARSE ALL COMMIT MESSAGE TAGS
Record:
- No `Fixes:` tag on the revert itself (it names the reverted SHA in the
body)
- No `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`, `Link:`,
or `Cc: stable`
- `Signed-off-by: Christian Brauner <brauner@kernel.org>` (VFS
maintainer; ignore Sasha Levin SOB)
- Body: `This reverts commit 8189ab688f5a2c745dc90b63374da02f213d6994`
The original commit that is being reverted **did** have `Fixes:
8ed936b5671b`, `Cc: stable@vger.kernel.org`, and `Link:
https://patch.msgid.link/20261002-work-mount-
fixes-4-v1-2-dd44b89d44ce@kernel.org`. That is why 8189ab68 already
landed in this stable series; the revert is the follow-up.
Step 1.3: ANALYZE THE COMMIT BODY TEXT
Record:
- Bug description: the earlier “simple patch” does not cover this bug
class
- Symptom/failure mode: not a crash; an incomplete pathwalk/mount-cover
check that must be withdrawn
- Version information: none in the revert message
- Root cause (author): “This bug class is bigger than can be fixed with
this simple patch. Back out of it and give us some time to fix this
properly.”
Linus’s `vfs-7.3-rc8.fixes` pull restates the same thing: *Revert
"namei: don't reveal overmounted entries in refwalk" from the rc7 fixes.
The bug class is bigger than that patch.*
Step 1.4: DETECT HIDDEN BUG FIXES
Record: Not disguised cleanup. This is an explicit revert of a recently
merged, incomplete VFS change. A later (unmerged) follow-up documented a
real regression: any unlink/rename racing a refwalk returned `-ESTALE`
and forced a full `LOOKUP_REVAL` walk, which on NFS/FUSE revalidates
every path component.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
Step 2.1: INVENTORY THE CHANGES
Record:
- File: `fs/namei.c` only
- Mainline revert: remove `unlink_may_reveal()` (~27 lines) and its one
call site (2 lines). Net −29
- Local 6.18.y equivalent of the original add is `5f97df8ca7fad`; the
revert is its inverse
- Functions: delete `unlink_may_reveal`; one call removed from
`step_into()` (this tree) / `step_into_slowpath()` (mainline after a
later split)
- Scope: single-file surgical revert
Step 2.2: UNDERSTAND THE CODE FLOW CHANGE
Record:
- Before (v6.18.57): in the non-RCU / refwalk arm of `step_into()`,
after `handle_mounts()`, `unlink_may_reveal()` returns true if the
dentry is `cant_mount()` **and** `d_unlinked()`, the walk has not
crossed a mount, and flags are not `WALK_NOFOLLOW`/`LOOKUP_REVAL`. It
`dput()`s `path->dentry` and the caller returns `-ESTALE`.
- After: that check is gone; refwalk accepts the dentry as it did before
5f97.
- Path: common pathname lookup (`open`/`stat`/`lookup`), refwalk only
(RCU walk already retries via `d_seq`/`mount_lock`).
Step 2.3: IDENTIFY THE BUG MECHANISM
Record:
- Category: revert of an incomplete logic/correctness change that also
caused a lookup-retry regression
- Mechanism: `dont_mount()` runs on **every** successful `vfs_unlink()`
/ `vfs_rmdir()` / rename-over, not only mountpoints. Combined with
`d_unlinked()`, the helper treated ordinary unlinks as “may reveal a
covered file” and forced `-ESTALE` → `LOOKUP_REVAL`. That is far
broader than the mount-cover race it meant to close, and it still did
not cover the rest of the bug class.
Verified `dont_mount()` call sites in this tree: `vfs_rmdir()` ~4558,
`vfs_unlink()` ~4679, `vfs_rename()` ~5230.
Verified ESTALE retry in this tree:
```2708:2709:fs/namei.c
if (unlikely(retval == -ESTALE))
retval = path_lookupat(&nd, flags | LOOKUP_REVAL, path);
```
Same pattern in `do_filp_open()` / `do_file_open_root()`. The helper
itself skipped the check when `LOOKUP_REVAL` was already set, which
matches the “fail once, then full revalidate” behavior.
Step 2.4: ASSESS THE FIX QUALITY
Record:
- Quality: obviously correct as a revert — it is the exact inverse of
5f97 in this tree
- Regression risk of the revert: re-opens the remaining refwalk window
where a walker that already held the dentry can see a file after its
covering mounts were detached. That race is old (Fixes target
8ed936b5671b, 2014). 6.18.y lived with it until 6.18.57. A sibling
backport, `d29cd99db3db7` (“unhash a dentry before detaching the
mounts”), remains and still shrinks the *new lookup* side of the race.
- Risk of keeping 5f97: extra `LOOKUP_REVAL` on common unlink/create
storms; incomplete security fix in core pathwalk
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
Step 3.1: BLAME THE CHANGED LINES
Record: At v6.18.57, every `unlink_may_reveal` line and the
`step_into()` call are `5f97df8ca7fad` (Christian Brauner, 2026-10-02;
committed to 6.18.y by Greg Kroah-Hartman on 2026-10-15). Surrounding
`step_into()` logic is old Al Viro/Linus code (2011–2022).
Step 3.2: FOLLOW THE FIXES: TAG
Record: The revert has no Fixes tag. It reverts mainline `8189ab688f5a`
= local `5f97df8ca7fad`.
- `git merge-base --is-ancestor 5f97df8ca7fad v6.18.57` → yes
(v6.18.57~479)
- `git merge-base --is-ancestor 5f97df8ca7fad v6.18.56` → no
- Original 8189ab68 `Fixes: 8ed936b5671b` **is** an ancestor of this
tree (2014-10-09)
Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES
Record:
- `d29cd99db3db7 namespace: unhash a dentry before detaching the mounts
on it` is also in v6.18.57 (same Greg KH batch, same original series).
It stays; this revert does not undo it.
- No other `unlink_may_reveal` / `DCACHE_UNCOVERED` commits in this tree
- Standalone revert, not patch 2/N of something this tree still needs
Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS
Record: Christian Brauner is the VFS maintainer. Recent `fs/namei.c`
work in this history includes the original add, this revert, and other
VFS fixes. Weight: high.
Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS
Record: The only prerequisite is that 5f97 exists, which it does. The
revert applies as the exact inverse of 5f97 (`step_into`, not mainline’s
later `step_into_slowpath`). No new helpers. Standalone.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION
Record:
- `b4 dig -c 214897acc8c2` failed: lore.kernel.org returned errors
(UNVERIFIED: no dedicated mailed revert thread found; Brauner likely
committed this directly)
- WebFetch of lore.kernel.org returned 403 (UNVERIFIED: full lore
thread)
- Original add was series `work-mount-fixes-4` patch 02/21 (msgid
`20261002-work-mount-fixes-4-v1-2-dd44b89d44ce@kernel.org`); also on
lkml.iu.edu / patchew
- Linus merged the revert in `1dbb7422053fa` *Merge tag
'vfs-7.3-rc8.fixes'* and called it out in the pull text
- An unmerged follow-up (syzbot CI series) explained the LOOKUP_REVAL
regression and tried `DCACHE_UNCOVERED`; that approach was **not**
merged — they reverted instead
Step 4.2: CHECK WHO REVIEWED THE PATCH
Record: `b4 dig -w` failed (lore error). Author is the VFS maintainer;
Linus merged it as an rc8 VFS fix. That is stronger than a typical
mailing-list Ack.
Step 4.3: SEARCH FOR THE BUG REPORT
Record: No syzbot/bugzilla on the revert. The **original** add included
a userspace reproducer (`unlink_covered`) showing the mount-cover race
(~27 hits/60s without widening). The unmerged follow-up’s `unlink_storm`
reproducer showed 463 LOOKUP_REVAL retries in 16.8 million opens from
the too-broad check.
Step 4.4: CHECK FOR RELATED PATCHES AND SERIES
Record: Original add was 02/21 of a large mount-fixes series. Sibling
`unhash a dentry before detaching` is already in 6.18.57 and is **not**
reverted. No merged “proper” replacement for `unlink_may_reveal` exists
in this tree or in the nearby mainline objects inspected.
Step 4.5: CHECK STABLE MAILING LIST HISTORY
Record: lore.kernel.org/stable search could not be fetched (403).
UNVERIFIED whether anyone separately requested this revert on the stable
list. Not needed: the original went to 6.18.57 via `Cc: stable`, and the
author immediately backed it out in mainline.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF
Record: `unlink_may_reveal` (removed), `step_into` (call removed). No
semcode tools available.
Step 5.2: TRACE CALLERS
Record: `step_into()` callers in this tree:
- `handle_dots()` (WALK_NOFOLLOW — helper returned false anyway)
- `walk_component()` — every normal path component
- one more WALK_NOFOLLOW site ~2660
- `open_last_lookups` path ~3922 (`WALK_TRAILING`)
Impact surface: **every pathname lookup** that drops out of RCU-walk
into refwalk.
Step 5.3: TRACE CALLEES
Record: Helper used `cant_mount()`, `d_unlinked()`,
`dput(no_free_ptr(path->dentry))`. Returning `-ESTALE` skipped the
normal `dput(nd->path.dentry)` / `mntput` in `step_into()`;
`terminate_walk()` still owns `nd->path`. The extra `dput` was only on
the ESTALE path.
Step 5.4: FOLLOW THE CALL CHAIN
Record: userspace `open`/`openat`/`stat`/`fstatat`/etc. →
`filename_lookup` / `do_filp_open` → `link_path_walk` → `walk_component`
→ `step_into`. Unprivileged users trigger this constantly. The false
ESTALE path is a realistic unlink vs open race, not only the exotic
mount-cover case.
Step 5.5: SEARCH FOR SIMILAR PATTERNS
Record: No other `unlink_may_reveal` copies. `cant_mount()` remains in
`fs/namespace.c` and `fs/dcache.c` for mount setup, which is correct and
unrelated.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?
Record: Yes, in **v6.18.57** (`fs/namei.c` `unlink_may_reveal` + call in
`step_into`). Tree identity: `git describe HEAD` =
`v6.18.57-155-gcdf239ca885b2`, `make kernelversion` = 6.18.57.
Introduced here by 5f97 (upstream 8189ab68), first in 6.18.57, not
6.18.56.
Step 6.2: CHECK FOR BACKPORT COMPLICATIONS
Record: Clean apply. Local revert hunks match 5f97 exactly (`step_into`,
not `step_into_slowpath`). No conflicting namei refactor between 5f97
and v6.18.57.
Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE
Record: No later replacement for this check. `d29cd99` (unhash before
detach) is present and complementary; it must stay. This revert does not
remove it.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY
Record: `fs/namei.c` — VFS pathname lookup. **CORE**. Affects every user
of every filesystem.
Step 7.2: ASSESS SUBSYSTEM ACTIVITY
Record: Mature, still actively fixed (recent mount/namespace series).
This is not staging/experimental code.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
Step 8.1: DETERMINE WHO IS AFFECTED
Record: Universal for anyone running 6.18.57+. NFS/FUSE users feel the
LOOKUP_REVAL retry cost most. Container/user-ns users care about the
original mount-cover race; that race is only partially addressed by
d29cd99 and was not fully fixed by 5f97.
Step 8.2: DETERMINE THE TRIGGER CONDITIONS
Record:
- False ESTALE: concurrent unlink/rename vs refwalk on any name
(`dont_mount` is unconditional). Common.
- Original cover-reveal race: unlink of a mountpoint while another task
walks the covering name. Narrow but real; original commit measured 27
hits/60s.
- Unprivileged users can trigger both (open/unlink). The cover case
typically needs mount privileges or a shared mount namespace.
Step 8.3: DETERMINE THE FAILURE MODE SEVERITY
Record:
- Keeping 5f97: extra full-path revalidation (MEDIUM–HIGH on NFS/FUSE;
LOW on local tmpfs except under unlink storms). Incomplete mount-cover
fix (the author says the class is bigger).
- After revert: restore pre-6.18.57 `step_into` behavior; remaining
cover-reveal race is the long-standing one (MEDIUM, security-adjacent,
narrow), with d29cd99 still reducing new-lookup hits.
- Severity of **not** reverting a known-wrong core pathwalk change that
already shipped in 6.18.57: HIGH (stable regression vs 6.18.56).
Step 8.4: CALCULATE RISK-BENEFIT RATIO
Record:
- Benefit: remove a too-broad ESTALE/LOOKUP_REVAL regression from core
lookup; align 6.18.y with the VFS maintainer’s rc8 correction; drop an
incomplete security bandage rather than pretend it closed the class
- Risk: very low — exact inverse of a 29-line function added in 6.18.57
- Ratio: strongly favors backporting the revert
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
Step 9.1: COMPILE THE EVIDENCE
FOR:
- The commit being reverted **is in this tree** (5f97, v6.18.57~479)
- VFS maintainer authored the revert; Linus merged it as
vfs-7.3-rc8.fixes
- Documented too-broad check: every unlink/rename vs refwalk, not only
overmounts
- Small, exact inverse, applies cleanly
- Sibling unhash-before-detach fix remains
- Revert of a stable-to-stable mistake (5f97 was itself a 6.18.57
backport)
AGAINST:
- Re-opens the remaining “walker already holds the dentry” cover-reveal
window
- Original add had a reproducer and `Cc: stable`
UNRESOLVED:
- Full lore thread for the revert itself (b4/lore failed)
- Whether a later proper fix exists only on branches not in this object
store (none found on 214897’s nearby history)
The “against” point is why 5f97 was written. The maintainer’s later
judgment, merged by Linus, is that keeping that patch is worse than
living with the old narrow race until a real fix exists. For stable,
shipping 5f97 for a few days and then following the revert is the
conservative choice.
Step 9.2: APPLY THE STABLE RULES CHECKLIST
1. Obviously correct and tested? **PASS** — exact revert of a known SHA;
merged as an rc8 VFS fix
2. Fixes a real bug that affects users? **PASS** — removes a too-broad
ESTALE/LOOKUP_REVAL pathwalk regression introduced in 6.18.57
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
core VFS lookup regression; incomplete security check is worse than
no check plus the remaining unhash fix
4. Small and contained? **PASS** — −29 lines, one file
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — inverse of 5f97 on
`step_into()`
Step 9.3: CHECK FOR EXCEPTION CATEGORIES
Record: Not a device-ID/quirk/DT/docs case. It is the documented revert
case: original commit **is** in this stable tree, so the revert is
needed.
Step 9.4: MAKE YOUR DECISION
This tree is 6.18.57. It already contains the incomplete refwalk check.
The revert is the maintainer’s correction of that 6.18.57 backport and
should be applied here.
What it does for stable users: it takes `unlink_may_reveal()` out of
`step_into()` so ordinary unlink/open races no longer force a full
`LOOKUP_REVAL` walk, and it stops treating a known-incomplete mount-
cover check as a finished fix. `d29cd99` still unhashes before detaching
mounts.
Risk vs benefit: benefit is high (undo a core-pathwalk regression that
is only days old in 6.18.57); risk is very low (surgical revert).
Concerns: the original cover-reveal race is not fully solved; that is
intentional per the author. No extra prerequisites.
Verification:
- [Phase 1] Parsed revert tags: Brauner SOB only; body reverts
8189ab688f5a; original add had Fixes/Cc stable/Link
- [Phase 1] Linus vfs-7.3-rc8.fixes pull explicitly names this revert
- [Phase 2] Diff: −29 lines in fs/namei.c; removes unlink_may_reveal +
ESTALE return in step_into
- [Phase 2] Verified dont_mount() on vfs_unlink/vfs_rmdir/vfs_rename;
ESTALE→LOOKUP_REVAL in filename_lookup/do_filp_open
- [Phase 3] git describe HEAD: v6.18.57-155-gcdf239ca885b2;
kernelversion 6.18.57
- [Phase 3] git merge-base --is-ancestor 5f97df8ca7fad v6.18.57: yes;
v6.18.56: no
- [Phase 3] git blame v6.18.57: unlink_may_reveal entirely 5f97df8ca7fad
- [Phase 3] 8ed936b5671b is an ancestor; d29cd99 unhash-before-detach is
in v6.18.57 and stays
- [Phase 3] Local revert is the exact inverse of 5f97 on step_into (not
step_into_slowpath)
- [Phase 4] b4 dig -c 214897acc8c2: lore server error — UNVERIFIED
mailed thread
- [Phase 4] lore WebFetch 403 — UNVERIFIED lore discussion
- [Phase 4] Unmerged DCACHE_UNCOVERED follow-up documents LOOKUP_REVAL
regression (syzbot CI / series text)
- [Phase 5] step_into callers: walk_component, handle_dots,
open_last_lookups — core pathwalk
- [Phase 6] Buggy helper present at v6.18.57:fs/namei.c:1983; clean
apply
- [Phase 8] Failure mode: extra LOOKUP_REVAL on unlink vs open; NFS/FUSE
revalidate whole path
- UNVERIFIED: stable@ mailing list discussion of this specific revert
**YES**
fs/namei.c | 29 -----------------------------
1 file changed, 29 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 2e86733b4bacb..003c9a1b2f904 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1973,33 +1973,6 @@ static const char *pick_link(struct nameidata *nd, struct path *link,
return NULL;
}
-/*
- * Be careful in case the dentry is unlinked or renamed. Any mounts
- * stacked on top of it are going away. We need to make sure that we
- * don't reveal the underyling dentry during refwalk. In rcuwalk we
- * catch this via d_seq and another lookup for the name. Give the same
- * guarantee in refwalk.
- */
-static bool unlink_may_reveal(struct nameidata *nd, int flags,
- struct path *path, struct dentry *dentry)
-{
- /* ".." and LOOKUP_DOWN may land on an unhashed directory */
- if (flags & WALK_NOFOLLOW)
- return false;
- if (nd->flags & LOOKUP_REVAL)
- return false;
- /* We crossed onto a mount and the name led us here while it still existed */
- if (path->mnt != nd->path.mnt)
- return false;
- /* only a name on its way out is flagged */
- if (likely(!cant_mount(dentry)))
- return false;
- if (!d_unlinked(dentry))
- return false;
- dput(no_free_ptr(path->dentry));
- return true;
-}
-
/*
* Do we need to follow links? We _really_ want to be able
* to do this check without having to look at inode->i_op,
@@ -2028,8 +2001,6 @@ static const char *step_into(struct nameidata *nd, int flags,
if (unlikely(!inode))
return ERR_PTR(-ENOENT);
} else {
- if (unlikely(unlink_may_reveal(nd, flags, &path, dentry)))
- return ERR_PTR(-ESTALE);
dput(nd->path.dentry);
if (nd->path.mnt != path.mnt)
mntput(nd->path.mnt);
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH AUTOSEL 6.18] fs/namespace: notify pollers of legacy propagation changes
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18] hfs: validate catalog CNIDs before instantiating inodes Sasha Levin
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.12] Revert "namei: don't reveal overmounted entries in refwalk" Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] hfsplus: validate extent record length before writing it back 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: Guopeng Zhang, Christian Brauner (Amutable), Sasha Levin, viro,
linux-fsdevel, linux-kernel
From: Guopeng Zhang <zhangguopeng@kylinos.cn>
[ Upstream commit 0ae44a20835e5cf37d03a83e70c92e92deda3703 ]
Changing mount propagation through the legacy mount API changes
user-visible mountinfo contents, including the shared: and master:
optional fields.
The mount_setattr() path already touches the mount namespace after
change_mnt_propagation(), so pollers of /proc/<pid>/mountinfo are woken
when the namespace event changes.
The legacy mount --make-* path also changes propagation through
change_mnt_propagation(), and MOVE_MOUNT_SET_GROUP updates the
propagation relationship of the target mount. Both paths currently
return without touching the affected mount namespace.
As a result, userspace polling /proc/<pid>/mountinfo can miss these
propagation-only changes even though mountinfo has changed.
A simple reproducer that polls /proc/self/mountinfo while changing
propagation shows the inconsistency.
Before this change:
legacy MS_SHARED: poll ret=0 revents=0x0
mount_setattr MS_SHARED: poll ret=1 revents=0xa
After this change:
legacy MS_SHARED: poll ret=1 revents=0xa
mount_setattr MS_SHARED: poll ret=1 revents=0xa
Fix this by touching the affected mount namespace after successful
propagation changes in do_change_type() and do_set_group().
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Link: https://patch.msgid.link/20260601032911.940507-1-guopeng.zhang@linux.dev
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have enough to decide: the miss is real in 6.18.53, the helpers exist,
and the patch is a small ABI wake-up fix. Below is the full 9-phase
write-up.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: `[fs/namespace] [notify] notify pollers of legacy propagation
changes`
Subsystem is VFS mount-namespace (`fs/namespace.c`). Verb is “notify”,
which here means adding a missing userspace wake-up after a state
change.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Signed-off-by:** Guopeng Zhang `<zhangguopeng@kylinos.cn>` (author)
- **Link:** `https://patch.msgid.link/20260601032911.940507-1-
guopeng.zhang@linux.dev`
- **Signed-off-by:** Christian Brauner (Amutable) `<brauner@kernel.org>`
(VFS maintainer; applied the patch)
- **No Fixes: tag** (expected for this review queue)
- **No Cc: stable** (expected)
- **No Reported-by / Tested-by / Reviewed-by / Acked-by**
- Pipeline-only tags (`[Upstream commit …]`, `Signed-off-by: Sasha
Levin`) ignored per instructions
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `mount(MS_SHARED|MS_PRIVATE|MS_SLAVE|MS_UNBINDABLE)`
(`do_change_type`) and `move_mount(MOVE_MOUNT_SET_GROUP)`
(`do_set_group`) change propagation (and therefore
`/proc/<pid>/mountinfo` `shared:` / `master:` / `unbindable` fields)
but never call `touch_mnt_namespace()`.
- **Symptom:** `poll()` on `/proc/self/mountinfo` returns 0 /
`revents=0` after a legacy `--make-*` change. The same change via
`mount_setattr()` already wakes pollers (`revents=0xa` =
`EPOLLERR|EPOLLPRI`).
- **Failure mode:** userspace that waits on mountinfo can miss a real,
user-visible table change.
- **Version info:** none in the message. Bug is in the legacy `mount()`
path, which has existed since shared-subtree days.
- **Root cause (author):** those two paths return after
`change_mnt_propagation()` / SET_GROUP list updates without bumping
`ns->event` or waking `ns->poll`.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. It is an explicit correctness/ABI fix:
pollers must be woken when mountinfo contents change.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `fs/namespace.c` only (`+7` lines, no deletions)
- **Functions:** `do_change_type()`, `do_set_group()`
- **Scope:** single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Hunk 1 (`do_change_type`):** after the `change_mnt_propagation()`
walk, previously returned 0 with no namespace event. After: take
`guard(mount_locked_reader)()`, then
`touch_mnt_namespace(mnt->mnt_ns)`, then return 0. Normal success path
of `mount(--make-shared/slave/private/unbindable)`. Error paths
unchanged.
- **Hunk 2 (`do_set_group`):** after copying slave/share linkage onto
`to`, previously returned 0. After: same lock +
`touch_mnt_namespace(to->mnt_ns)`. Normal success path of
`MOVE_MOUNT_SET_GROUP`. Error paths unchanged.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** logic/correctness + userspace ABI (missing poll wake-up)
- **Mechanism:** `touch_mnt_namespace()` does `ns->event = ++event` and
`wake_up_interruptible(&ns->poll)`. `mounts_poll()` in
`fs/proc_namespace.c` waits on that queue and sets `EPOLLERR|EPOLLPRI`
when `ns->event` changes. `show_mountinfo` prints `shared:%i`,
`master:%i`, `propagate_from:%i`, `unbindable` from the same
propagation state. Those fields change without a corresponding event,
so pollers keep a stale view.
- Locking: v1 used `lock_mount_hash()` (`write_seqlock`), which bumps
`mount_lock` and forces seqcount readers to retry. Christian asked for
`guard(mount_locked_reader)()` (`read_seqlock_excl`) so topology
seqcount readers are not spuriously invalidated. That is the committed
form.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct. Same helper already used by
`mount_setattr_commit()` after `change_mnt_propagation()`.
- **Minimal:** two call sites, no API or structural change.
- **Regression risk:** low. Extra exclusive seqlock after
`namespace_excl` matches existing lock order (`namespace_sem` then
`mount_lock`). Wake-up is after a successful mutation only. Using the
reader-exclusive guard avoids extra `mount_lock` sequence bumps, which
is why v1 was revised.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Propagation loop in `do_change_type` dates to Al Viro / Miklos Szeredi
(2008–2011); `do_change_type` itself goes back to shared-subtree
(`07b20889e3052`, contained in v2.6.15-rc1).
- `do_set_group` body is from Pavel Tikhomirov (`9ffb14ef61bab`,
contained in v5.15-rc1).
- Neither path ever called `touch_mnt_namespace()` until this commit.
- `mount_setattr_commit()` has called `touch_mnt_namespace()` since
`e257039f0fc7da` / `2a1867219c7b2` (mount_setattr, contained in
v5.12-rc1). That is the inconsistency the author measured.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Not applicable. The missing wake-up is original
behavior of `do_change_type`, not a recent regression.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Related: `12cdd1af7a6d1 do_change_type(): use guards`, `7b99ee2c5c83b
do_set_group(): use guards` (already in v6.18.53; this patch sits on
that form).
- `cffd0441872e7` uniform permission checks; `d8cc0362f918d` SET_GROUP
graph fix — adjacent but not prerequisites for this wake-up.
- Standalone. Not `patch X/Y`.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Guopeng Zhang is not the VFS maintainer. Other `master` commits
under that name are mostly cgroup/docs. This mount patch was applied by
Christian Brauner to `vfs-7.3.mount`.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- Needs `touch_mnt_namespace()`, `guard(mount_locked_reader)()`,
`do_change_type()`, `do_set_group()`. All exist at v6.18.53
(`mount_locked_reader` from `d154f18575899`, contained in the 6.18
cycle).
- Applies standalone. No follow-up in this file after 2026-06-01 that
amends this wake-up.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig:** https://patch.msgid.link/20260601032911.940507-1-
guopeng.zhang@linux.dev
- **v1:** 2026-05-29, https://patch.msgid.link/20260529095441.1744006-1-
guopeng.zhang@linux.dev
- **v2:** 2026-06-01 (committed version)
- Christian on v1: `lock_mount_hash()` would make seqcount readers retry
on propagation-only changes; use `guard(mount_locked_reader)()`
instead. Author agreed and sent v2.
- No NAK. Christian applied v2 to `vfs/vfs.git` branch `vfs-7.3.mount`
(commit `0ae44a20835e`).
- No reviewer `Cc: stable` in the thread.
- lore HTTP fetch returned 403; threads were retrieved with `b4 dig -m`
/ `b4 mbox`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (`b4 dig -w`): To/Cc Alexander Viro, Christian Brauner, Jan Kara,
linux-fsdevel, linux-kernel. Brauner reviewed v1 and merged v2. Viro and
Kara were CCed; no extra Acked-by in the mbox.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No syzbot, bugzilla, or extra reporters. Evidence is the
author’s side-by-side poll reproducer (`legacy MS_SHARED: poll ret=0` vs
`mount_setattr: poll ret=1`). Severity from that: userspace misses a
mountinfo update. How many users: not quantified; the interfaces (`mount
--make-*`, `MOVE_MOUNT_SET_GROUP`, poll on mountinfo) are used by
systemd/libmount, `findmnt --poll`, and container runtimes.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Single patch, v1→v2 lock change only. Not part of a larger
series.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search via WebFetch was 403. No stable
discussion was found in the fsdevel mboxes. UNVERIFIED whether anyone
previously requested or rejected this for -stable.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
semcode MCP is not available. Callers/callees were traced with grep and
by reading the functions.
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `do_change_type`, `do_set_group`; they call
`touch_mnt_namespace` and `guard(mount_locked_reader)`.
**Step 5.2: TRACE CALLERS**
Record:
- `do_change_type()` ← `path_mount()` when flags include
`MS_SHARED|MS_PRIVATE|MS_SLAVE|MS_UNBINDABLE` ← `do_mount()` /
`mount()` syscall.
- `do_set_group()` ← `vfs_move_mount()` when `MNT_TREE_PROPAGATION` ←
`move_mount()` with `MOVE_MOUNT_SET_GROUP`.
- `touch_mnt_namespace()` also used from `commit_tree`,
`set_mount_attributes`, `mount_setattr_commit`, umount/expiry paths.
- `mounts_poll()` is `.poll` for `/proc/<pid>/mounts` and
`/proc/<pid>/mountinfo`.
**Step 5.3: TRACE CALLEES**
Record: `change_mnt_propagation()` (under `namespace_sem` exclusive),
`invent_group_ids()`, `may_change_propagation()`, then the new lock +
`touch_mnt_namespace()` (`ns->event++`, `wake_up_interruptible`). No
allocation on the added path.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace-reachable via `mount(2)` and `move_mount(2)` with
`CAP_SYS_ADMIN` in the relevant namespace (`may_change_propagation`).
Unprivileged user+mount namespaces (containers) can hit it. Pollers of
procfs mountinfo are the observers. This is a common container/systemd
path, not an obscure config.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: `mount_setattr_commit()` already does `change_mnt_propagation()`
then `touch_mnt_namespace()`. Historical `0e55a7cca4b66`
(“touch_mnt_namespace when the mount flags change”) and `5addc5dd8836a`
(make `/proc/mounts` pollable) are the same contract: if
mountinfo/mounts text changes, bump the event. This commit fills the
remaining propagation-only hole.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Local tree: **6.18.53** (`Makefile` VERSION=6 PATCHLEVEL=18
SUBLEVEL=53; `git describe HEAD` = `v6.18.53-5049-g0f0bdc2b0dffb`;
branch `autosel`).
- At **v6.18.53**, `do_change_type()` and `do_set_group()` return after
propagation updates with **no** `touch_mnt_namespace()`. The bug is
present.
- The autosel cherry-pick (`3e32bcbd6febbb`) is ignored as pipeline
state, not as “already selected.”
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** to v6.18.53. Exact context matches both hunks.
`guard(mount_locked_reader)()` and `touch_mnt_namespace()` are already
in this series.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: `mount_setattr_commit()` already notifies. No earlier equivalent
fix for the legacy/`SET_GROUP` paths at v6.18.53.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **VFS mount namespace** (`fs/namespace.c`,
`fs/proc_namespace.c`). **CORE** for anyone using mounts; poll on
mountinfo is a long-standing userspace ABI.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively maintained (Brauner/Viro). The bug is old; the new
mount API made the missing wake-up obvious by contrast.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: Anyone polling `/proc/*/mountinfo` or `/proc/*/mounts` while
another task changes propagation via legacy `mount()` or
`MOVE_MOUNT_SET_GROUP`. Typical: systemd/libmount monitors, `findmnt
--poll`, container runtimes. Not universal on every syscall, but not a
rare driver either.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Trigger is `mount --make-shared/slave/private/unbindable`
(optionally `MS_REC`) or `move_mount(..., MOVE_MOUNT_SET_GROUP)` while a
poller is waiting. Needs mount-admin capability in the namespace.
Unprivileged in a user+mount ns. Not a race in the kernel sense; it is a
missing notification on a fully deterministic path. Common during
container setup; easy to miss if the same process both changes
propagation and does not poll.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- Failure: poll does not wake; userspace keeps a stale view of
`shared:`/`master:`/`unbindable` until some later unrelated mount
event.
- Actual propagation in the kernel is correct; this is not a graph
corruption bug.
- **Severity: MEDIUM.** Userspace ABI miss, not
oops/deadlock/corruption. Can make mount monitors and orchestrators
act on stale topology.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** medium. Restores the documented poll contract for a
change that is already visible in mountinfo, and matches
`mount_setattr()`.
- **Risk:** very low. +7 lines, existing helper, lock form chosen by the
VFS maintainer specifically to avoid extra seqcount retries.
- **Ratio:** favorable for 6.18.y.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**For backporting:**
- Real, reproduced bug: legacy `MS_SHARED` poll ret=0 vs `mount_setattr`
poll ret=1.
- User-visible `/proc/<pid>/mountinfo` contents change (`shared:`,
`master:`, `unbindable`) without a poll event.
- Same subsystem already notifies on the `mount_setattr()` path; this
closes the hole for `mount()` and `MOVE_MOUNT_SET_GROUP`.
- 7-line, obviously correct change; VFS maintainer reviewed locking and
merged v2.
- Bug and helpers exist in this 6.18.53 tree; patch applies cleanly.
- Reachable from `mount(2)` / `move_mount(2)`; pollable proc files have
been an ABI since `5addc5dd8836a`.
**Against backporting:**
- Not a crash, deadlock, data-corruption, or security fix.
- No extra user/syzbot reports; author found it by comparing APIs.
- `do_change_type` has omitted this wake-up since shared-subtree
(~2.6.15) without a flood of reports.
- No `Cc: stable` in review (absence is expected, not a veto).
- Importance is “oh, that’s not good” ABI correctness, not oops-class.
**Unresolved:**
- Could not query lore.kernel.org/stable over HTTP (403). No evidence
the patch was previously rejected for stable.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — matches
`mount_setattr_commit`; v2 lock change from Brauner; author showed
before/after poll results.
2. Fixes a real bug that affects users? **PASS** — pollers of mountinfo
miss propagation-only updates; documented proc poll ABI
(`5addc5dd8836a`, `mounts_poll`).
3. Important issue? **PASS** (medium / “oh, that’s not good”) — stale
mount-table view for systemd/libmount/container monitors; not oops-
class, but it is a userspace-visible contract break on core VFS, not
trivia.
4. Small and contained? **PASS** — 7 lines, one file, two success-path
sites.
5. No new features or APIs? **PASS** — only wakes existing waiters.
6. Can apply to the local tree? **PASS** — v6.18.53 context matches;
`mount_locked_reader` and `touch_mnt_namespace` exist.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: None (not a device ID, quirk, DT, build, or docs fix). It is a
normal small ABI/correctness fix.
**Step 9.4: MAKE YOUR DECISION**
This should be backported to **this** 6.18.53 tree. The bug is present
here, the fix is small and already the pattern used by
`mount_setattr()`, and leaving poll silent while mountinfo
`shared:`/`master:` fields change is a real userspace-visible failure,
not a cleanup.
Stable rules in this tree explicitly allow “oh, that’s not good” issues.
A broken `poll()` on `/proc/<pid>/mountinfo` after `mount --make-*` is
that: monitors used in production can miss a topology change the kernel
already committed. Risk is negligible compared with that benefit.
===============================================================
What the commit solves for stable users
===============================================================
On 6.18.y, `mount --make-shared` (and related flags) and
`MOVE_MOUNT_SET_GROUP` update propagation and the text of mountinfo, but
do not increment `mnt_namespace->event` or wake `ns->poll`. Tools
waiting on that fd do not re-read, so they can keep stale `shared:` /
`master:` / `unbindable` state. `mount_setattr()` already notifies. The
patch adds the same `touch_mnt_namespace()` after the two missing
success paths, under `mount_locked_reader` so lookup seqcounts are not
bumped.
No extra commits are required. No new ABI. Regression risk is a spurious
extra wake (harmless) or the brief exclusive seqlock (same class as
other namespace operations).
===============================================================
Verification
===============================================================
- [Phase 1] Parsed message/tags: author Guopeng Zhang; Link msgid
20260601032911; SOB Christian Brauner; no Fixes/Reported-by/Cc-stable.
- [Phase 2] Diff: +7 lines in `fs/namespace.c`; `do_change_type` and
`do_set_group` success paths only.
- [Phase 2] Confirmed `touch_mnt_namespace()` increments `ns->event` and
wakes `ns->poll` (`fs/namespace.c` ~971–976).
- [Phase 2] Confirmed `mounts_poll()` waits on `ns->poll` and sets
`EPOLLERR|EPOLLPRI` on event change (`fs/proc_namespace.c` 21–37).
- [Phase 2] Confirmed mountinfo prints `shared:`/`master:`/`unbindable`
from that state (`fs/proc_namespace.c` 158–169).
- [Phase 2] Confirmed `mount_setattr_commit()` already calls
`touch_mnt_namespace()` after `change_mnt_propagation()` (line 4785).
- [Phase 3] `git describe HEAD` / `make kernelversion` / Makefile: this
tree is 6.18.53.
- [Phase 3] `git merge-base --is-ancestor 3e32bcbd6febbb v6.18.53`: not
in v6.18.53 (bug still there).
- [Phase 3] `git show v6.18.53:fs/namespace.c`: both functions return
without `touch_mnt_namespace`.
- [Phase 3] Blame: `do_change_type` loop 2008–2011; `do_set_group`
`9ffb14ef61bab` (v5.15-rc1); `git describe --contains 07b20889e3052` →
v2.6.15-rc1.
- [Phase 3] `mount_locked_reader` present at v6.18.53 (`d154f18575899` /
`fs/mount.h:164`).
- [Phase 3] Patch context matches v6.18.53 exactly
(`do_change_type`/`do_set_group` context match: True).
- [Phase 4] `b4 dig -c 0ae44a20835e`: v2 lore URL; `b4 dig -a`: v1
2026-05-29, v2 2026-06-01.
- [Phase 4] v1 mbox: Brauner requested `guard(mount_locked_reader)`
instead of `lock_mount_hash`; v2 applied to vfs-7.3.mount; no NAK.
- [Phase 4] `b4 dig -w`: Viro, Brauner, Jan Kara, linux-fsdevel CCed.
- [Phase 4] WebFetch lore/stable: 403; UNVERIFIED stable-list history.
- [Phase 5] Callers: `path_mount` → `do_change_type`; `vfs_move_mount` →
`do_set_group` for `MOVE_MOUNT_SET_GROUP` (UAPI in
`include/uapi/linux/mount.h`).
- [Phase 6] Local 6.18.53 has the bug; helpers present; clean apply.
- [Phase 8] Failure mode: missed poll on mountinfo, severity MEDIUM
(ABI), not oops.
- UNVERIFIED: whether systemd/libmount has a public bug for this
specific miss; no reporter besides the author.
The patch is small, already matches the `mount_setattr()` notification
path, and restores poll on a core proc file for a change that is already
user-visible in this 6.18.y tree.
**YES**
fs/namespace.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/fs/namespace.c b/fs/namespace.c
index 3b6d7271eb3d1..ab050a939cb53 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2932,6 +2932,9 @@ static int do_change_type(const struct path *path, int ms_flags)
for (m = mnt; m; m = (recurse ? next_mnt(m, mnt) : NULL))
change_mnt_propagation(m, type);
+ guard(mount_locked_reader)();
+ touch_mnt_namespace(mnt->mnt_ns);
+
return 0;
}
@@ -3414,6 +3417,10 @@ static int do_set_group(const struct path *from_path, const struct path *to_path
list_add(&to->mnt_share, &from->mnt_share);
set_mnt_shared(to);
}
+
+ guard(mount_locked_reader)();
+ touch_mnt_namespace(to->mnt_ns);
+
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH AUTOSEL 6.18-5.10] hfsplus: validate extent record length before writing it back
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18] fs/namespace: notify pollers of legacy propagation changes Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Jiaming Zhang, Viacheslav Dubeyko, Sasha Levin, frank.li,
linux-fsdevel, linux-kernel
From: Jiaming Zhang <r772577952@gmail.com>
[ Upstream commit 627b7865c062ff642c0000f3a4775f1a44b986a4 ]
__hfsplus_ext_write_extent() writes the cached extent record back into a
B-tree node using fd->entrylength as the length, and fd->entrylength is
derived in __hfs_brec_find() from two on-disk values:
fd->entrylength = len - keylen;
A crafted image can keep both len and keylen valid but make fd->entrylength
negative (keylen > len). __hfsplus_ext_write_extent() doesn't check
fd->entrylength before consuming it, and hfs_bnode_write() takes the length
as u32, so the negative value turns into a huge one. The copy then reads
data past the end of hip->cached_extents, which is only
sizeof(hfsplus_extent_rec) bytes long, and leaks kernel memory into the
image.
Reject an fd->entrylength that does not match sizeof(hfsplus_extent_rec) in
__hfsplus_ext_write_extent(), mirroring the check already performed in
__hfsplus_ext_read_extent().
Link: https://lore.kernel.org/lkml/cbd7003314c530d4f910eacf019ff80adad6687e.camel@dubeyko.com/
Signed-off-by: Jiaming Zhang <r772577952@gmail.com>
Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
Link: https://lore.kernel.org/r/20260810092422.1691377-1-r772577952@gmail.com
Signed-off-by: Viacheslav Dubeyko <slava@dubeyko.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 two-line write-side
length check is missing here, the buggy path is present, and the patch
applies cleanly.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[hfsplus] [validate] [reject a corrupt overflow-extent record
length before writing it back into the extents B-tree]**
- Subsystem: `hfsplus`
- Verb: “validate” — a missing safety check, not a feature
- Intent: stop `__hfsplus_ext_write_extent()` from using an on-disk
`entrylength` that does not match `sizeof(hfsplus_extent_rec)`
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Mainline commit: `627b7865c062ff642c0000f3a4775f1a44b986a4` (not in this
tree).
Record:
- **Fixes:** none (expected for this review)
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** Viacheslav Dubeyko `<slava@dubeyko.com>` (HFS/HFS+
maintainer)
- **Acked-by:** none
- **Link:** `https://lore.kernel.org/lkml/cbd7003314c530d4f910eacf019ff8
0adad6687e.camel@dubeyko.com/` (maintainer discussion that prompted
the patch)
- **Link:** `https://lore.kernel.org/r/20260810092422.1691377-1-
r772577952@gmail.com` (patch submission)
- **Cc: stable:** none (expected)
- **Signed-off-by:** Jiaming Zhang (author); Viacheslav Dubeyko
(maintainer)
Notable: the sent patch CCs `syzkaller@googlegroups.com`. No explicit
syzbot hash in the committed message.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `__hfs_brec_find()` sets `fd->entrylength = len - keylen`
from two on-disk u16s. A crafted node can keep both values “valid”
while `keylen > len`, so `entrylength` is negative.
- **Symptom:** `__hfsplus_ext_write_extent()` passes that signed length
into `hfs_bnode_write()`, which takes `u32 len`. The negative value
becomes a huge length. The copy reads past `hip->cached_extents` (only
`sizeof(hfsplus_extent_rec)` bytes) and **writes kernel memory into
the filesystem image**.
- **Version info:** none in the message. The write site has existed
since early HFS+ code; `hfs_bnode_write()` taking `u32` is already in
this 6.12 tree.
- **Root cause:** write path never checks `entrylength`; the read path
already does.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** This is an explicit memory-safety / info-leak
fix, described as such.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `fs/hfsplus/extents.c` — **+2 / -0**
- **Function:** `__hfsplus_ext_write_extent()`
- **Scope:** single-file surgical fix (two lines)
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** on the existing-record path (`!(extent_state &
HFSPLUS_EXT_NEW)`), a successful `hfs_brec_find()` is followed
immediately by `hfs_bnode_write(..., fd->entryoffset,
fd->entrylength)`.
- **After:** if `fd->entrylength != sizeof(hfsplus_extent_rec)`, return
`-EIO` and do not write.
- **Path:** overflow-extent **update** (not the `HFSPLUS_EXT_NEW` insert
path, which already passes `sizeof(hfsplus_extent_rec)`).
- Insert path is unchanged.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** memory safety — out-of-bounds read of kernel memory,
leaked into the on-disk B-tree node (info leak + image corruption).
Also a logic/correctness gap: write did not mirror the read-side check
that has existed since `1da177e4c3f41` (Linux-2.6.12-rc2).
- **Mechanism verified in this tree:**
- `fd->entrylength` is `int` (`fs/hfsplus/hfsplus_fs.h`).
- `__hfs_brec_find()` does `fd->entrylength = len - keylen` with `u16
len, keylen` (`fs/hfsplus/bfind.c`). Integer promotions make `keylen
> len` a **negative int**.
- `hfs_bnode_write()` takes `u32 len` (`fs/hfsplus/bnode.c`), already
true here via `350183396918f` (“hfs/hfsplus: prevent getting
negative values of offset/length”).
- `cached_extents` is `hfsplus_extent_rec` = 8 × `{start, count}` =
**64 bytes**.
- `check_and_correct_requested_length()` caps the **destination** to
the remaining node size, but `memcpy_to_page()` still reads `len`
bytes from the **source** `cached_extents`. A huge/mismatched length
over-reads adjacent inode-slab memory and writes it into the node.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct. Same predicate as
`__hfsplus_ext_read_extent()`. Valid HFS+ overflow records are a fixed
64-byte extent array; a different size is corrupt.
- **Minimal:** two lines, no unrelated edits.
- **Regression risk:** very low. Valid images already satisfy this on
read; insert already uses `sizeof`. Invalid images now get `-EIO`
instead of a kernel leak. `HFSPLUS_EXT_DIRTY` is left set, so a later
write_inode can retry; that is existing dirty-extent behavior, not a
new API.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- The `hfs_bnode_write(..., fd->entrylength)` site dates to
`6af502de224c37` (Christoph Hellwig, 2010) in current blame; the
surrounding write function is older (`^1da177e4c3f41`, 2.6.12).
- The matching **read-side** check is `^1da177e4c3f41` (2005). The
write-side gap is long-standing.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Not applicable. Bug is missing validation on
a path that has been in the tree for many releases, not a recently
introduced regression.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Related in this tree: `99202d94909d3` (KMSAN in
`__hfsplus_ext_cache_extent`), `350183396918f` (signed offset/length →
`u32`), `c91bbd6193c70` / `7898beee166c1` (other on-disk record size
checks).
- On origin: `00aedd67d75d7` “hfsplus: validate B-tree record offset
table” is a **separate** larger patch, **not** in this tree and
**not** required for these two lines.
- **Standalone.** Not patch X/Y of a series that must land together.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record:
- Author Jiaming Zhang has no other `fs/hfsplus/` commits in this
checkout.
- Reviewer/committer Viacheslav Dubeyko is the HFS maintainer; this tree
already carries many of his HFS+ fuzz/corruption fixes.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- No new helpers or structs.
- Relies on existing `sizeof(hfsplus_extent_rec)` and `fd->entrylength`.
- `hfs_bnode_write()` being `u32` is already here; the check is valid
even without that conversion.
- **`git apply --check` of `627b7865c062f` succeeded** on this tree. The
only nearby mainline difference (`HFSPLUS_EXT_TREE_I` extra `set_bit`)
is **after** this hunk and does not block the apply.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c 627b7865c062f` → `https://patch.msgid.link/20260810092422.1
691377-1-r772577952@gmail.com`
- Direct lore/WebFetch returned **403**; thread was recovered with `b4
dig -m`.
- `b4 dig -a` shows earlier revisions were a **different** patch
(“validate B-tree record offset table”, v1–v6). This extent-length
check is a **new standalone v1** (2026-08-10), not a later revision of
the offset-table series.
- Maintainer reply
(`5f60c60a03b3d3e4b6d4adc2f241371b72977206.camel@dubeyko.com`):
**“Looks good.”** + `Reviewed-by`. **No NAK, no stability concern.**
- No reviewer `Cc: stable` in the two-message thread (absence is not a
negative).
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record:
- Recipients (from the mbox): Viacheslav Dubeyko, Yangtao Li, John Paul
Adrian Glaubitz, linux-fsdevel, linux-kernel, syzkaller.
- Reviewed by the subsystem maintainer.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record:
- No syzbot hash / bugzilla in the commit.
- Body describes a crafted-image trigger; patch CCs syzkaller.
- Original Dubeyko lore URL could not be fetched (403). Unverified
beyond the Link: tag existing.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record:
- Offset-table series is related thematically, not a dependency.
- No follow-up on `origin/master` after `627b7865c062f` touching
`fs/hfsplus/extents.c`.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable search returned 403. **UNVERIFIED**
whether stable@ already discussed this SHA. Decision does not depend on
that.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
semcode MCP tools are **not available**; callers/callees traced with
grep and file reads.
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: `__hfsplus_ext_write_extent()` only.
**Step 5.2: TRACE CALLERS**
Record:
- `hfsplus_ext_write_extent_locked()` → `hfsplus_ext_write_extent()`
(`hfsplus_write_inode` in `super.c`) and `hfsplus_file_extend()`
(`insert_extent`).
- `__hfsplus_ext_cache_extent()` writes a dirty cache before reading
another extent; used from `hfsplus_ext_read_extent()` →
`hfsplus_get_block()`, and from `hfsplus_file_truncate()`.
- Context: inode writeback, file extend, truncate, block mapping —
normal file I/O on HFS+, not init-only.
**Step 5.3: TRACE CALLEES**
Record: `hfsplus_ext_build_key`, `hfs_brec_find`, `hfs_bmap_reserve` /
`hfs_brec_insert` (new records), `hfs_bnode_write` (updates). The new
check sits immediately before `hfs_bnode_write`.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record:
- Userspace: mount HFS+ → write/truncate/fsync/umount on a file with
overflow extents.
- Crafted images are the trigger (USB/disk image, fuzzing). Mount
typically needs privilege, but that is the usual model for filesystem
image bugs already backported here.
- **Reachable from VFS file ops**, not an obscure debug path.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record:
- Read path already has the identical check (`extents.c` ~170–171).
- Catalog paths already reject bad `entrylength` (`dir.c`, `catalog.c`,
`inode.c`).
- This is the overflow-extent **write** hole in that same pattern.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** Local `__hfsplus_ext_write_extent()` still does:
```113:114:fs/hfsplus/extents.c
hfs_bnode_write(fd->bnode, hip->cached_extents,
fd->entryoffset, fd->entrylength);
```
No length check. `git merge-base --is-ancestor 627b7865c062f HEAD` is
false. Bug is old (write site 2010 / structure since 2.6.12). `u32`
conversion that turns a negative length into a huge copy **is already in
this 6.12.111 tree**.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** (`git apply --check` on `627b7865c062f`
succeeded). Hunk context around `hfs_bnode_write` matches. No rework
needed.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record:
- Read-side check: yes (since 2.6.12).
- Write-side check: **no**.
- `350183396918f` changed the failure mode (negative `int` → huge `u32`)
but did not add this check.
- No alternate fix for this write-side hole.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **fs/hfsplus** — Apple HFS+. **IMPORTANT** (not core MM/VFS, not
staging). Used for Mac-formatted disks; this tree already backports many
HFS+ crafted-image fixes.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Mature filesystem, still getting sanitizer/corruption fixes in
6.12.y. Not dead code.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Filesystem-specific / config-specific** (`CONFIG_HFSPLUS_FS`).
Anyone who mounts HFS+ (including untrusted/crafted images).
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: mount crafted HFS+ with a malformed extents overflow record
(`keylen > len` or any `entrylength != 64`), then dirty and write back
cached overflow extents (write, truncate, writeback).
- Not every boot; requires HFS+ + overflow extent writeback + corrupt
record.
- Unprivileged users cannot usually mount; a privileged mount of
untrusted media is still the standard stable threat model for these
bugs.
- The write-side `hfs_brec_find()` is independent of the earlier read. A
crafted tree (duplicate keys, confused search after splits) can
present a bad `entrylength` on update even though the read path has a
check. The code is also simply wrong: updates trust on-disk length
instead of the known 64-byte record size.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- **Failure mode:** kernel out-of-bounds read of inode-adjacent memory;
those bytes are written into the extents B-tree node (info leak on
disk + corruption of the overflow tree). With KASAN this is an OOB
report; without it, silent leak/corruption.
- **Severity: HIGH** (memory safety + info leak + image corruption). Not
a guaranteed panic on production kernels, but not cosmetic.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for HFS+ users and for the same crafted-image class
already being fixed in 6.12.y.
- **Risk:** very low (2 lines, same predicate as a 20-year-old read
check, clean apply).
- **Ratio:** strongly favors backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real bug: OOB read of `cached_extents` and kernel memory leaked into
the image.
- Important: memory safety, info leak, B-tree corruption on crafted
HFS+.
- Tiny, obviously correct, maintainer-reviewed.
- Buggy code and the `u32` write signature are in **this** 6.12.111
tree.
- Applies cleanly; no prerequisites.
- Matches existing read-side check and other HFS+ validation backports
already here.
**AGAINST:**
- HFS+ is not universal.
- No `Fixes:` / syzbot hash / `Cc: stable` (expected; not a reason to
reject).
- Typical flow reads first, so a naive reading might call it
“defensive.” That does not make the write path safe: find is redone,
on-disk length is still trusted, and the maintainer accepted the leak
description.
**UNRESOLVED:**
- Full original Dubeyko lore mail (403).
- Whether stable@ already discussed this SHA (403).
- No public reproducer in the two-message thread.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — mirrors the read check;
maintainer Reviewed-by.
2. Fixes a real bug that affects users? **PASS** — crafted HFS+ image,
kernel memory leak into the filesystem.
3. Important issue? **PASS** — OOB read / info leak / image corruption
(HIGH).
4. Small and contained? **PASS** — +2 lines, one function.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — `git apply --check`
succeeded; buggy code present.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/build/docs exception. It is a normal
small bugfix.
**Step 9.4: MAKE YOUR DECISION**
Backport to this 6.12.111 tree. The hole is here, the `u32` conversion
that turns a negative length into a huge copy is here, and the fix is
two lines that valid images already satisfy.
------------------------------------------------
What the commit solves for this tree
------------------------------------------------
`__hfsplus_ext_write_extent()` updates an overflow extent using
`fd->entrylength` from on-disk B-tree fields. A crafted node can make
that length negative or otherwise not 64. `hfs_bnode_write()` treats it
as `u32`, over-reads the 64-byte `cached_extents` in the inode, and
writes kernel memory into the image. The read path has rejected this
since 2.6.12; the write path in 6.12.111 still does not.
It meets the stable rules: real bug, important (memory safety / leak /
corruption), two lines, no new API, clean apply, no missing
prerequisites.
Risk vs benefit: benefit is closing an HFS+ crafted-image leak already
in this tree; risk is negligible because valid overflow records are
fixed-size.
------------------------------------------------
Verification
------------------------------------------------
- **[Phase 1]** Parsed tags on `627b7865c062f`: Reviewed-by Dubeyko; two
Link: URLs; no Fixes/Reported-by/Cc-stable.
- **[Phase 2]** Diff: +2 lines in `__hfsplus_ext_write_extent()` before
`hfs_bnode_write()`.
- **[Phase 2]** Confirmed `entrylength` is `int`; `__hfs_brec_find()`
uses `len - keylen`; `hfs_bnode_write(..., u32 len)`; `cached_extents`
is 64 bytes; dest length is capped but source is not.
- **[Phase 3]** Blame: write uses `entrylength` since Hellwig 2010;
read-side check since `1da177e4c3f41`.
- **[Phase 3]** No Fixes: tag. Standalone, not the offset-table series.
- **[Phase 3]** `git apply --check` of `627b7865c062f` on this tree:
success.
- **[Phase 4]** `b4 dig -c 627b7865c062f`: msgid
`20260810092422.1691377-1-r772577952@gmail.com`; maintainer “Looks
good” + Reviewed-by. `b4 dig -a`: this is a new v1, not v7 of the
offset-table patch.
- **[Phase 4]** lore WebFetch/curl: 403. Thread recovered via `b4 dig
-m`.
- **[Phase 5]** Callers: `hfsplus_write_inode`, `hfsplus_file_extend`,
`__hfsplus_ext_cache_extent` (get_block/truncate). semcode
unavailable.
- **[Phase 6]** `git describe HEAD`: `p-6.12-3-gc504cf0baa3d8`; `make
kernelversion`: 6.12.111.
- **[Phase 6]** `git merge-base --is-ancestor 627b7865c062f HEAD`: not
an ancestor. Local write path still unchecked.
- **[Phase 6]** `350183396918f` (u32 bnode lengths) **is** in HEAD.
- **[Phase 8]** Failure mode: OOB read + kernel memory leaked into the
extents tree; severity HIGH.
- **UNVERIFIED:** original Dubeyko prompt mail body; stable@ archive
discussion.
The fix is small, surgical, present-bug, and meets the stable criteria
for this tree.
**YES**
fs/hfsplus/extents.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index 8e886514d27f1..11c990d18ac8a 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -110,6 +110,8 @@ static int __hfsplus_ext_write_extent(struct inode *inode,
} else {
if (res)
return res;
+ if (fd->entrylength != sizeof(hfsplus_extent_rec))
+ return -EIO;
hfs_bnode_write(fd->bnode, hip->cached_extents,
fd->entryoffset, fd->entrylength);
hip->extent_state &= ~HFSPLUS_EXT_DIRTY;
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread