* [PATCH AUTOSEL 6.18-6.6] ext4: use FGP_WRITEBEGIN for tail block zeroing
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:44 ` Sasha Levin
2026-10-19 14:04 ` sashiko-bot
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] ext4: check return value of ext4_get_block() in ext4_load_tail_bh() Sasha Levin
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-10-19 13:44 UTC (permalink / raw)
To: patches, stable
Cc: Zhang Yi, Jan Kara, Theodore Ts'o, Sasha Levin, linux-ext4,
linux-kernel
From: Zhang Yi <yi.zhang@huawei.com>
[ Upstream commit e1cf7b5f5c8ad1e00d00c50c61c3e8fdc79cd422 ]
ext4_load_tail_bh() returns a locked folio that callers immediately
mutate through folio_zero_range() and mark_buffer_dirty(). Use
FGP_WRITEBEGIN so that, on backing devices that require stable writes,
__filemap_get_folio() waits for writeback to finish before returning
the folio; on regular devices the wait is a no-op.
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20260714080044.4038124-2-yi.zhang@huaweicloud.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have enough to decide: this is a one-line stable-writes data-integrity
fix in a userspace-reachable ext4 tail-zeroing path, and the helper and
`FGP_WRITEBEGIN` both exist in this 7.3-rc4 tree.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[ext4] [use] use FGP_WRITEBEGIN for tail block zeroing**
Subsystem prefix `ext4:`. Verb is “use” (correctness change of folio-get
flags, not a feature). Claimed intent: get the tail folio with the
write-begin flag set so writeback finishes before the folio is mutated.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Verified from `git show e1cf7b5f5c8ad1e00d00c50c61c3e8fdc79cd422`:
- **Fixes:** none (expected for this review pipeline)
- **Reported-by:** none
- **Tested-by:** none
- **Reviewed-by:** Jan Kara \<jack@suse.cz\> (ext4 co-maintainer)
- **Acked-by:** none
- **Link:** https://patch.msgid.link/20260714080044.4038124-2-
yi.zhang@huaweicloud.com
- **Cc: stable:** none (expected; not a negative signal)
- **Signed-off-by:** Zhang Yi \<yi.zhang@huawei.com\> (author); Theodore
Ts'o \<tytso@mit.edu\> (ext4 maintainer)
No syzbot, no kernel-test-robot, no multiple reporters.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `ext4_load_tail_bh()` returns a locked folio that callers
immediately mutate with `folio_zero_range()` and
`mark_buffer_dirty()`, without waiting for in-flight writeback.
- **Symptom/failure mode:** On backing devices that require stable
writes, page-cache contents can change while DMA/writeback is in
progress, which can corrupt the in-flight write or break integrity
checksums. On ordinary devices the extra wait is a no-op.
- **Version information:** none in the message. History shows the helper
landed in v7.1-rc1 and this fix in v7.3-rc1.
- **Root cause:** folio obtained with `FGP_LOCK | FGP_ACCESSED |
FGP_CREAT` (no `FGP_STABLE` / `FGP_WRITE`).
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Yes, this is a real bug fix**, not cleanup. The cover letter
for the series (v4) states it explicitly: “Add patch 1 to use
FGP_WRITEBEGIN in ext4_load_tail_bh(), fixing a pre-existing stable-
writes bug exposed by patch 4's reordering.”
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `fs/ext4/inode.c` — **1 insertion, 1 deletion**
- Function: `ext4_load_tail_bh()`
- Scope: **single-file surgical fix** (one flag expression)
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
**Before:** `__filemap_get_folio(..., FGP_LOCK | FGP_ACCESSED |
FGP_CREAT, ...)`. Locks/creates the folio and marks it accessed. Does
**not** wait for writeback.
**After:** `__filemap_get_folio(..., FGP_WRITEBEGIN | FGP_ACCESSED,
...)`.
`FGP_WRITEBEGIN` is defined in `include/linux/pagemap.h` as:
`FGP_LOCK | FGP_WRITE | FGP_CREAT | FGP_STABLE`
Net additions: `FGP_WRITE` and `FGP_STABLE`. `FGP_STABLE` calls
`folio_wait_stable()` after the folio is locked (`mm/filemap.c`). That
waits for writeback only if `mapping_stable_writes()` is set.
Path affected: every caller of `ext4_load_tail_bh()` immediately zeros
the folio (`ext4_block_do_zero_range()`,
`ext4_block_journalled_zero_range()`). This is a **normal mutator
path**, not an error-only path.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** memory-safety / data integrity (stable-pages race with
writeback), plus a small GFP/idle-flag side effect from `FGP_WRITE`
- **Mechanism:** On `AS_STABLE_WRITES` mappings, in-flight writeback
DMA’s from page cache. Zeroing the folio without `folio_wait_stable()`
can mix old and new contents in the I/O. `FGP_WRITEBEGIN` is the
established write-begin flag set that waits first.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct. Same flags every `->write_begin`
implementation uses. `folio_wait_stable()` is a no-op when
`mapping_stable_writes()` is false, so typical disks are unchanged.
- **Regression risk:** very low. Waiting while the folio is locked is
the documented write-begin pattern (`folio_wait_stable` comment: if
unlocked, writeback can restart). `FGP_ACCESSED | FGP_WRITEBEGIN`
still takes the `FGP_ACCESSED` branch in `__filemap_get_folio()`, so
idle-flag clearing does not change vs the old flags; `FGP_WRITE` only
adds `__GFP_WRITE` on allocation.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Current line is `e1cf7b5f5c8ad` (this commit).
- Same flags were already in `__ext4_block_zero_page_range()` before the
helper was factored out (`3b312a6f510ca`, v7.1-rc1, “ext4: factor out
journalled block zeroing range”).
- Folio conversion `9d3973de9a374` (“ext4: Convert
__ext4_block_zero_page_range() to use a folio”, v6.4 era) replaced
`find_or_create_page()` with `FGP_LOCK | FGP_ACCESSED | FGP_CREAT`.
`find_or_create_page()` also did **not** wait for stable writeback, so
the gap is older than the helper.
- Helper `ext4_load_tail_bh` first appeared in **v7.1-rc1**
(`3b312a6f510ca`).
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Not applicable. The immediate pre-image is
the helper as introduced in `3b312a6f510ca`, which copied the old flags.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record: This is **patch 1/9** of Zhang Yi’s series “ext4: fix unaligned
edge handling in FALLOC_FL_WRITE_ZEROES”, landed together around
2026-07-22:
1. `e1cf7b5f5c8ad` this commit
2. `2fff82f081401` skip tail zeroing for inline data
3. `705a3fd3bac16` check `ext4_get_block()` return
4. `b16e9d27a643a` move partial zeroing earlier in `ext4_zero_range()`
(the reorder that **exposes** the stable-writes bug)
5–9. WRITE_ZEROES edge-handling / orphan-list work
This patch does **not** depend on 2–9. Later patches that increase tail-
zeroing (especially 4) depend on this one for correctness on stable-
write devices.
Related earlier commit `df0b5afc62f33` (“ext4: remove the redundant
folio_wait_stable()”) is a different path (`ext4_da_write_begin`); it
only shows that ext4 already treats `FGP_WRITEBEGIN` as implying the
stable wait.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Zhang Yi is a regular, high-volume ext4 contributor (tail-
zeroing, WRITE_ZEROES, journal paths). This series was reviewed by Jan
Kara and applied by Ted Ts'o. Related commits in the same week are the
rest of the 1/9 series.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record:
- Prerequisite API: `FGP_WRITEBEGIN` from `e999a5c5a19cf` (“fs: Add
FGP_WRITEBEGIN”), present since **v6.4-rc1**. Present in this tree.
- Prerequisite function: `ext4_load_tail_bh()` since **v7.1-rc1**.
Present in this tree.
- **Can apply standalone:** yes. One flag change; no new helpers.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c e1cf7b5f5c8ad` → https://patch.msgid.link/20260714080044.40
38124-2-yi.zhang@huaweicloud.com
- Series revisions: **v3** (2026-07-08) and **v4** (2026-07-14). Cover
letter: v4 has **no code changes** vs v3; v2 **added this patch**.
- Cover letter quote (verified in saved mbox): “Add patch 1 to use
FGP_WRITEBEGIN in ext4_load_tail_bh(), fixing a pre-existing stable-
writes bug exposed by patch 4's reordering.”
- Ted Ts'o applied the series; patch 9 was dropped separately over
error-handling concerns.
- No NAK on this patch. No reviewer asked for `Cc: stable` on patch 1
(patches 7/8 in the same mbox do have `Cc: stable`, for WRITE_ZEROES-
specific bugs).
- WebFetch of lore.kernel.org returned **403**; discussion recovered via
`b4 dig -m /tmp/ext4-fgp.mbox`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
`b4 dig -w` recipients included linux-ext4, linux-fsdevel, tytso,
adilger, Jan Kara, Ojaswin Mujoo, Ritesh Harjani, Huawei ext4
developers.
Record: **Jan Kara Reviewed-by** (in the patch); **Ted Ts'o** applied
it. Right maintainers were on the thread.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: **No standalone bugzilla/syzbot report.** The bug was found
while reworking WRITE_ZEROES unaligned edges. Severity from the author:
pre-existing **stable-writes data-integrity** bug.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Standalone as a fix. The rest of 1/9 is WRITE_ZEROES contract
work and two other pre-existing helper fixes (inline-data skip;
propagate `ext4_get_block()` errors). This commit does not require those
to be correct.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable WebFetch returned **403**. No evidence in
the ext4 thread that this patch 1 was requested for stable or rejected
for stable. **UNVERIFIED** whether linux-stable@ discussed it.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: **`ext4_load_tail_bh()`** only.
**Step 5.2: TRACE CALLERS**
Verified in this tree:
- `ext4_block_do_zero_range()` → `folio_zero_range()` +
`mark_buffer_dirty()`
- `ext4_block_journalled_zero_range()` → journal write access +
`folio_zero_range()`
- Both from `ext4_block_zero_range()`
- That is used by:
- `ext4_block_zero_eof()` — truncate (`ext4_truncate`), append
(`ext4_write_checks` in `fs/ext4/file.c`), extent operations
(`fs/ext4/extents.c`)
- `ext4_zero_partial_blocks()` — punch hole / zero_range /
WRITE_ZEROES (`ext4_punch_hole`, `ext4_zero_range`, extents
WRITE_ZEROES)
Record: **Common filesystem operations**, process context, folio locked.
Not IRQ.
**Step 5.3: TRACE CALLEES**
Record: `__filemap_get_folio()` → on `FGP_STABLE`, `folio_wait_stable()`
→ `folio_wait_writeback()` if `mapping_stable_writes()`. Then
`create_empty_buffers` / `ext4_get_block` / `ext4_read_bh_lock` as
before.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: **Userspace-reachable** without privilege beyond write access to
an ext4 file: `truncate`,
`fallocate(PUNCH_HOLE|ZERO_RANGE|WRITE_ZEROES)`, append `write()`.
**HIGH** reachability.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Other ext4 write paths already use `FGP_WRITEBEGIN`
(`fs/ext4/inline.c`, `fs/ext4/move_extent.c`) and/or explicit
`folio_wait_stable()` (`ext4_write_begin`, `ext4_page_mkwrite`). Tail-
block zeroing was the outlier.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Local tree: **`git describe HEAD` = `v7.3-rc4-27-gf0100363d8c37`**,
`VERSION=7 PATCHLEVEL=3 SUBLEVEL=0 EXTRAVERSION=-rc4`. Origin is
torvalds/linux (mainline), detached HEAD.
- `ext4_load_tail_bh()` exists at `fs/ext4/inode.c` (currently already
using `FGP_WRITEBEGIN | FGP_ACCESSED` because `e1cf7b5f5c8ad` **is an
ancestor of HEAD**).
- Buggy pre-image (helper with `FGP_LOCK | FGP_ACCESSED | FGP_CREAT`)
existed from **v7.1-rc1** (`3b312a6f510ca`) until this fix in
**v7.3-rc1**.
- `FGP_WRITEBEGIN` exists here (`include/linux/pagemap.h:720`).
The pipeline instruction is to ignore “already selected” markers, not to
treat mainline already containing the original commit as a reason to
reject a backport-worthy fix. The code and prerequisites are in this
tree.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply** on this tree (one-line flag change;
`FGP_WRITEBEGIN` present). `git log --oneline -10 -- fs/ext4/inode.c`
shows normal ext4 churn; nothing that would conflict with this hunk.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This exact change **is** HEAD’s current code (`git blame` on the
flags line is `e1cf7b5f5c8ad`). No alternate fix for the same stable-
writes hole in `ext4_load_tail_bh()`. Other `folio_wait_stable()` sites
in ext4 do not cover this helper.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **Subsystem: fs/ext4. Criticality: IMPORTANT** (widely used
filesystem; data-integrity on stable-write devices). Not core mm/VFS,
but truncate/punch/zero are universal ext4 operations.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: **Actively developed** (WRITE_ZEROES series, iomap conversion,
journal/writeback fixes in 7.3). The helper itself is recent (v7.1) but
the missing stable wait is the same pattern as the older zero-page-range
code.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **ext4 users whose backing device requires stable writes**, i.e.
`bdev_stable_writes()`: integrity checksums (`CONFIG_BLK_DEV_INTEGRITY`)
**or** `BLK_FEAT_STABLE_WRITES`.
Verified setters of `BLK_FEAT_STABLE_WRITES` in this tree include NVMe
(when `data_digest` is enabled), RAID5 `skip_copy`, dm-integrity, zram,
RBD, iSCSI TCP, MMC, DRBD. Superblock then sets `SB_I_STABLE_WRITES` and
inode mappings get `AS_STABLE_WRITES`.
Not universal, but not a toy config: enterprise integrity, RAID, dm-
integrity, some NVMe.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- Trigger: tail/partial-block zeroing while that folio is under
writeback.
- How common: truncate, punch, zero_range, append past EOF —
**routine**. The race still needs concurrent writeback of that folio,
so it is **timing-dependent**.
- Cover letter: patch 4’s reorder **widens** the window; the bug is
still pre-existing without patch 4. This tree **already has patch 4**
(`b16e9d27a643a`).
- Unprivileged users with write access **can** trigger the operations.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Failure mode: in-flight writeback sees torn folio contents /
integrity checksum mismatch.** **Severity: CRITICAL** (data corruption),
limited to stable-write devices. Not a panic path.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit: high** for ext4 on stable-write devices (prevents silent
data corruption on common ops). **Low** for everyone else (wait is a
no-op).
- **Risk: very low** (1 line, standard flags, no API change, no new
locking order).
- **Ratio: strongly favors applying.**
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
**FOR backporting:**
- Author and cover letter call it a **pre-existing stable-writes bug**.
- Mutating a folio under writeback on `AS_STABLE_WRITES` is a known
class of **data corruption**.
- Reachable from truncate / fallocate / append.
- One-line, matches `FGP_WRITEBEGIN` used everywhere else.
- Reviewed by Jan Kara; applied by Ted Ts'o.
- `FGP_WRITEBEGIN` and `ext4_load_tail_bh()` both exist in this 7.3-rc4
tree.
- This tree also has the later reorder (`b16e9d27a643a`) that
**exposes** the race more.
**AGAINST backporting:**
- No Fixes tag, no user/syzbot report (not a valid NO by pipeline
rules).
- Nested in a 9-patch WRITE_ZEROES series; later patches are feature-
contract fixes. This patch is still self-contained.
- Author did not `Cc: stable` on patch 1 (other patches in the same
series did).
- Bug existed in spirit since `find_or_create_page()`; field reports are
absent, so the race may be rare without the v7.3 reorder.
**UNRESOLVED:**
- linux-stable@ archives could not be fetched (HTTP 403).
- Full v2 mailing-list review text for this hunk was not retrieved (lore
403); Jan’s Reviewed-by on the committed patch is verified.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — one flag change to the
standard write-begin set; reviewed by Jan Kara; in mainline since
v7.3-rc1.
2. Fixes a real bug that affects users? **PASS** — stable-writes devices
(NVMe digest, RAID5 skip_copy, dm-integrity, zram, T10 PI, etc.).
3. Important issue? **PASS** — data corruption on in-flight writeback,
not a crash, but explicitly in the stable rules list.
4. Small and contained? **PASS** — 1 line, one function.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — `FGP_WRITEBEGIN` and
`ext4_load_tail_bh()` are here; the hunk is identical to what already
landed.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: **None** (not a device ID, quirk, DT, build, or docs exception).
It qualifies as a normal data-integrity bug fix.
**Step 9.4: MAKE YOUR DECISION**
This tree is **Linux 7.3-rc4** (Linus mainline). The helper lives here,
`FGP_WRITEBEGIN` lives here, and callers still zero the folio
immediately. The change is a one-line restoration of the stable-write
wait that every other write-begin path already does. That is exactly the
kind of small, important data-integrity fix stable trees should take.
The surrounding WRITE_ZEROES series is not required for this hunk to be
correct.
### What the commit solves
`ext4_load_tail_bh()` was grabbing a locked folio without `FGP_STABLE`.
Callers then call `folio_zero_range()` and dirty the buffer. On devices
that set `AS_STABLE_WRITES` (integrity checksums or
`BLK_FEAT_STABLE_WRITES`), that can change page-cache bytes while
writeback DMA is still running. Switching to `FGP_WRITEBEGIN |
FGP_ACCESSED` waits for writeback first; elsewhere the wait is a no-op.
### Stable rules / risk vs benefit
Meets all six stable criteria. Benefit is preventing filesystem data
corruption on a real (if not universal) device class; risk is a single
established flag combination.
### Concerns
- Nested in a larger series, but this patch is standalone.
- No field report; the author found it while reordering zeroing. Still a
concrete integrity bug, and this tree already contains the reorder
that widens it.
- Lore stable-list search was blocked (403); that does not change the
code-level conclusion.
Verification:
- [Phase 1] Parsed tags from `git show e1cf7b5f5c8ad`: Reviewed-by Jan
Kara; Link msgid `...4038124-2...`; no Fixes/Reported-by/Cc:stable
- [Phase 1] Cover letter in `/tmp/ext4-fgp.mbox` msg 5: “pre-existing
stable-writes bug exposed by patch 4's reordering”
- [Phase 2] Diff: 1 line in `ext4_load_tail_bh()`; flags
`FGP_LOCK|FGP_ACCESSED|FGP_CREAT` → `FGP_WRITEBEGIN|FGP_ACCESSED`
- [Phase 2] `FGP_WRITEBEGIN` = `FGP_LOCK|FGP_WRITE|FGP_CREAT|FGP_STABLE`
(`include/linux/pagemap.h:720`)
- [Phase 2] `__filemap_get_folio` calls `folio_wait_stable()` when
`FGP_STABLE` (`mm/filemap.c:1986-1987`)
- [Phase 2] `folio_wait_stable()` only waits if
`mapping_stable_writes()` (`mm/page-writeback.c:3105-3108`)
- [Phase 3] `git describe --contains --match 'v7.*' 3b312a6f510ca` →
v7.1-rc1 (helper introduced)
- [Phase 3] `git describe --contains --match 'v7.*' e1cf7b5f5c8ad` →
v7.3-rc1 (this fix)
- [Phase 3] `git merge-base --is-ancestor e1cf7b5f5c8ad HEAD` true;
`3b312a6f510ca` also ancestor
- [Phase 3] `git show 9d3973de9a374`: folio conversion used the same
non-stable flags; previously `find_or_create_page()`
- [Phase 3] `git show 3b312a6f510ca`: factor-out copied those flags into
`ext4_load_tail_bh()`
- [Phase 4] `b4 dig -c e1cf7b5f5c8ad`: lore URL; v3 then v4; v4 no code
change
- [Phase 4] `b4 dig -w`: linux-ext4, tytso, Jan Kara, adilger, Ojaswin,
Ritesh on Cc
- [Phase 4] `b4 dig -m /tmp/ext4-fgp.mbox`: Ted applied 1–8; patch 9
dropped; no NAK on patch 1
- [Phase 4] WebFetch lore.kernel.org 403 — UNVERIFIED stable-list
discussion
- [Phase 5] Callers: `ext4_block_do_zero_range` /
`ext4_block_journalled_zero_range` → `ext4_block_zero_range` →
`ext4_block_zero_eof` / `ext4_zero_partial_blocks` (truncate, punch,
zero_range, WRITE_ZEROES, append)
- [Phase 5] Other ext4 write paths already use `FGP_WRITEBEGIN` or
`folio_wait_stable()`
- [Phase 6] `git describe HEAD` = v7.3-rc4-27-gf0100363d8c37; Makefile
7.3.0-rc4
- [Phase 6] Current `fs/ext4/inode.c:4042-4043` already has
`FGP_WRITEBEGIN | FGP_ACCESSED`
- [Phase 7] fs/ext4, important filesystem
- [Phase 8] `bdev_stable_writes()` = integrity csum OR
`BLK_FEAT_STABLE_WRITES` (`include/linux/blkdev.h:1410-1414`)
- [Phase 8] `BLK_FEAT_STABLE_WRITES` set by nvme (data_digest), raid5
skip_copy, dm-integrity, zram, rbd, iscsi_tcp, mmc, drbd
- [Phase 8] Failure: data corruption on stable-write devices; wait is
no-op otherwise
- UNVERIFIED: linux-stable mailing list history (HTTP 403)
- UNVERIFIED: full v2 review mail body for this hunk (lore 403);
Reviewed-by on the commit is verified
**YES**
fs/ext4/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 2e3f7ba1a924b..c3fb5f16f6e39 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4092,7 +4092,7 @@ static int __ext4_block_zero_page_range(handle_t *handle,
int err = 0;
folio = __filemap_get_folio(mapping, from >> PAGE_SHIFT,
- FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
+ FGP_WRITEBEGIN | FGP_ACCESSED,
mapping_gfp_constraint(mapping, ~__GFP_FS));
if (IS_ERR(folio))
return PTR_ERR(folio);
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH AUTOSEL 6.18-5.10] ext4: check return value of ext4_get_block() in ext4_load_tail_bh()
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:44 ` [PATCH AUTOSEL 6.18-6.6] ext4: use FGP_WRITEBEGIN for tail block zeroing Sasha Levin
@ 2026-10-19 13:47 ` Sasha Levin
2026-10-19 14:32 ` sashiko-bot
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] ext4: guard against NULL s_group_info in ext4_get_group_info Sasha Levin
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
To: patches, stable
Cc: Zhang Yi, Jan Kara, Theodore Ts'o, Sasha Levin, linux-ext4,
linux-kernel
From: Zhang Yi <yi.zhang@huawei.com>
[ Upstream commit 705a3fd3bac167f1dabe3d3bba46cd484c5528dd ]
ext4_load_tail_bh() ignores the return value of ext4_get_block(), so an
I/O or allocation failure is silently discarded. buffer_mapped(bh) stays
false and the function returns NULL, which callers such as
ext4_block_do_zero_range() treat as "nothing to do" and return success.
This can mask real failures during zero-range, truncate, or punch-hole
operations, potentially exposing stale data if the block was not
actually a hole and needed zeroing. So propagate the error to the
callers.
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20260714080044.4038124-4-yi.zhang@huaweicloud.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Local tree
This checkout is **v7.3-rc4** (`7.3.0-rc4`, HEAD `f0100363d8c37`).
`ext4_load_tail_bh()` exists here (since v7.1-rc1). The candidate
`705a3fd3bac16` is already an ancestor of HEAD (merged in v7.3-rc1). The
verdict is whether this fix belongs in **this** tree on technical merit,
not because it was already applied.
---
## Phase 1: Commit message forensics
**Step 1.1 — Subject**
- Record: **[ext4] [check] check return value of `ext4_get_block()` in
`ext4_load_tail_bh()`**
- Verb “check” here means propagate a previously ignored error.
**Step 1.2 — Tags**
- Record:
- `Signed-off-by: Zhang Yi <yi.zhang@huawei.com>` (author; ext4
reviewer in MAINTAINERS)
- `Reviewed-by: Jan Kara <jack@suse.cz>` (ext4 reviewer)
- `Link: https://patch.msgid.link/20260714080044.4038124-4-
yi.zhang@huaweicloud.com` (v4 3/9)
- `Signed-off-by: Theodore Ts'o <tytso@mit.edu>` (ext4 maintainer)
- No `Fixes:`, no `Cc: stable`, no `Reported-by`, no syzbot. Absence
of those tags is expected for this review.
**Step 1.3 — Body**
- Record: **Bug:** `ext4_load_tail_bh()` ignores `ext4_get_block()`
errors. On failure `buffer_mapped(bh)` stays false, the helper returns
`NULL`, and callers such as `ext4_block_do_zero_range()` treat that as
“hole / nothing to do” and return success.
- **Symptom:** Silent success of zero-range, truncate, or punch-hole
while the tail block was not zeroed.
- **Failure mode:** Stale data can later be exposed if the block was not
a hole.
- **Root cause:** Error conflated with “unmapped hole”.
- **No version range** in the message.
**Step 1.4 — Hidden bug fix?**
- Record: **Not hidden.** It is an explicit error-propagation / stale-
data fix. Cover letter later confirms patches 2 and 3 were “pre-
existing fixes … originally going to be sent separately,” then folded
into the WRITE_ZEROES series.
---
## Phase 2: Diff analysis
**Step 2.1 — Inventory**
- Record: **One file**, `fs/ext4/inode.c`: **+3 / −1**. One function:
`ext4_load_tail_bh()`. Surgical, single-file.
**Step 2.2 — Flow**
- Record: **Before:** unmapped bh → `ext4_get_block(..., create=0)`
return ignored → if still unmapped, `goto unlock` with `err == 0` →
`return NULL`.
- **After:** negative return sets `err` and `goto unlock` → existing
`return err ? ERR_PTR(err) : NULL`.
- Path: mapping lookup on the tail block during partial-block zeroing
(error path only).
**Step 2.3 — Bug mechanism**
- Record: **Logic / correctness (ignored error → false success).**
Category closest to “return value checking / missing error handling.”
Not UAF, leak, or race.
- Mechanism: I/O or allocation failure from `ext4_get_block()` is
treated as a hole, so truncate/punch/zero-range skip tail zeroing and
still succeed.
**Step 2.4 — Fix quality**
- Record: **Obviously correct.** Callers already do `IS_ERR_OR_NULL(bh)`
/ `PTR_ERR_OR_ZERO(bh)` (present since the helper was factored out).
Unlock/put already runs on the `unlock` path. Regression risk is low
**in this tree** because parent `2fff82f081401` skips inline-data EOF
zeroing (otherwise append writes on inline files could see `-ERANGE`).
No API change.
---
## Phase 3: Git history
**Step 3.1 — Blame**
- Record: Ignored `ext4_get_block()` call dates to **`d863dc3614e489`
(2013, Lukas Czerner)** inside `__ext4_block_zero_page_range`. Helper
**`ext4_load_tail_bh`** was factored out in **`3b312a6f510ca` (“ext4:
factor out journalled block zeroing range”, Zhang Yi, 2026-03)** and
kept the same ignored return. First in this lineage: **v7.1-rc1**.
**Step 3.2 — Fixes: tag**
- Record: **None.** Introducing commit for the helper is
`3b312a6f510ca`, ancestor of this HEAD. The ignored-return bug is
older than the helper.
**Step 3.3 — File history**
- Record: Same v4 series around this commit:
1. `e1cf7b5f5c8ad` FGP_WRITEBEGIN
2. `2fff82f081401` skip inline-data tail zeroing **(prerequisite)**
3. **`705a3fd3bac16` this fix**
4. `b16e9d27a643a` move partial zeroing earlier
5. `012e6a2c9ff4d` comment-only return-semantics clarification
6–9. WRITE_ZEROES edge handling (some of those have `Cc: stable`)
- This patch is **standalone** given (2). (5) is comments only; 6–9 are
WRITE_ZEROES, not required for this error check.
**Step 3.4 — Author**
- Record: **Zhang Yi** is an ext4 **Reviewer** in MAINTAINERS; 178 ext4
commits in this tree. Not a drive-by.
**Step 3.5 — Dependencies**
- Record: Needs `ext4_load_tail_bh()` + `ERR_PTR` return +
`IS_ERR_OR_NULL` callers — all present since `3b312a6f510ca` (v7.1).
Needs **`2fff82f081401`** so inline-data append does not start
failing. Both are in this tree. **Applies standalone on top of that
parent.**
---
## Phase 4: Mailing list
**Step 4.1 — Original discussion**
- Record: `b4 dig -c 705a3fd3bac16` → **https://patch.msgid.link/2026071
4080044.4038124-4-yi.zhang@huaweicloud.com**
- Series: **[PATCH v4 0/9] ext4: fix unaligned edge handling in
FALLOC_FL_WRITE_ZEROES**
- Revisions: v1 → v4. v4 vs v3: **no code change** on this patch; v3
collected Jan’s R-b.
- Cover letter: patches 2 and 3 were **pre-existing fixes folded in**,
not WRITE_ZEROES-only prep.
- No NAK found on this patch. Ted merged it.
**Step 4.2 — Recipients (`b4 dig -w`)**
- Record: `linux-ext4`, `linux-fsdevel`, **tytso@mit.edu**,
**jack@suse.cz**, adilger, libaokun, ojaswin, ritesh.list —
maintainers/reviewers were on Cc. Jan reviewed; Ted committed.
**Step 4.3 — Bug report**
- Record: **No external bugzilla/syzbot report.** Issue found by code
inspection (author). Severity is argued in the commit: silent success
+ possible stale data.
**Step 4.4 — Series**
- Record: 9 patches. This one is independently useful. Sibling patches
7–9 carry `Cc: stable` for WRITE_ZEROES; this one does not. That is
**not** used as a NO vote (tags are expected to be absent here).
**Step 4.5 — stable@**
- Record: **UNVERIFIED.** lore.kernel.org WebFetch returned 403; no
separate stable-thread confirmation. `b4 dig` matched the ext4 thread.
---
## Phase 5: Semantic / call graph
**Step 5.1 — Functions**
- Record: **`ext4_load_tail_bh`** only.
**Step 5.2 — Callers**
- Record (this tree):
- `ext4_block_do_zero_range()` → `IS_ERR_OR_NULL` / `PTR_ERR_OR_ZERO`
- `ext4_block_journalled_zero_range()` → same
- via `ext4_block_zero_range()`:
- `ext4_block_zero_eof()` — truncate (`ext4_truncate`), setattr
grow, append (`ext4_write_checks` in `fs/ext4/file.c`)
- `ext4_zero_partial_blocks()` — punch-hole (`ext4_punch_hole`),
`ext4_zero_range()` / WRITE_ZEROES in `fs/ext4/extents.c`
**Step 5.3 — Callees**
- Record: `__filemap_get_folio`, `ext4_get_block` (lookup only),
`ext4_read_bh_lock`, folio lock/put.
**Step 5.4 — Reachability**
- Record: **Yes, from userspace:** `truncate`/`ftruncate`, `fallocate`
(punch/zero/WRITE_ZEROES), append `write`. Unprivileged processes on
their own files can hit the path. The **error** itself needs
`ext4_get_block()` failure (I/O error, ENOMEM, corruption), not a
normal happy path.
**Step 5.5 — Similar patterns**
- Record: Same ignored return lived in `__ext4_block_zero_page_range`
since 2013. No second copy of this exact bug in current
`ext4_load_tail_bh`.
---
## Phase 6: This checkout
**Step 6.1 — Buggy code here?**
- Record: **Yes.** Helper is in `fs/ext4/inode.c` (~4033). Introduced
**v7.1-rc1** (`3b312a6f510ca`). This 7.3-rc4 tree has that code. The
ignored-return bug existed until `705a3fd3bac16` (v7.3-rc1).
**Step 6.2 — Backport difficulty**
- Record: **Clean apply** onto the parent in this tree
(`2fff82f081401`). Three lines. Later `b4e409c18c797` removed fs-layer
fscrypt decrypt from this function; that is after/around the same
merge window and does not change the get_block check.
**Step 6.3 — Related fixes already here?**
- Record: Prerequisite `2fff82f081401` is in HEAD. Follow-up
`012e6a2c9ff4d` is comments only. No competing alternate fix.
---
## Phase 7: Subsystem
**Step 7.1**
- Record: **ext4 (`fs/ext4`)** — **IMPORTANT** (default Linux
filesystem; data integrity).
**Step 7.2**
- Record: **Active** in 7.3 (WRITE_ZEROES series, iomap, fastcommit,
etc.). Helper itself is recent (7.1) but the ignored-return pattern is
old.
---
## Phase 8: Impact and risk
**Step 8.1 — Who**
- Record: **Filesystem-specific:** ext4 users doing truncate, punch-
hole, zero-range, append, WRITE_ZEROES.
**Step 8.2 — Trigger**
- Record: Partial-block zeroing **plus** `ext4_get_block()` failure.
Uncommon (I/O error / ENOMEM / bad metadata). Unprivileged users can
invoke the operations; they cannot usually force get_block failure
except under memory/IO pressure.
**Step 8.3 — Failure mode**
- Record: **Stale data / false success** on truncate/punch/zero-range.
Not a panic. **Severity: HIGH** for a filesystem (classic “truncate
succeeded but tail not zeroed → later extend exposes old data”). Not
CRITICAL (no oops/deadlock).
**Step 8.4 — Ratio**
- Record: **Benefit: high** for ext4 data integrity on error paths.
**Risk: very low** (3 lines, existing ERR_PTR path, Jan + Ted).
Prerequisite for inline data is already in this tree.
---
## Phase 9: Synthesis
**Step 9.1 — Evidence**
**For:**
- Real correctness bug: mapping lookup failure reported as success.
- Concrete data-integrity outcome on truncate/punch/zero-range.
- Userspace-reachable (common ext4 ops).
- 3-line, obviously correct; callers already handle `ERR_PTR`.
- Reviewed by Jan Kara, merged by Ted Ts'o; author is an ext4 reviewer.
- Cover letter: intended as a standalone pre-existing fix.
- Buggy helper **is in this 7.3 tree** (since 7.1).
- Prerequisite inline-data skip **is in this tree**.
**Against:**
- Error-path only; no syzbot/user `Reported-by`.
- Bundled in a 9-patch WRITE_ZEROES series (mitigated: explicitly called
out as a separate pre-existing fix).
- Trigger requires get_block failure (rare).
**Unresolved:** stable@ archive (lore 403); full v1 review text beyond
the on-commit `Reviewed-by`.
**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** (trivial check; R-b Jan; in
7.3-rc1)
2. Real bug affecting users? **PASS** (false success + possible stale
data)
3. Important (crash/security/corruption/deadlock)? **PASS** (stale data
/ integrity, HIGH)
4. Small and contained? **PASS** (+3/−1, one function)
5. No new features/APIs? **PASS**
6. Can apply to this tree? **PASS** (helper + callers + prerequisite
present)
**Step 9.3 — Exceptions**
- Record: **None** (not a device ID/quirk/DT/build/doc exception).
Ordinary bugfix.
**Step 9.4 — Decision**
This is a small, reviewed ext4 integrity fix for code that exists in
this 7.3-rc4 tree. Callers already propagate `ERR_PTR`. The inline-data
prerequisite is present, so the change does not turn append-on-inline-
data into `-ERANGE`. It meets stable criteria for **this** tree.
---
## Why it matters here
On this 7.3 tree, `ext4_load_tail_bh()` is the shared tail-zero helper
for truncate, punch-hole, fallocate zero/WRITE_ZEROES, and post-EOF
append. Ignoring `ext4_get_block()` makes a failed extent lookup look
like a hole, so those operations can succeed without zeroing a mapped
tail. Later grow/read can then expose stale bytes. Checking the return
and using the existing `ERR_PTR` path is the right, minimal fix.
Risk is low: three lines, no locking/API change, prerequisite
`2fff82f081401` already in this checkout.
---
## Verification
- **[Phase 1]** Parsed commit
`705a3fd3bac167f1dabe3d3bba46cd484c5528dd`: Zhang Yi, R-b Jan Kara,
SOB Ted Ts'o, Link msgid `…4038124-4…`. No Fixes/Cc:stable/Reported-
by.
- **[Phase 2]** `git show 705a3fd3bac16`: +3/−1 in `ext4_load_tail_bh`;
`err = ext4_get_block(...); if (err < 0) goto unlock`.
- **[Phase 2]** Confirmed unlock already returns `err ? ERR_PTR(err) :
NULL` before the patch (`git show 705a3fd3bac16^:fs/ext4/inode.c`).
- **[Phase 2]** Callers at the fix: `IS_ERR_OR_NULL` / `PTR_ERR_OR_ZERO`
(`git grep` on `705a3fd3bac16`).
- **[Phase 3]** `git blame 705a3fd3bac16^` around the call:
`d863dc3614e489` (2013).
- **[Phase 3]** `git log -S ext4_load_tail_bh -- fs/ext4/inode.c` →
`3b312a6f510ca`; `git describe --contains` → **v7.1-rc1**.
- **[Phase 3]** `git merge-base --is-ancestor 3b312a6f510ca HEAD` true;
`705a3fd3bac16` ancestor of HEAD true; **not** in v7.2; **in**
v7.3-rc1.
- **[Phase 3]** Parent `2fff82f081401` documents `-ERANGE` on inline
data if this check is added without the skip; that skip is in HEAD.
- **[Phase 4]** `b4 dig -c 705a3fd3bac16` → v4 3/9 lore URL; `-a` showed
v3 and v4; `-w` listed ext4 maintainers.
- **[Phase 4]** Cover letter (mbox): “Fold in two pre-existing fixes
(now patches 2 and 3)”.
- **[Phase 4]** WebFetch lore/msgid.link: **403**. Stable-list search:
**UNVERIFIED**.
- **[Phase 5]** Grep callers: `ext4_block_do_zero_range`,
`ext4_block_journalled_zero_range` → `ext4_block_zero_eof` /
`ext4_zero_partial_blocks` → truncate, setattr, punch-hole,
zero_range, `file.c` append.
- **[Phase 5]** Truncate: `ext4_truncate` → `ext4_block_zero_eof`; on
error `goto out_trace` (does **not** keep shrinking after a failed
zero). Punch-hole/`ext4_zero_range` return the error.
- **[Phase 6]** `git describe HEAD` = `v7.3-rc4-27-gf0100363d8c37`;
`make kernelversion` = `7.3.0-rc4`.
- **[Phase 6]** Buggy helper present in this tree since v7.1;
prerequisite and this commit both ancestors of HEAD.
- **[Phase 7]** MAINTAINERS: Zhang Yi is `R:` on EXT4; Jan Kara is `R:`;
Ted is `M:`.
- **[Phase 8]** Failure mode verified from commit text + truncate/punch
callers, not from a live repro (no reproducer in the message).
- **UNVERIFIED:** Whether `ext4_get_block` can return an error while
leaving `BH_Mapped` set (fix still correct: any `err < 0` is
propagated). No live I/O-error repro.
The patch is a small, reviewed ext4 integrity fix for code that exists
in this 7.3 tree, with callers already wired for `ERR_PTR` and the
inline-data prerequisite present.
**YES**
fs/ext4/inode.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 9a5c08650bd1e..2e3f7ba1a924b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4119,7 +4119,9 @@ static int __ext4_block_zero_page_range(handle_t *handle,
}
if (!buffer_mapped(bh)) {
BUFFER_TRACE(bh, "unmapped");
- ext4_get_block(inode, iblock, bh, 0);
+ err = ext4_get_block(inode, iblock, bh, 0);
+ if (err < 0)
+ goto unlock;
/* unmapped? It's a hole - nothing to do */
if (!buffer_mapped(bh)) {
BUFFER_TRACE(bh, "still unmapped");
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH AUTOSEL 6.18-5.10] ext4: check return value of ext4_get_block() in ext4_load_tail_bh()
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] ext4: check return value of ext4_get_block() in ext4_load_tail_bh() Sasha Levin
@ 2026-10-19 14:32 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-10-19 14:32 UTC (permalink / raw)
To: Sasha Levin; +Cc: linux-ext4, tytso
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ext4: unchecked ext4_block_truncate_page() return in truncate callers
--
commit 26fe59dd3c7737fcb95b3fe2c9732b862d57565e
Author: Zhang Yi <yi.zhang@huawei.com>
ext4: check return value of ext4_get_block() in ext4_load_tail_bh()
This patch checks and propagates the return value of ext4_get_block() in
__ext4_block_zero_page_range() so that I/O or allocation failures are not
silently ignored as holes during zero-range, truncate, or punch-hole
operations.
Link: https://patch.msgid.link/20260714080044.4038124-4-yi.zhang@huaweicloud.com
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index e99306a8f47ce..234dfe20ef4f3 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -4092,7 +4092,9 @@ static int __ext4_block_zero_page_range(handle_t *handle,
> }
> if (!buffer_mapped(bh)) {
> BUFFER_TRACE(bh, "unmapped");
> - ext4_get_block(inode, iblock, bh, 0);
> + err = ext4_get_block(inode, iblock, bh, 0);
> + if (err < 0)
> + goto unlock;
> /* unmapped? It's a hole - nothing to do */
> if (!buffer_mapped(bh)) {
> BUFFER_TRACE(bh, "still unmapped");
[Severity: Medium]
While this propagates the negative error from ext4_get_block() out of
__ext4_block_zero_page_range() via ext4_block_zero_page_range(),
ext4_block_truncate_page(), and ext4_zero_partial_blocks(), several
truncate and fallocate callers ignore the returned error.
In fs/ext4/inode.c, a shrinking truncate via
ext4_setattr()->ext4_truncate()->ext4_block_truncate_page() ignores the
return value and proceeds with block truncation:
if (inode->i_size & (inode->i_sb->s_blocksize - 1))
ext4_block_truncate_page(handle, mapping, inode->i_size);
Similarly, an extending truncate in ext4_setattr() ignores the error from
ext4_block_truncate_page() and proceeds to update i_disksize and i_size:
if (!shrink) {
inode_set_mtime_to_ts(inode,
inode_set_ctime_current(inode));
if (oldsize & (inode->i_sb->s_blocksize - 1))
ext4_block_truncate_page(handle,
inode->i_mapping, oldsize);
}
And in fs/ext4/extents.c, an extending fallocate or zero-range
preallocation in ext4_alloc_file_blocks() ignores the error returned by
ext4_zero_partial_blocks() when zeroing the old EOF tail block:
if (epos > old_size) {
pagecache_isize_extended(inode, old_size, epos);
ext4_zero_partial_blocks(handle, inode,
old_size, epos - old_size);
}
When the tail buffer_head is unmapped and ext4_get_block() fails, can
these paths still silently discard the error, update the inode size, and
return success with stale data left in the partial tail block?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019135441.1420968-141-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH AUTOSEL 6.18-5.10] ext4: guard against NULL s_group_info in ext4_get_group_info
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:44 ` [PATCH AUTOSEL 6.18-6.6] ext4: use FGP_WRITEBEGIN for tail block zeroing Sasha Levin
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-5.10] ext4: check return value of ext4_get_block() in ext4_load_tail_bh() Sasha Levin
@ 2026-10-19 13:49 ` Sasha Levin
2026-10-19 14:53 ` sashiko-bot
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] ext4: set EXT4_STATE_NO_EXPAND in ext4_evict_inode Sasha Levin
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-10-19 13:49 UTC (permalink / raw)
To: patches, stable
Cc: Junzhe Yu, Theodore Ts'o, Sasha Levin, linux-ext4,
linux-kernel
From: Junzhe Yu <junzheyu1@gmail.com>
[ Upstream commit 8f3901fbb40745046e513226ad84e8b84492632d ]
Resend: previous attempt was rejected by vger for containing HTML.
==================================================================
ext4_mark_group_bitmap_corrupted() already treats a NULL return from
ext4_get_group_info() as "nothing to do", but ext4_get_group_info()
indexes s_group_info without checking whether the array exists.
During mount, fast-commit replay runs inside jbd2_journal_load() from
ext4_load_and_init_journal(), which is before ext4_mb_init() allocates
s_group_info. Replaying an FC UNLINK for an inode whose bitmap bit is
already clear takes:
ext4_fc_replay_unlink -> iput -> ext4_evict_inode -> ext4_free_inode
-> ext4_mark_group_bitmap_corrupted -> ext4_get_group_info
and faults on the NULL s_group_info base. Userspace only mounts a dirty
ext4 image; this is a supported recovery path.
Return NULL when s_group_info (or the per-block grp_info row) is unset
so the existing caller check is effective during early mount.
Tested on Linux v6.6.145 KASAN: crafted FC-unlink image previously
triggered KASAN null-ptr-deref / panic in ext4_get_group_info; with this
patch, mount succeeds (EXT4 "bit already cleared" may still log). Also
observed on v6.6.144; still present on torvalds/linux as of
f5098b6bae76 (2026-07-26).
A self-contained Docker/QEMU reproducer (craft + mount + patch verify) is
available on request.
Signed-off-by: Yu Junzhe <junzheyu1@gmail.com>
Link: https://patch.msgid.link/65c955b0-716b-4599-b925-59c2782e38b4@gmail.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit message forensics
**Step 1.1 — Subject**
Record: **[ext4] [guard / prevent] NULL `s_group_info` dereference in
`ext4_get_group_info`**. “Guard against” is a bug-fix verb: add a
missing safety check.
**Step 1.2 — Tags**
Record:
- `Signed-off-by: Yu Junzhe <junzheyu1@gmail.com>` (author)
- `Link: https://patch.msgid.link/65c955b0-716b-4599-b925-
59c2782e38b4@gmail.com`
- `Signed-off-by: Theodore Ts'o <tytso@mit.edu>` (ext4 maintainer
applied it)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-
by:`, or `Cc: stable` (absence is expected in this pipeline)
- Upstream SHA after maintainer application:
`8f3901fbb40745046e513226ad84e8b84492632d`
**Step 1.3 — Body**
Record:
- **Bug:** `ext4_get_group_info()` indexes `s_group_info` without
checking that the array exists.
- **Symptom:** KASAN null-ptr-deref / panic in `ext4_get_group_info`
while mounting a dirty ext4 image that needs fast-commit UNLINK
replay.
- **Path:** `ext4_fc_replay_unlink` → `iput` → `ext4_evict_inode` →
`ext4_free_inode` → `ext4_mark_group_bitmap_corrupted` →
`ext4_get_group_info`
- **Root cause:** FC replay runs inside `jbd2_journal_load()` from
`ext4_load_and_init_journal()`, **before** `ext4_mb_init()` allocates
`s_group_info`.
- **Author testing:** KASAN on v6.6.145 (also seen on v6.6.144); still
present on torvalds/linux as of `f5098b6bae76` (2026-07-26). Mount
succeeds with the patch.
**Step 1.4 — Hidden bug fix?**
Record: Not hidden. This is an explicit NULL-deref fix. The “already-
cleared bitmap bit” path looks like a recovery edge case, but it is a
real oops on a supported mount/recovery path.
## Phase 2: Diff analysis
**Step 2.1 — Inventory**
Record:
- File: `fs/ext4/balloc.c` only (`+4 / -0`)
- Function: `ext4_get_group_info()`
- Scope: **single-file surgical fix**
**Step 2.2 — Code flow**
Record (verified against local `fs/ext4/balloc.c`):
Before:
```
if (group >= s_groups_count) return NULL;
indexv/indexh = ...
grp_info = sbi_array_rcu_deref(..., s_group_info, indexv); //
NULL[index] if array missing
return grp_info[indexh];
```
After:
```
if (group >= s_groups_count) return NULL;
if (!s_group_info) return NULL; // new
... deref row ...
if (!grp_info) return NULL; // new
return grp_info[indexh];
```
This is an early-mount / missing-table path, not a new feature path.
**Step 2.3 — Bug mechanism**
Record: **NULL pointer dereference (memory safety)**.
`sbi_array_rcu_deref()` does
`rcu_dereference(sbi->s_group_info)[index]`. `sbi` is `kzalloc`’d, so
`s_group_info` is NULL until `ext4_mb_alloc_groupinfo()` /
`rcu_assign_pointer`. The per-row check covers `ext4_mb_add_groupinfo()`
error paths that set `group_info[idx] = NULL`.
**Step 2.4 — Fix quality**
Record: Obviously correct. Matches the existing contract
(`ext4_get_group_info()` may return NULL;
`ext4_mark_group_bitmap_corrupted()` already does `if (!grp || !gdp)
return;`). Four `unlikely()` lines, no API/lock/data-structure change.
Regression risk is very low: after `ext4_mb_init()`, `s_group_info` is
non-NULL and these checks are no-ops. Returning NULL instead of oopsing
cannot be worse for callers that already handle NULL.
## Phase 3: Git history
**Step 3.1 — Blame**
Record: Current function body in this tree is from the 6.4-era change
“allow `ext4_get_group_info()` to fail” (this history: `b4319e457d6e3`;
mainline `5354b2af34064`, 2023-05-13) plus `fa285d799d1d5` (“drop
`ext4_error()` from `ext4_get_group_info()`”). Those commits taught the
helper to return NULL for a bad group number and taught callers to check
NULL — but they never checked the array pointer itself.
**Step 3.2 — Fixes: tag**
Record: None. Reachability comes from fast-commit replay, added in
`6866d7b3f2bb4` (“ext4 / jbd2: add fast commit initialization”,
`tags/v5.10-rc1~59^2~7`). Journal load still runs before
`ext4_mb_init()` in this tree.
**Step 3.3 — File history**
Record: Recent `fs/ext4/balloc.c` commits here are unrelated
(capability-check reorder, fault-injection race, kunit stubs). No
prerequisite series. Standalone 1/1 patch.
**Step 3.4 — Author**
Record: Yu Junzhe is not the ext4 maintainer. Theodore Ts’o applied it
by hand after noting mangled whitespace, then posted “Applied, thanks!”
with commit `8f3901fbb407`.
**Step 3.5 — Dependencies**
Record: None. The helper, `s_group_info`, FC replay, and the caller NULL
check all exist in this checkout. Applies standalone.
## Phase 4: Mailing list / external
**Step 4.1 — Original discussion**
Record:
- Lore/msgid: https://patch.msgid.link/65c955b0-716b-4599-b925-
59c2782e38b4@gmail.com
- Direct lore WebFetch: **403** (could not read lore HTML)
- Openwall/LKML mirrors and `b4` succeeded
- `b4 dig -a`: **v1 only** (no v2/v3)
- Thread: author submission; tytso note about gmail whitespace (still
applied); tytso “Applied, thanks!” — **no NAK**
- No reviewer `Cc: stable` in the thread (tytso also did not add it when
committing)
**Step 4.2 — Recipients (`b4 dig -w`)**
Record: Junzhe Yu, Theodore Ts’o, Andreas Dilger, linux-ext4, linux-
kernel. Both ext4 maintainers were on the original To/Cc.
**Step 4.3 — Bug report**
Record: No syzbot/bugzilla link. Author reports KASAN on a crafted FC-
unlink image and offers a Docker/QEMU reproducer. I did not run that
reproducer (UNVERIFIED empirically; the code path is verified).
**Step 4.4 — Series**
Record: Standalone `[PATCH]` 1/1. Related-in-time FC-replay crashes
(writeback rwsem not initialized, RO mount replay) are separate bugs,
not prerequisites.
**Step 4.5 — Stable list**
Record: lore.kernel.org/stable WebFetch returned **403**. No stable-list
discussion found via search. No evidence it was rejected for stable.
## Phase 5: Semantic / call-chain analysis
**Step 5.1 — Functions**
Record: `ext4_get_group_info()` only. (No semcode MCP in this
environment; used `git grep` + reads.)
**Step 5.2 — Callers**
Record: Many, across `fs/ext4/{balloc,ialloc,mballoc,super}.c`. Almost
all already handle NULL (`if (!grp)` / `if (!grinfo)`), because of the
6.4 “allow to fail” work. The crashing caller is
`ext4_mark_group_bitmap_corrupted()` in `fs/ext4/super.c`, which already
returns on NULL.
**Step 5.3 — Callees**
Record: `sbi_array_rcu_deref()` →
`rcu_dereference(s_group_info)[indexv]`. That is the NULL deref.
**Step 5.4 — Reachability**
Record: User-reachable: mount an ext4 filesystem with a journal that
still has fast-commit UNLINK tags, when the inode bitmap bit is already
clear. `ext4_fc_init()` installs `j_fc_replay_callback` even if fast
commit is currently off, so leftover FC blocks still replay. Mount of a
dirty ext4 image is a normal recovery path (crash, unclean unmount, or
an untrusted image). Typically needs mount privilege, but the failure is
a **kernel oops**, not a userspace error.
Verified mount order in this tree (`fs/ext4/super.c`):
1. `ext4_load_and_init_journal()` → `ext4_load_journal()` →
`jbd2_journal_load()` (FC replay)
2. much later: `ext4_mb_init()` (allocates `s_group_info`)
Verified free path: `ext4_free_inode()` skips the group-info corruption
check under `EXT4_FC_REPLAY`, then on `!cleared` still calls
`ext4_error` + `ext4_mark_group_bitmap_corrupted()`.
**Step 5.5 — Similar patterns**
Record: `ext4_count_free_clusters()` already does `if
(EXT4_SB(sb)->s_group_info)` before calling `ext4_get_group_info()`.
Bitmap validators skip work under `EXT4_FC_REPLAY`. The hole is
specifically the helper used by `ext4_mark_group_bitmap_corrupted()`.
## Phase 6: This checkout (6.12.y)
**Step 6.1 — Does the buggy code exist here?**
Record: **Yes.** Local tree is **6.12.111** (`pending-6.12`; `git
describe` was `p-6.12-3-g88406e77611c6` at start).
`ext4_get_group_info()` in `fs/ext4/balloc.c` still has only the
`s_groups_count` check. `git merge-base --is-ancestor 8f3901fbb407 HEAD`
→ **not an ancestor**. `git grep` for the new `!s_group_info` guard →
**not present**. Fast commit and the journal-before-mballoc order
**are** present.
**Step 6.2 — Backport difficulty**
Record: **Clean apply.** Local function text matches the upstream hunk
context exactly. Confirmed by reading the function and by comparing to
`git show 8f3901fbb407`.
**Step 6.3 — Related fixes already here?**
Record: The “allow to fail” / drop-`ext4_error` work is here
(`b4319e457d6e3`, `fa285d799d1d5`). That is why the caller already
treats NULL as success-noop — and why this four-line guard is
sufficient. No alternate fix for this crash is in the tree.
`origin/master` has `8f3901fbb407` as the latest `balloc.c` change; no
follow-up revert/amend on that function.
## Phase 7: Subsystem context
**Step 7.1**
Record: **ext4 (fs/ext4). Criticality: IMPORTANT** (default Linux
filesystem; not quite mm/VFS core, but production-critical).
**Step 7.2**
Record: Actively maintained (tytso/Dilger, linux-ext4). The hole is old
(FC since 5.10 + helper NULL contract since 6.4), not a brand-new
subsystem.
## Phase 8: Impact and risk
**Step 8.1 — Who is affected**
Record: Anyone mounting an ext4 filesystem that still has fast-commit
journal data to replay (crash recovery, or a crafted/dirty image). That
is a large, real population — not an obscure driver.
**Step 8.2 — Trigger**
Record: Mount-time FC UNLINK replay when the inode bitmap bit is already
clear. Not every boot, but a **supported recovery path**. Unprivileged
users cannot usually mount; a user who can mount a malicious ext4 image
(containers, loop, removable media with automount) can panic the kernel.
**Step 8.3 — Failure mode**
Record: **NULL deref → KASAN report / oops / panic during mount.**
Severity: **CRITICAL** (host crash while recovering a dirty filesystem).
**Step 8.4 — Risk/benefit**
Record: **Benefit: high** (prevents oops on ext4 recovery). **Risk: very
low** (4 lines, existing NULL contract, hot path unchanged after
`ext4_mb_init()`). Ratio strongly favors backport.
Note: `ext4_error("bit already cleared...")` can still log or remount-ro
depending on `errors=`; the author states mount succeeds with this
patch. That `ext4_error` is pre-existing, not introduced here.
## Phase 9: Synthesis
**Step 9.1 — Evidence**
FOR:
- Real NULL deref on a supported ext4 mount/recovery path
- KASAN-confirmed; author tested the patch
- Call path and journal-before-`mb_init` order verified in **this** 6.12
tree
- 4-line, obviously correct guard; callers already handle NULL
- Ext4 maintainer applied it upstream (`8f3901fbb407`); it is on
`origin/master`
- No dependencies; applies cleanly
- Ext4 is widely used; FC exists since 5.10
AGAINST:
- No `Fixes:` / `Cc: stable` (expected; not a negative)
- Trigger is an FC replay edge (already-cleared bit), not every mount
- I did not run the author’s QEMU reproducer
- tytso did not nominate stable in the apply mail
UNRESOLVED:
- Empirical reproducer not run here
- lore.kernel.org HTML and stable-list search returned HTTP 403 (thread
recovered via b4/openwall)
**Step 9.2 — Stable rules**
1. Obviously correct and tested? **PASS** (KASAN + maintainer apply;
logic matches existing NULL contract)
2. Real bug affecting users? **PASS** (oops on dirty-ext4 mount / FC
replay)
3. Important (crash/security/corruption/deadlock)? **PASS** — kernel
oops/panic, severity CRITICAL
4. Small and contained? **PASS** — +4 lines, one function
5. No new features/APIs? **PASS**
6. Can apply to this tree? **PASS** — buggy code present, clean context
**Step 9.3 — Exceptions**
Record: Not an ID/quirk/DT/docs exception. It is a normal crash fix.
**Step 9.4 — Decision**
This tree is 6.12.y. The bug exists here; the fix is a four-line NULL
guard with no prerequisites. It should be backported.
---
### What this solves for stable users
Mounting a dirty ext4 filesystem with leftover fast-commit UNLINK
records can take `ext4_free_inode()` →
`ext4_mark_group_bitmap_corrupted()` **before** mballoc has allocated
`s_group_info`. The helper then does `NULL[index]` and oopses. The patch
makes the helper return NULL so the existing caller check is a no-op and
mount/recovery can continue.
### Risk vs benefit
Benefit is a real mount-time crash fix on the default Linux filesystem.
Risk is a pair of `unlikely()` NULL returns in a helper that is already
allowed to fail. After `ext4_mb_init()`, behavior is unchanged.
### Concerns
None that block backport. No missing commits, no API change, no follow-
up on `origin/master` that reworks this function.
---
Verification:
- [Phase 1] Parsed subject/tags/body from the candidate message and from
`git show 8f3901fbb407`
- [Phase 1] Confirmed no Fixes/Cc:stable; maintainer SOB present
- [Phase 2] Diff is +4 lines in `ext4_get_group_info()`; local function
matches the hunk
- [Phase 2] `sbi_array_rcu_deref` indexes `s_group_info` without a NULL
check (`fs/ext4/ext4.h`)
- [Phase 2] `ext4_mark_group_bitmap_corrupted()` already returns if
`!grp` (`fs/ext4/super.c`)
- [Phase 3] Blame: helper NULL-for-bad-group from `b4319e457d6e3` /
`fa285d799d1d5` in this history
- [Phase 3] Fast commit added in `6866d7b3f2bb4` (v5.10-rc1); present
here
- [Phase 3] `git log --grep` on `fs/ext4/`: this subject is **not** in
HEAD
- [Phase 4] `b4 am` / `b4 dig -c 8f3901fbb407`: thread at msgid
`65c955b0-716b-4599-b925-59c2782e38b4@gmail.com`; v1 only
- [Phase 4] `b4 dig -w`: tytso + Dilger + linux-ext4 on the original
mail
- [Phase 4] tytso applied as `8f3901fbb407`; noted whitespace but no NAK
- [Phase 4] lore.kernel.org WebFetch: 403; used openwall + b4 mbox
instead
- [Phase 5] Callers of `ext4_get_group_info` grepped; crashing caller
checks NULL
- [Phase 5] `ext4_free_inode()` FC_REPLAY skip + “bit already cleared” →
`ext4_mark_group_bitmap_corrupted()` verified
- [Phase 5] Mount order: `jbd2_journal_load` in `ext4_load_journal()`
before `ext4_mb_init()` verified
- [Phase 5] `sbi = kzalloc` zeroes `s_group_info`; assigned later via
`rcu_assign_pointer` in mballoc
- [Phase 6] `make kernelversion` / Makefile: **6.12.111**; branch
`pending-6.12`
- [Phase 6] `git merge-base --is-ancestor 8f3901fbb407 HEAD` → not in
this tree
- [Phase 6] Same SHA **is** an ancestor of `origin/master`; no later
balloc.c follow-up on that fix
- [Phase 6] Clean apply context present in local `ext4_get_group_info()`
- [Phase 8] Failure mode: NULL deref oops on mount/recovery, severity
CRITICAL
- UNVERIFIED: Did not run the author’s crafted-image/QEMU reproducer
- UNVERIFIED: Could not search lore.kernel.org/stable HTML (HTTP 403)
**YES**
fs/ext4/balloc.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index c9329ed5c094e..08556dd45760d 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -331,9 +331,13 @@ struct ext4_group_info *ext4_get_group_info(struct super_block *sb,
if (unlikely(group >= EXT4_SB(sb)->s_groups_count))
return NULL;
+ if (unlikely(!EXT4_SB(sb)->s_group_info))
+ return NULL;
indexv = group >> (EXT4_DESC_PER_BLOCK_BITS(sb));
indexh = group & ((EXT4_DESC_PER_BLOCK(sb)) - 1);
grp_info = sbi_array_rcu_deref(EXT4_SB(sb), s_group_info, indexv);
+ if (unlikely(!grp_info))
+ return NULL;
return grp_info[indexh];
}
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH AUTOSEL 6.18-5.10] ext4: set EXT4_STATE_NO_EXPAND in ext4_evict_inode
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.10] ext4: guard against NULL s_group_info in ext4_get_group_info Sasha Levin
@ 2026-10-19 13:50 ` Sasha Levin
2026-10-19 14:55 ` sashiko-bot
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-6.12] ext4: reject mount if clusters/inodes per group are not 8-aligned Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] ext4: reduce max cluster size to match documented 256MB limit Sasha Levin
5 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-10-19 13:50 UTC (permalink / raw)
To: patches, stable
Cc: Yun Zhou, Jan Kara, Theodore Ts'o, Sasha Levin, linux-ext4,
linux-kernel
From: Yun Zhou <yun.zhou@windriver.com>
[ Upstream commit 2c0e4fb511f97467dafc91b6afb3414100289081 ]
An inode being evicted will never need its extra isize expanded. Set
EXT4_STATE_NO_EXPAND before ext4_mark_inode_dirty() in ext4_evict_inode()
to make this explicit and prevent any unnecessary work in
ext4_try_to_expand_extra_isize().
This also provides defense-in-depth for the s_writepages_rwsem deadlock
during mount-time orphan cleanup, ensuring the expand path is blocked
for inodes under eviction regardless of how they are reached.
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20260623061903.2148767-2-yun.zhou@windriver.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I have the series context, Jan Kara’s review, and the syzbot stack.
Compiling the nine-phase analysis and the backport verdict for this
tree.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[ext4] [set] set EXT4_STATE_NO_EXPAND in ext4_evict_inode**
- Subsystem: `ext4`
- Action verb: **set** (the body also uses “prevent” and “ensuring … is
blocked”)
- Claimed intent: mark an inode under eviction so extra-isize expansion
is skipped
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Reviewed-by:** Jan Kara `<jack@suse.cz>` (ext4 reviewer / long-time
maintainer)
- **Link:** `https://patch.msgid.link/20260623061903.2148767-2-
yun.zhou@windriver.com` (patch **2/2**)
- **Signed-off-by:** Yun Zhou `<yun.zhou@windriver.com>` (author)
- **Signed-off-by:** Theodore Ts'o `<tytso@mit.edu>` (ext4 maintainer;
merged)
- **No Fixes:** tag on the committed version (expected for this review
pipeline)
- **No Cc: stable** (expected)
- **No Reported-by / Closes** on this revision (those tags live on patch
1/2)
Notable: this is explicitly patch **2/2** of a two-patch series.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug description:** An evicted inode never needs extra isize
expanded. Doing that work is unnecessary and can participate in the
`s_writepages_rwsem` deadlock during mount-time orphan cleanup.
- **Symptom/failure mode:** Circular lock dependency / deadlock
(`s_writepages_rwsem` → jbd2 handle → `xattr_sem` →
`s_writepages_rwsem`).
- **Version information:** none in this commit; sibling patch 1/2 has
`Fixes: c8585c6fcaf2` (v4.7-rc1).
- **Author’s root-cause framing:** skip expansion by setting
`EXT4_STATE_NO_EXPAND` before `ext4_mark_inode_dirty()` in
`ext4_evict_inode()`. Described as making the invariant explicit
**and** as defense-in-depth for the mount-time orphan deadlock.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Yes — this is a hidden/downplayed deadlock-path fix, not mere
cleanup.**
The committed text says “unnecessary work” and “defense-in-depth”. The
**v1** of this exact one-line change was titled *“skip extra isize
expansion on inode eviction to avoid deadlock”*, carried the syzbot
`Reported-by`/`Closes`/`Fixes:` tags, and the syzbot stack is
`ext4_orphan_cleanup` → `evict` → **`ext4_evict_inode`** →
`__ext4_mark_inode_dirty` → `ext4_try_to_expand_extra_isize`. The tags
moved to patch 1/2 (`!SB_ACTIVE`) after Jan Kara asked for the real
lock-order fix; this line remained as the eviction-path closer.
---
## PHASE 2: DIFF ANALYSIS - LINE BY LINE
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **Files:** `fs/ext4/inode.c` — **+1 / −0**
- **Function:** `ext4_evict_inode()`
- **Scope:** single-file, one-line surgical change
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** deletion path sets `i_size = 0`, then calls
`ext4_mark_inode_dirty()`. That calls `__ext4_mark_inode_dirty()` → if
`i_extra_isize < s_want_extra_isize`,
`ext4_try_to_expand_extra_isize()`, which may take `xattr_sem`, move
xattrs, and `iput()` EA inodes.
- **After:** `ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND)` runs
first. `ext4_try_to_expand_extra_isize()` already returns `-EOVERFLOW`
when that flag is set (check present since 2017), so expansion is
skipped for this and later `ext4_mark_inode_dirty()` calls in the same
eviction.
- **Path:** deletion path only (after the `i_nlink` / `is_bad_inode`
early outs; not the cache-drop `no_delete` path).
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** deadlock / lock-ordering (plus skipping pointless
expansion that can `iput` EA inodes under a jbd2 handle).
- **Mechanism:** mount-time orphan cleanup (`!SB_ACTIVE`) of a deleted
inode (`i_nlink == 0`) hits `ext4_evict_inode` → expand extra isize →
`ext4_xattr_block_set` → `iput(ea_inode)` → `write_inode_now()` →
`s_writepages_rwsem`, while a jbd2 handle and `xattr_sem` are held.
Setting `EXT4_STATE_NO_EXPAND` removes that edge for every inode under
eviction.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct. An inode being deleted cannot benefit
from extra isize. The flag’s save/restore in `ext4_write_lock_xattr` /
`ext4_write_unlock_xattr` keeps it set (`*save == 1` → do not clear).
- **Regression risk:** very low. One inode-state bit on an inode that is
about to be freed. No API, locking, or on-disk format change.
- **Red flags:** none (1 line, one function). It is patch 2/2 but does
**not** depend on patch 1’s `!SB_ACTIVE` check.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Added line: commit **`2c0e4fb511f97`** (this change, 2026-06-23).
- Surrounding `inode->i_size = 0` / `ext4_mark_inode_dirty()`: Dave
Kleikamp 2006 / Ted Ts’o **`bc965ab3f2b4b7`** (2008). Extra-isize
expansion from dirtying: **`6dd4ee7cab7e3`** (2007).
`EXT4_STATE_NO_EXPAND` early-out in `ext4_try_to_expand_extra_isize`:
**`cf0a5e818fe216`** (2017, from **`2e81a4eeedcaa`** in 2016).
- This path has been in-tree for many years; not a newly introduced
subsystem.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag on this commit.** Sibling **`7461c60b9c6a8`**
(patch 1/2, parent of this commit) has `Fixes: c8585c6fcaf2` (“ext4: fix
races between changing inode journal mode and ext4_writepages”,
2016-04-25, **v4.7-rc1**). That commit is an ancestor of this tree. It
introduced `s_writepages_rwsem` (originally `s_journal_flag_rwsem`),
which is the lock in the cycle.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- **`7461c60b9c6a8`** — skip expansion when `!SB_ACTIVE` (primary mount-
time fix).
- **`2c0e4fb511f97`** — this commit (eviction-path flag).
- **`4f48af3dc7554` / `a54778243e88f`** — later deferred `iput` for EA
inodes (Jan Kara’s deeper lock-order fix; larger).
- This change is **standalone**: it only uses `ext4_set_inode_state()`
and a flag check that has existed since 2017. It is part of a series
but is not patch-X-that-needs-Y to compile or run.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Yun Zhou is a regular ext4 contributor (orphan/xattr/deadlock
work in this area), not the maintainer. Ted Ts’o merged it; Jan Kara
reviewed it. Related author commits: the `!SB_ACTIVE` skip, then
deferred EA `iput` helpers.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **No hard prerequisites beyond code that is already old.** Needs
`EXT4_STATE_NO_EXPAND` and the test at the top of
`ext4_try_to_expand_extra_isize()` (2016–2017). Does **not** need patch
1. Applies as a single insertion before the existing
`ext4_mark_inode_dirty()` call.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig:** `[PATCH 2/2] ext4: set EXT4_STATE_NO_EXPAND in
ext4_evict_inode` — `https://patch.msgid.link/20260623061903.2148767-
2-yun.zhou@windriver.com`
- **Revisions:** evolved from standalone v1 (2026-06-11) through v5–v8
of a larger xattr-iput series, then a simplified 2-patch series that
Ted applied. Applied version is that final 2/2.
- **v6/v8 cover letter (author):** “Patch 2 is a belt-and-suspenders
semantic improvement: an inode under eviction never needs extra isize
expansion.” “v1 … Only fixes the syzbot reproducer, not the underlying
lock ordering violation.”
- **Jan Kara (2026-06-11) on the original identical change:** agrees
expansion is pointless in `ext4_evict_inode`; says it **does not fix
the real lock-order bug**, only the particular syzbot reproducer. No
NAK of this line; he wanted a deeper fix as well.
- **No NAK** of setting the flag. Ted merged it with Jan’s Reviewed-by.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (b4 dig -w): `tytso@mit.edu`, `adilger.kernel@dilger.ca`,
`libaokun@linux.alibaba.com`, `jack@suse.cz`, `ojaswin@linux.ibm.com`,
`ritesh.list@gmail.com`, `yi.zhang@huawei.com`, `linux-
ext4@vger.kernel.org`. Subsystem maintainers/reviewers were on To/Cc.
Jan Kara reviewed; Ted merged.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record:
- Syzbot: `https://syzkaller.appspot.com/bug?extid=5d19358d7eb30ffb0cc5`
— **possible deadlock in wait_transaction_locked**, prio:high, C
repro, Denial-of-service via filesystem mount.
- Stack (verified): `ext4_orphan_cleanup` → `evict` →
**`ext4_evict_inode`** → `__ext4_mark_inode_dirty` →
`ext4_try_to_expand_extra_isize` → `ext4_expand_extra_isize_ea` →
`ext4_xattr_block_set` → `iput` → `write_inode_now` →
`ext4_writepages` / `s_writepages_rwsem`.
- Still reported on linux-5.15 / 6.1 / 6.6 at the time of the dashboard
snapshot. Syzbot’s “fix commit” is **`7461c60b9c6a`** (patch 1),
because that commit carries the tags; this commit closes the **same**
`ext4_evict_inode` frame.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Series is (1) skip expansion during mount, (2) this flag in
evict, then later deferred-iput patches. Patch 1 covers all `!SB_ACTIVE`
callers (including `ext4_process_orphan` → `ext4_truncate`). Patch 2
covers **all** eviction, including post-mount. They complement; this one
is not incomplete on its own for the syzbot stack.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: No stable-list thread found for this subject. Absence of Cc:
stable is expected for this pipeline.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: **`ext4_evict_inode()`** (modified). Indirectly
**`__ext4_mark_inode_dirty()`**, **`ext4_try_to_expand_extra_isize()`**.
**Step 5.2: TRACE CALLERS**
Record:
- `ext4_evict_inode` is `ext4_sops.evict_inode` (`fs/ext4/super.c`).
Called from VFS `evict()` on last `iput` when `i_nlink == 0`.
- Reachable from unlink/close, and from **`ext4_process_orphan()` →
`iput()`** during `ext4_orphan_cleanup()` at mount
(`fs/ext4/orphan.c`). That is the syzbot path.
**Step 5.3: TRACE CALLEES**
Record: After the new flag: `ext4_mark_inode_dirty` →
`__ext4_mark_inode_dirty` → `ext4_try_to_expand_extra_isize` (now a no-
op via `-EOVERFLOW`) → later `ext4_truncate`, `ext4_xattr_delete_inode`,
`ext4_free_inode`. No new callees.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: **Yes, userspace-reachable.** Unprivileged unlink hits eviction
(no deadlock there once `SB_ACTIVE` is set). The deadlock needs mount-
time orphan cleanup (`!SB_ACTIVE`), typically `mount(2)` (admin /
crafted image). Syzbot reproduced it. Assessment: DoS, filesystem
trigger.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Only this site sets `EXT4_STATE_NO_EXPAND` outside the xattr
lock helpers. No sibling-driver clone. Patch 1 is the related early-
return in `ext4_try_to_expand_extra_isize`.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Local tree: **`v7.3-rc4-27-gf0100363d8c37`**, `make kernelversion` =
**7.3.0-rc4** (mainline, not a `*.y` stable branch).
- `ext4_evict_inode` + `ext4_mark_inode_dirty` + expand-on-dirty all
exist.
- This commit **is already an ancestor** of HEAD (`2c0e4fb511f97`). That
is expected (source tree), not a reason to treat it as “already
selected for stable”.
- Bug class dates to extra-isize expansion (2007) plus
`s_writepages_rwsem` (2016 / v4.7).
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** Insert one call between long-stable
`inode->i_size = 0` and `ext4_mark_inode_dirty()`. No refactor of that
sequence. `EXT4_STATE_NO_EXPAND` exists here.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: Patch 1 (`7461c60b9c6a8`) and later deferred-iput work are also
in this mainline checkout. They do not make this line wrong or redundant
for stable: patch 1 is mount-only; this flag also covers post-mount
eviction and is the direct closer of the syzbot `ext4_evict_inode`
frame.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **ext4 (`fs/ext4/`)** — **IMPORTANT** (default filesystem on a
large fraction of Linux systems; mount/orphan cleanup is a common path
after crash).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively developed (recent writeback, xattr, orphan, deadlock
fixes). The eviction dirtying sequence itself has been stable for many
years; this is a small invariant on old code, not a feature on new code.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Filesystem-specific (ext4), but a very large user
population.** Triggered on mount of a filesystem with orphans that still
need extra-isize expansion and have xattrs that must move.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record:
- **Deadlock:** mount-time orphan cleanup, inode with `i_extra_isize <
s_want_extra_isize`, expansion moves xattrs, `!SB_ACTIVE` forces
`write_inode_now` on EA `iput`. Syzbot has a C repro.
- **Post-mount:** every deletion of such an inode would previously
attempt a pointless expand (not this deadlock).
- **Unprivileged mount:** generally no; crafted/unclean ext4 images and
admin mount yes. Syzbot still reached it from syscall fuzzing.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: **Deadlock / lockdep circular dependency → hang or lockdep
splat. Severity: HIGH (DoS on mount/orphan cleanup).** Not a data-
corruption bug.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for a 1-line closer of a documented, syzbot-
reproduced deadlock path on ext4 eviction; also stops pointless expand
(xattr shuffle / EA `iput`) on every delete.
- **Risk:** very low (1 line, existing flag, deletion-only).
- **Ratio:** strongly favorable.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR backporting:**
- Syzbot C-repro deadlock; stack is exactly `ext4_evict_inode` → mark
dirty → expand extra isize.
- 1-line, obviously correct; Jan Kara Reviewed-by; Ted Ts’o merged.
- Uses a flag and check that have existed since 2016–2017; no new API.
- Independent of patch 1; complements the `!SB_ACTIVE` skip.
- Defense-in-depth on a HIGH-severity hang in a core filesystem.
- Applies cleanly to this tree’s `ext4_evict_inode`.
**AGAINST:**
- Final message frames it as “unnecessary work” / “defense-in-depth”;
author called v1 incomplete vs the real lock-order bug.
- Patch 2/2; the tagged “real” mount fix is patch 1.
- Jan Kara: this line alone does not fix `ext4_xattr_block_set` taking
`s_writepages_rwsem` under a handle.
- Deadlock is mount-time / orphan-specific, not every unlink.
**UNRESOLVED:** lore.kernel.org returned 403 to WebFetch; discussion
recovered via b4 dig, openwall/lkml mirrors, patchew, and the syzbot
dashboard. No stable@ thread found.
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — 1-line invariant; Reviewed-
by Jan Kara; in mainline; syzbot exercised the path.
2. Fixes a real bug that affects users? **PASS** — same path as the
syzbot deadlock; also skips harmful extra work on delete.
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
deadlock / DoS (HIGH), not theoretical.
4. Small and contained? **PASS** — +1 line, one function.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS** — call site and
`EXT4_STATE_NO_EXPAND` are present; insertion is mechanical.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not a device-ID/quirk/DT/docs/build exception. It is a normal
small correctness/deadlock-path fix.
**Step 9.4: MAKE YOUR DECISION**
This tree is **7.3-rc4 mainline**. The eviction dirtying path exists;
the extra-isize expand-on-dirty path exists; the syzbot stack is this
function. The committed wording is modest, but the change is the direct
closer of a real, reproduced deadlock edge, not a refactor or a new
feature. Risk is negligible. It should be backported.
---
### What the commit solves for stable users
On ext4, `ext4_mark_inode_dirty()` may try to grow `i_extra_isize`.
During **mount-time orphan cleanup**, that runs from
**`ext4_evict_inode`** while `!SB_ACTIVE`. Expanding can move xattrs and
`iput()` EA inodes, which then calls `write_inode_now()` and takes
`s_writepages_rwsem` under a jbd2 handle and `xattr_sem`. Syzbot
reproduced that cycle. Setting `EXT4_STATE_NO_EXPAND` before the dirty
call is correct (the inode is being deleted) and stops that expand/iput
edge for every eviction.
### Stable rules / risk
Meets all six criteria: one line, reviewed, real deadlock path, no new
API, applies cleanly. Sibling patch 1 (`!SB_ACTIVE`) is the broader
mount fix; this line does not need it, and it still helps post-mount by
skipping pointless expansion.
### Concerns
Incomplete as the *only* lock-order fix (Jan Kara). That is a reason to
also want patch 1 / deferred iput, not a reason to drop this 1-liner. No
backport rework needed on this tree.
---
Verification:
- [Phase 1] Subject/tags: ext4 set NO_EXPAND; Reviewed-by Jan Kara; Link
patch 2/2; Ted Ts’o SOB; no Fixes/Reported-by on this revision
- [Phase 1] Body: skip expand on evict; defense-in-depth for
s_writepages_rwsem deadlock
- [Phase 2] Diff: +1 line in `ext4_evict_inode` before
`ext4_mark_inode_dirty`
- [Phase 2] Expand skip: `ext4_try_to_expand_extra_isize` returns
`-EOVERFLOW` if `EXT4_STATE_NO_EXPAND` (inode.c:6597)
- [Phase 2] Flag restore: `ext4_write_unlock_xattr` clears only if
`*save == 0` (xattr.h:166-170)
- [Phase 3] `git describe HEAD`: v7.3-rc4-27-gf0100363d8c37; `make
kernelversion`: 7.3.0-rc4
- [Phase 3] `git merge-base --is-ancestor 2c0e4fb511f97 HEAD`: yes
(mainline already has it)
- [Phase 3] Blame: `i_size=0` 2006; `mark_inode_dirty` in evict
`bc965ab3f2b4b7` 2008; expand-on-dirty `6dd4ee7cab7e3` 2007; NO_EXPAND
check `cf0a5e818fe216` 2017
- [Phase 3] Sibling 7461c60b9c6a8 Fixes: c8585c6fcaf2 (v4.7-rc1);
`merge-base --is-ancestor c8585c6fcaf2 HEAD`: yes
- [Phase 4] `b4 dig -c 2c0e4fb511f97`: msgid 20260623061903.2148767-2
- [Phase 4] `b4 dig -a`: v5–v8 larger series then final 2-patch series
- [Phase 4] `b4 dig -w`: tytso, adilger, jack@suse.cz, linux-ext4 on
To/Cc
- [Phase 4] Jan Kara 2026-06-11 (openwall/lkml): agrees with the flag;
says it only covers the syzbot reproducer, not the full lock-order bug
— no NAK
- [Phase 4] Cover letter: “belt-and-suspenders semantic improvement”
- [Phase 4] Syzbot bug extid=5d19358d7eb30ffb0cc5: stack through
`ext4_evict_inode`; prio high; C repro; linux-5.15/6.1/6.6 still
listed
- [Phase 4] lore.kernel.org WebFetch: 403; used b4 + mirrors. Stable
list search: no hits
- [Phase 5] Caller: `ext4_sops.evict_inode`; orphan:
`ext4_process_orphan` → `iput` (orphan.c:359)
- [Phase 6] Local `ext4_evict_inode` already contains the line at
inode.c:268; apply would be clean on the pre-patch form of the same
function
- [Phase 7] ext4, IMPORTANT, actively developed
- [Phase 8] Failure: circular lockdep / deadlock on mount orphan
cleanup; severity HIGH; risk very low
- UNVERIFIED: live lore.kernel.org HTML of the thread (403);
reconstructed from b4, openwall, patchew, syzbot
**YES**
fs/ext4/inode.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index bbc437be5e63b..9a5c08650bd1e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -261,6 +261,7 @@ void ext4_evict_inode(struct inode *inode)
if (ext4_inode_is_fast_symlink(inode))
memset(EXT4_I(inode)->i_data, 0, sizeof(EXT4_I(inode)->i_data));
inode->i_size = 0;
+ ext4_set_inode_state(inode, EXT4_STATE_NO_EXPAND);
err = ext4_mark_inode_dirty(handle, inode);
if (err) {
ext4_warning(inode->i_sb,
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH AUTOSEL 6.18-6.12] ext4: reject mount if clusters/inodes per group are not 8-aligned
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (3 preceding siblings ...)
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.10] ext4: set EXT4_STATE_NO_EXPAND in ext4_evict_inode Sasha Levin
@ 2026-10-19 13:51 ` Sasha Levin
2026-10-19 15:06 ` sashiko-bot
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.10] ext4: reduce max cluster size to match documented 256MB limit Sasha Levin
5 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-10-19 13:51 UTC (permalink / raw)
To: patches, stable
Cc: Baokun Li, Theodore Ts'o, Sashiko, Jan Kara, Zhang Yi,
Sasha Levin, linux-ext4, linux-kernel
From: Baokun Li <libaokun@linux.alibaba.com>
[ Upstream commit d42dc7726a003262b73ffef2b744ff57a5488799 ]
The block and inode bitmap checksums are computed over a whole number of
bytes: ext4_inode_bitmap_csum_*() use EXT4_INODES_PER_GROUP(sb) >> 3 and
ext4_block_bitmap_csum_*() use EXT4_CLUSTERS_PER_GROUP(sb) / 8 as the
length passed to ext4_chksum().
If s_inodes_per_group or s_clusters_per_group is not a multiple of 8, the
trailing fractional bits are excluded from the checksum. Those bits are
then unprotected, and any incremental csum update path that assumes a
byte-aligned bitmap can compute a checksum inconsistent with the full
recalculation, corrupting the on-disk bitmap checksum.
Reject such filesystems at mount time by adding the missing " & 7"
alignment checks alongside the existing range validation.
Suggested-by: Theodore Ts'o <tytso@mit.edu>
Link: https://patch.msgid.link/h3n7jlfhyna64dn5o76qxcspnhxdddcs6crpxftmy7gnl7b3sx@jenszfpcsnit
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260508121539.4174601-1-libaokun%40linux.alibaba.com?part=10
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Zhang Yi <yi.zhang@huawei.com>
Link: https://patch.msgid.link/20260608111150.827117-2-libaokun@linux.alibaba.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
This is a small ext4 mount-time geometry check. Local tree is
**7.3.0-rc4** (`v7.3-rc4-27-gf0100363d8c37`). The candidate is
`d42dc7726a003`. The checksum path still uses byte-truncated lengths, so
the bug this closes is real in this tree.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[ext4] [reject] reject mount if clusters/inodes per group are
not 8-aligned**
Subsystem `ext4`. Verb `reject` is a defensive mount-time check, not a
feature or refactor.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Suggested-by:** Theodore Ts'o `<tytso@mit.edu>` (ext4 maintainer)
- **Link:** `https://patch.msgid.link/h3n7jlfhyna64dn5o76qxcspnhxdddcs6c
rpxftmy7gnl7b3sx@jenszfpcsnit` (Ted’s suggestion)
- **Reported-by:** Sashiko `<sashiko-bot@kernel.org>` (static-analysis
bot, not syzbot)
- **Closes:** sashiko.dev URL pointing at RFC series part 10
- **Signed-off-by:** Baokun Li `<libaokun@linux.alibaba.com>` (author)
- **Reviewed-by:** Jan Kara `<jack@suse.cz>` (ext4 co-maintainer)
- **Reviewed-by:** Zhang Yi `<yi.zhang@huawei.com>`
- **Link:** `https://patch.msgid.link/20260608111150.827117-2-
libaokun@linux.alibaba.com` (v2 1/3)
- **Signed-off-by:** Theodore Ts'o `<tytso@mit.edu>` (applied)
- **No Fixes:** tag (expected)
- **No Cc: stable** (expected)
- Ignore pipeline markers: none of `[Upstream commit]` / Sasha Levin SOB
on this commit
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** inode/block bitmap checksums hash a whole number of bytes
(`EXT4_INODES_PER_GROUP >> 3`, `EXT4_CLUSTERS_PER_GROUP / 8`). If
those counts are not multiples of 8, the last 1–7 bits are omitted
from the CRC.
- **Symptom:** those bits are unprotected; an incremental CRC path that
assumes byte alignment can disagree with a full recalculation and
write a bad on-disk bitmap checksum.
- **Version info:** none in the message.
- **Root cause (author):** missing `& 7` checks next to the existing
range checks in `ext4_handle_clustersize()` /
`ext4_block_group_meta_init()`.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: **Not hidden.** Explicit mount rejection for invalid superblock
geometry. Cover letter (`[PATCH v2 0/3]`) states the series prevents
crafted images from causing bitmap checksum corruption, integer
overflow, or OOB inode-table access. This patch is the
checksum/alignment piece.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `fs/ext4/super.c` only (`+6 / -4`)
- **Functions:** `ext4_handle_clustersize()`,
`ext4_block_group_meta_init()`
- **Scope:** single-file surgical fix, two existing `if` conditions
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Hunk 1 (`ext4_handle_clustersize`):** previously rejected only
`s_clusters_per_group > blocksize * 8`. After: also reject
`s_clusters_per_group & 7`. Error text becomes `"invalid #clusters per
group"`.
- **Hunk 2 (`ext4_block_group_meta_init`):** previously rejected inodes-
per-group outside `[inodes_per_block, blocksize*8]`. After: also
reject `s_inodes_per_group & 7`. Trailing `\n` in the format string
dropped.
- **Path:** mount / `__ext4_fill_super()` only. Legitimate 8-aligned
filesystems are unchanged.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **[Logic / correctness + metadata integrity]**
Verified in `fs/ext4/bitmap.c`:
```31:31:fs/ext4/bitmap.c
sz = EXT4_INODES_PER_GROUP(sb) >> 3;
```
```68:68:fs/ext4/bitmap.c
int sz = EXT4_CLUSTERS_PER_GROUP(sb) / 8;
```
Set and verify use the same truncated length, so kernel-internal CRC is
consistent. Trailing bits are still live inode/cluster bits
(`ext4_set_bit` in alloc/free) but are **not** covered by
`metadata_csum`. `ialloc.c` zeros with `(EXT4_INODES_PER_GROUP(sb) + 7)
/ 8` (round up) while CRC uses `>> 3` (round down).
The “incremental csum update” failure mode is what Sashiko flagged in
**unmerged** RFC helper `ext4_inode_bitmap_csum_set_fast()`.
`crc32c_flip_range` is **not** in this tree. The gap that **is** in this
tree is unprotected trailing bitmap bits plus disagreement with any tool
that hashes a different length.
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct. `& 7` matches the CRC byte length.
Same style as the existing upper-bound checks.
- **Regression risk:** only rejects non-8-aligned geometries. Ted
documented that old Android `make_ext4fs` could emit those; he also
said that was fixed more than a decade ago and those devices should be
gone. Follow-up `a34de48f329b2` only adds another conjunct on the same
`if`; it does not revert this check.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Cluster range check introduced in `40da553f5da02` (“ext4: verify
s_clusters_per_group even without bigalloc”, 2024-03-07, Jan Kara).
- Inode range check lived in this helper since `107d2be90116a` (2023
factor-out); the `blocksize * 8` bound is older (`c8267c51425ab` moved
a local).
- Bitmap CRC with `/ 8` / `>> 3` dates to `41a246d1ff75a` /
`fa77dcfafeaa6` (2012, metadata_csum).
- Alignment checks themselves are `d42dc7726a003` (this commit).
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: **No Fixes: tag.** Buggy assumption is as old as metadata_csum
(2012), not a recent regression.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- Same series, independent patches:
- `d42dc7726a003` — this commit (v2 1/3)
- `afa3caf2fbc41` — max cluster size (v2 2/3)
- `a34de48f329b2` — inodes_per_group % inodes_per_block (v2 3/3)
- Cover letter: patches 2 and 3 are “independent of Patch 1”.
- This patch does not need 2/3.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Baokun Li is a regular ext4 contributor (many DIO/DAX and
validation patches). Not the maintainer; Ted applied it. Jan Kara and
Zhang Yi reviewed.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: **Standalone.** Touches existing `if`s in helpers that exist
here (`ext4_handle_clustersize` at `super.c:4461`,
`ext4_block_group_meta_init` at `super.c:5294`). No new helpers, no
incremental CRC.
`git merge-base --is-ancestor d42dc7726a003 HEAD` → **yes** (exit 0).
Already in this checkout via `ext4_for_linus-7.3-rc1`.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig -c d42dc7726a003:** `https://patch.msgid.link/20260608111150.
827117-2-libaokun@linux.alibaba.com`
- **Revisions:**
- v1 RFC: part of 17-patch incremental CRC series (`lib/crc: add
crc32c_flip_range()`), 2026-05-08
- v2: `[PATCH v2 0/3] ext4: tighten mount-time superblock geometry
validation`, 2026-06-08 (this is what landed)
- v2 vs v1 for **this** patch: only dropped a spurious `\n` in the error
format.
- **Reviews:** Zhang Yi “Looks good”; Jan Kara “Looks good”; Ted
“Applied, thanks!” with the three SHAs.
- **No NAKs.** No reviewer asked for `Cc: stable`.
- lore.kernel.org HTTP **403**; thread recovered via `b4 dig -m`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (**b4 dig -w**): linux-ext4, Ted Ts'o, Andreas Dilger, Jan Kara,
Zhang Yi, Ojaswin Mujoo, Ritesh, Sashiko. Subsystem maintainers were on
To/Cc. Jan Kara and Zhang Yi reviewed this patch; Ted applied it.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: Sashiko High finding on RFC 10/17
`ext4_inode_bitmap_csum_set_fast()`: incremental CRC would corrupt the
checksum if `INODES_PER_GROUP` is not a multiple of 8.
Ted’s reply (verified from mbox
`h3n7jlfhyna64dn5o76qxcspnhxdddcs6crpxftmy7gnl7b3sx@jenszfpcsnit`):
- Kernel historically **did not** enforce 8-alignment because old
Android `make_ext4fs` (MIT, pre-e2fsprogs) sometimes created non-
multiples of 8.
- That hit e2fsck ([sourceforge e2fsprogs bug
292](https://sourceforge.net/p/e2fsprogs/bugs/292/)).
- “Fixed long ago (over a decade ago)… any such mobile handsets are in
the landfill, so we probably should fix this by adding a check in
ext4_fill_super().”
So this is maintainer-directed policy: stop mounting geometries e2fsck
already treats as invalid.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: v2 is three independent mount checks. Incremental CRC RFC was
**not** merged (`crc32c_flip_range` absent). This patch does not pull in
that series.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: **UNVERIFIED** — lore.kernel.org/stable returned 403. No stable
discussion in the ext4 mbox.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: IDENTIFY KEY FUNCTIONS**
Record: `ext4_handle_clustersize`, `ext4_block_group_meta_init`.
Supporting: `ext4_inode_bitmap_csum_{set,verify}`,
`ext4_block_bitmap_csum_{set,verify}`.
**Step 5.2: TRACE CALLERS**
Record: both are called only from `__ext4_fill_super()` (`super.c:5443`
and `5451`). Mount path, not hot I/O. Reaching the bug is “mount this
filesystem” (loop, USB, etc.).
**Step 5.3: TRACE CALLEES**
Record: read superblock fields, `ext4_msg`, `return -EINVAL`. No alloc,
no new locks.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: `mount` / `fsopen` → `ext4_fill_super` → these checks.
Unprivileged users can hit it with a crafted image on a loop device if
they can mount (or via automount). **Userspace-reachable.**
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: same file already rejects other invalid geometry (blocks vs
clusters, inodes_per_block). Series patch 3 adds `% inodes_per_block` on
the same inode `if`. `ext4_count_free()` debug paths also use `/ 8`, so
free-count vs last bits can disagree on unaligned groups.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** This tree is **7.3.0-rc4**. `bitmap.c` still uses `>>
3` and `/ 8`. The alignment checks are already present at
`super.c:4494-4498` and `5327-5333` because `d42dc7726a003` is an
ancestor of HEAD. The bug the commit addresses (CRC length vs bit count)
is inherent in that checksum code, which has been here since
metadata_csum.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply on this tree** — already applied; helpers and
surrounding checks match the diff. (Older stables that still have these
checks inline in `ext4_fill_super` would need context adjustment; that
is **not** this checkout.)
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: this exact fix is in the tree. No alternate fix. `a34de48f329b2`
extends the inode `if` but does not replace the `& 7` check.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **ext4 (`fs/ext4/`) — IMPORTANT.** Default Linux filesystem;
corruption/integrity issues matter to a large user base.
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: active (recent 7.3 merges, DIO/DAX, writeback, fastcommit).
Superblock validation is mature; this fills a long-standing hole.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **filesystem-specific**, ext4 with `metadata_csum` (and bitmap
ops generally). Trigger is a superblock with `s_inodes_per_group` or
`s_clusters_per_group` not divisible by 8. Normal `mke2fs` images are
8-aligned (group size `blocksize * 8`). Population: crafted/fuzzed
images, plus theoretically ancient Android `make_ext4fs` images Ted
believes are gone.
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: mount of such an image. Not every boot of a normal distro disk.
Unprivileged trigger if the user can mount a file. Not a race.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record:
- **Without the check:** trailing bitmap bits omitted from
`metadata_csum` → integrity hole; debug free-counts ignore those bits;
incremental CRC (not in this tree) could write a wrong checksum.
- **With the check:** mount fails with `-EINVAL` and a clear error.
- **Severity: MEDIUM–HIGH** for crafted images (metadata integrity /
checksum vs tools). **Not** a panic on a normal disk. Fits the stable
“oh, that’s not good” / data-integrity class more than oops/deadlock.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** stops mounting invalid geometry; closes unprotected
bitmap bits; matches e2fsck/Ted’s request. High for anyone mounting
untrusted ext4 images; low for typical servers with `mke2fs` disks.
- **Risk:** very low on this tree — 6 lines, mount-time only, no
locking/API change. Residual risk is refusing a 10+ year-old Android
image; maintainer explicitly accepted that.
- **Ratio:** favorable.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR backporting**
- Real metadata-integrity bug: CRC covers `n/8` bytes, bitmaps have `n`
bits.
- Crafted-image mount path is userspace-reachable.
- 6-line, obviously correct addition to existing validation.
- Suggested and applied by Ted Ts'o; Reviewed-by Jan Kara and Zhang Yi;
no NAKs.
- Independent of the rest of the v2 series and of the unmerged CRC RFC.
- Code and helpers exist in this 7.3-rc4 tree; patch applies as written.
- Cover letter: “preventing crafted filesystem images from causing
bitmap checksum corruption”.
**AGAINST**
- Incremental CRC that would *write* a wrong checksum is **not** in this
kernel.
- No production oops/syzbot repro on a normal filesystem.
- Possible mount refusal of ancient `make_ext4fs` images (Ted:
landfill).
- Sashiko is static analysis, not a user crash.
**UNRESOLVED**
- Stable list archives (lore 403).
- Current e2fsprogs source not in this repo (Ted’s historical e2fsck
conflict is documented).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — `& 7` next to existing
bounds; reviewed by two ext4 developers; applied by maintainer.
2. Fixes a real bug that affects users? **PASS** — unprotected bitmap
bits and crafted-image checksum/integrity mismatch; maintainer
treated it as a real fill_super hole.
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
metadata checksum integrity / crafted-image corruption; not a panic,
but on-disk checksum and `metadata_csum` bypass for trailing bits.
4. Small and contained? **PASS** — 6 insertions / 4 deletions, one file.
5. No new features or APIs? **PASS**
6. Can apply to the local tree? **PASS** — already applies; context
matches.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: none (not a device ID, quirk, DT, build, or docs patch).
Qualified on the normal bug-fix path.
**Step 9.4: MAKE YOUR DECISION**
This belongs on this tree: the checksum code still truncates to whole
bytes, mount already validates these fields, and adding `& 7` is the
missing invariant the CRC and bitmaps assume. It is small, maintainer-
directed, and does not depend on the incremental-CRC series that never
landed.
### What problem it solves and why it matters here
`metadata_csum` hashes inode/block bitmaps in byte units. A superblock
with inodes or clusters per group not divisible by 8 leaves up to 7 bits
out of the CRC while alloc/free still uses those bits. That is an
integrity hole and a footgun for any checksum updater that assumes byte
alignment. Rejecting that geometry at mount is the same class of check
this file already does for “too big” groups.
### Risk vs benefit
Benefit is closing that hole and refusing crafted images. Risk is a
failed mount of a pathological superblock. Ted already signed off that
remaining Android `make_ext4fs` cases are historical.
---
Verification:
- [Phase 1] Parsed tags: Suggested-by Ted Ts'o; Reported-by Sashiko;
Reviewed-by Jan Kara, Zhang Yi; msgid 827117-2; no Fixes/Cc-stable
- [Phase 1] Cover letter in mbox: crafted images → bitmap checksum
corruption / overflow / OOB inode table
- [Phase 2] Diff: +6/-4 in `fs/ext4/super.c`; `s_clusters_per_group & 7`
and `s_inodes_per_group & 7`
- [Phase 2] `bitmap.c`: `>> 3` and `/ 8` confirmed; set and verify share
length
- [Phase 2] `ialloc.c:173`: memset uses `(n+7)/8` vs CRC `>> 3`
- [Phase 2] `crc32c_flip_range`: no matches — incremental CRC not in
this tree
- [Phase 3] `git describe HEAD` / `make kernelversion`: **v7.3-rc4 /
7.3.0-rc4**
- [Phase 3] `git merge-base --is-ancestor d42dc7726a003 HEAD`: yes
- [Phase 3] blame: cluster bound `40da553f5da02` (2024); inode bound via
`107d2be90116a`; CRC since `41a246d1ff75a` (2012)
- [Phase 3] series: `afa3caf2fbc41` and `a34de48f329b2` independent;
cover letter says so
- [Phase 4] `b4 dig -c`: patch.msgid.link/20260608111150.827117-2-…; v1
RFC 17-patch CRC, v2 3-patch validation
- [Phase 4] `b4 dig -a/-w/-m`: Zhang Yi + Jan Kara R-b; Ted applied;
recipients linux-ext4 + maintainers
- [Phase 4] Ted mbox: Android make_ext4fs history; add fill_super check
- [Phase 4] lore HTTP 403; used b4 instead
- [Phase 5] callers: only `__ext4_fill_super` at lines 5443 and 5451
- [Phase 6] local `super.c` already has both `& 7` checks; bitmap CRC
still truncated
- [Phase 7] ext4, important, actively merged in 7.3
- [Phase 8] failure mode: unprotected trailing bitmap bits / crafted-
image integrity; mount-time -EINVAL after fix
- [Phase 8] stable-kernel-rules.rst: oops/hang/corruption/security/“oh,
that’s not good”; 100-line cap
- UNVERIFIED: stable@ lore search (403); live e2fsprogs source not in
tree
The patch is a 6-line, maintainer-reviewed mount check that enforces an
invariant the bitmap checksum code has assumed since 2012. It is in
scope for this 7.3-rc4 tree and meets stable criteria.
**YES**
fs/ext4/super.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 9b45d27515f9f..cdb7e253015e5 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -4431,8 +4431,9 @@ static int ext4_handle_clustersize(struct super_block *sb)
sbi->s_cluster_bits = 0;
}
sbi->s_clusters_per_group = le32_to_cpu(es->s_clusters_per_group);
- if (sbi->s_clusters_per_group > sb->s_blocksize * 8) {
- ext4_msg(sb, KERN_ERR, "#clusters per group too big: %lu",
+ if (sbi->s_clusters_per_group > sb->s_blocksize * 8 ||
+ sbi->s_clusters_per_group & 7) {
+ ext4_msg(sb, KERN_ERR, "invalid #clusters per group: %lu",
sbi->s_clusters_per_group);
return -EINVAL;
}
@@ -5230,8 +5231,9 @@ static int ext4_block_group_meta_init(struct super_block *sb, int silent)
return -EINVAL;
}
if (sbi->s_inodes_per_group < sbi->s_inodes_per_block ||
- sbi->s_inodes_per_group > sb->s_blocksize * 8) {
- ext4_msg(sb, KERN_ERR, "invalid inodes per group: %lu\n",
+ sbi->s_inodes_per_group > sb->s_blocksize * 8 ||
+ sbi->s_inodes_per_group & 7) {
+ ext4_msg(sb, KERN_ERR, "invalid inodes per group: %lu",
sbi->s_inodes_per_group);
return -EINVAL;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH AUTOSEL 6.18-5.10] ext4: reduce max cluster size to match documented 256MB limit
[not found] <20261019135441.1420968-1-sashal@kernel.org>
` (4 preceding siblings ...)
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-6.12] ext4: reject mount if clusters/inodes per group are not 8-aligned Sasha Levin
@ 2026-10-19 13:53 ` Sasha Levin
2026-10-19 15:27 ` sashiko-bot
5 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-10-19 13:53 UTC (permalink / raw)
To: patches, stable
Cc: Baokun Li, Sashiko, Andreas Dilger, Theodore Ts'o,
Sasha Levin, linux-ext4, linux-kernel
From: Baokun Li <libaokun@linux.alibaba.com>
[ Upstream commit afa3caf2fbc417f755f36e271c4b3c2d45ccda9d ]
The mke2fs man page documents:
Valid cluster-size values are from 2048 to 256M bytes per cluster.
but EXT4_MAX_CLUSTER_LOG_SIZE was set to 30 (1GB), allowing crafted
filesystem images to specify cluster sizes up to 1GB.
On 32-bit systems with bigalloc enabled, the consistency check in
ext4_handle_clustersize():
s_blocks_per_group == s_clusters_per_group * (clustersize / blocksize)
can overflow when the cluster ratio is large enough. Since
s_blocks_per_group is not range-checked in the bigalloc path, the
wrapped product can pass the consistency check, leading to inconsistent
group geometry and potential out-of-bounds block allocation.
Reduce EXT4_MAX_CLUSTER_LOG_SIZE to 28 to match the documented 256MB
limit. With this cap, the maximum product is:
(blocksize * 8) * (256M / blocksize) = 2^31
which fits safely in a 32-bit unsigned long for all block sizes.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260608061112.392391-1-libaokun%40linux.alibaba.com
Signed-off-by: Baokun Li <libaokun@linux.alibaba.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Link: https://patch.msgid.link/20260608111150.827117-3-libaokun@linux.alibaba.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Local tree is **v7.3-rc4** (kernel 7.3.0-rc4, detached HEAD). The change
is a one-line cap on ext4 bigalloc cluster size to stop a 32-bit
overflow on crafted images.
===============================================================
PHASE 1: COMMIT MESSAGE FORENSICS
===============================================================
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[ext4]** **[reduce]** reduce `EXT4_MAX_CLUSTER_LOG_SIZE` from
30 (1GB) to 28 (256MB) to match the documented mke2fs limit and stop a
32-bit overflow.
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Record:
- **Reported-by:** Sashiko `<sashiko-bot@kernel.org>` (automated kernel
review, not syzbot)
- **Closes:** https://sashiko.dev/#/patchset/20260608061112.392391-1-
libaokun%40linux.alibaba.com (v1 series)
- **Reviewed-by:** Andreas Dilger `<adilger@dilger.ca>` (ext4 co-
maintainer)
- **Link:** https://patch.msgid.link/20260608111150.827117-3-
libaokun@linux.alibaba.com
- **Signed-off-by:** Baokun Li `<libaokun@linux.alibaba.com>` (author)
- **Signed-off-by:** Theodore Ts'o `<tytso@mit.edu>` (ext4 maintainer;
applied the series)
- **Fixes:** none (expected for this review pipeline)
- **Cc: stable:** none (expected)
- Ignore pipeline markers: none present in the supplied message beyond
the usual evaluation context
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** `EXT4_MAX_CLUSTER_LOG_SIZE` was 30, so crafted bigalloc
images can claim cluster sizes up to 1GB. mke2fs only documents
2048–256MB.
- **Symptom/failure mode:** On 32-bit, `s_clusters_per_group *
(clustersize / blocksize)` in `ext4_handle_clustersize()` wraps.
`s_blocks_per_group` is not range-checked on the bigalloc path, so the
wrapped product can pass the consistency check. That yields
inconsistent group geometry and possible out-of-bounds block
allocation.
- **Version info:** none beyond “32-bit systems with bigalloc”.
- **Root cause (author):** cluster-size cap is too high; product can
exceed 32-bit `unsigned long`.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. The message names overflow, crafted
images, and OOB allocation. It is an explicit mount-time sanity fix.
===============================================================
PHASE 2: DIFF ANALYSIS - LINE BY LINE
===============================================================
**Step 2.1: INVENTORY THE CHANGES**
Record:
- **File:** `fs/ext4/ext4.h` — 1 line changed (`30` → `28`)
- **Functions:** none; only `#define EXT4_MAX_CLUSTER_LOG_SIZE`
- **Scope:** single-file, one-constant surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Record:
- **Before:** `__ext4_fill_super()` rejects `s_log_cluster_size` only if
it is greater than `30 - 10 = 20`, so `clustersize = BLOCK_SIZE << log
= 1024 << 20 = 1GB` is accepted.
- **After:** max log is `28 - 10 = 18`, so max cluster size is `1024 <<
18 = 256MB`.
- **Path:** mount-time superblock validation in `__ext4_fill_super()`,
before `ext4_handle_clustersize()`.
Verified use site:
```5196:5202:fs/ext4/super.c
if (le32_to_cpu(es->s_log_cluster_size) >
(EXT4_MAX_CLUSTER_LOG_SIZE - EXT4_MIN_BLOCK_LOG_SIZE)) {
ext4_msg(sb, KERN_ERR,
"Invalid log cluster size: %u",
le32_to_cpu(es->s_log_cluster_size));
goto out;
}
```
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record:
- **Category:** integer overflow in mount-time geometry check, leading
to inconsistent geometry / possible OOB block allocation (memory-
safety / filesystem-image robustness).
- **Mechanism:** `s_blocks_per_group` and `s_clusters_per_group` are
`unsigned long` (`fs/ext4/ext4.h`). The check is:
```4500:4506:fs/ext4/super.c
if (sbi->s_blocks_per_group !=
(sbi->s_clusters_per_group * (clustersize /
sb->s_blocksize))) {
ext4_msg(sb, KERN_ERR,
"blocks per group (%lu) and clusters per group
(%lu) inconsistent",
sbi->s_blocks_per_group,
sbi->s_clusters_per_group);
return -EINVAL;
}
```
`s_clusters_per_group` is capped at `blocksize * 8`. Max product is
`(blocksize * 8) * (clustersize / blocksize) = 8 * clustersize`.
- With 1GB clusters: `8 * 2^30 = 2^33` → wraps on 32-bit `unsigned
long`.
- With 256MB clusters: `8 * 2^28 = 2^31` → fits in 32-bit `unsigned
long`.
The `s_blocks_per_group > sb->s_blocksize * 8` range check is only on
the **non-bigalloc** branch (verified in `ext4_handle_clustersize()`).
**Step 2.4: ASSESS THE FIX QUALITY**
Record:
- **Quality:** obviously correct. Tightens an existing mount-time bound
to the documented mkfs limit; the overflow math is checkable by
inspection.
- **Regression risk:** very low. mke2fs documents max 256MB; Andreas
Dilger noted real-world >1GB clusters are implausible. A 512MB (log
29) filesystem would also be rejected; Jan Kara said 28 or 29 were
both acceptable. **29 is not overflow-safe:** `8 * 512MB = 2^32` wraps
to 0 on 32-bit `unsigned long`. 28 is the right cap. Ted applied 28.
===============================================================
PHASE 3: GIT HISTORY INVESTIGATION
===============================================================
**Step 3.1: BLAME THE CHANGED LINES**
Record:
- Current line (already 28 in this checkout): `afa3caf2fbc41` Baokun Li,
2026-06-08 — the candidate itself.
- Previous value 30 introduced by **`9e47a4c9fc580`** (“ext4: sanity
check the block and cluster size at mount time”, Ted Ts'o,
2016-11-18), first in **v4.10-rc1**. Duplicate in-tree copy:
`8cdf3372fe836`.
- That original commit was **Cc: stable** and cited security refs
(SecurityFocus 539661, RH BZ 1332506). The 1GB cap has been in place
since then.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. The overflow is in the interaction of the 2016
cap (`9e47a4c9fc580`, in this tree) and the bigalloc consistency
multiply (bigalloc from `281b59959707d`, **v3.2-rc1**). Both ancestors
of HEAD.
**Step 3.3: CHECK FILE HISTORY FOR RELATED CHANGES**
Record:
- This is **v2 2/3** of “ext4: tighten mount-time superblock geometry
validation”.
- Sibling **1/3:** `d42dc7726a003` — 8-alignment of clusters/inodes per
group (bitmap csum).
- Sibling **3/3:** `a34de48f329b2` — inodes-per-group multiple of
inodes-per-block.
- Cover letter (Ted applied 2026-07-23): patches 2 and 3 were added in
v2 for additional Sashiko issues and are **independent of patch 1**.
- This commit is standalone: only the `#define` in `ext4.h`. No
prerequisite from 1/3 or 3/3.
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: Baokun Li is a regular, high-volume ext4 contributor (many
recent ext4 commits in this tree). Series was reviewed by
Dilger/Kara/Zhang Yi and applied by Ted Ts'o.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: None required. `EXT4_MAX_CLUSTER_LOG_SIZE` and the
`s_log_cluster_size` check exist here. The define is the only consumer
besides `ext4.h` itself (`git grep` shows only `fs/ext4/ext4.h` and
`fs/ext4/super.c`). Applies standalone.
===============================================================
PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
===============================================================
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- **b4 dig -c afa3caf2fbc41:** matched `[PATCH v2 2/3]` at https://patch
.msgid.link/20260608111150.827117-3-libaokun@linux.alibaba.com
- **b4 dig -a:** series is **v2** “ext4: tighten mount-time superblock
geometry validation” (2026-06-08). v1 was only patch 1; patches 2–3
are new in v2.
- **Jan Kara** (2026-06-09): no NAK; defer 28 vs 29 to Ted; both fine if
consistent.
- **Andreas Dilger** (2026-06-08): Reviewed-by; “pretty safe”; doubts
anyone uses >1GiB clusters.
- **Ted Ts'o** (2026-07-23): “Applied, thanks!” with this SHA.
- No NAKs. No reviewer `Cc: stable` in the thread (absence expected).
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record (**b4 dig -w**): linux-ext4, tytso, adilger, jack@suse.cz,
yi.zhang@huawei.com, ojaswin@linux.ibm.com, ritesh.list@gmail.com,
Sashiko. Maintainers and regular reviewers were on the To/Cc list.
Dilger R-b; Ted applied.
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: Sashiko is an LF/Google automated review bot, not a user crash
or syzbot repro. The Closes URL is the v1 patchset page (SPA; fetch
returned the Sashiko landing page, not the finding text). Cover letter:
patches 2 and 3 fix additional Sashiko-reported issues. Severity from
the patch itself: crafted-image overflow → inconsistent geometry / OOB
allocation.
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Three independent mount-time checks. This one does not need 1/3
or 3/3 to be correct. Siblings are also real bugfixes but out of scope
for this verdict.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: lore.kernel.org/stable returned 403. Openwall/web search found
no stable-list rejection of this patch. UNVERIFIED whether anyone
already requested it on stable@.
===============================================================
PHASE 5: CODE SEMANTIC ANALYSIS
===============================================================
**Step 5.1: IDENTIFY KEY FUNCTIONS IN THE DIFF**
Record: No function body changed. Consumer: `__ext4_fill_super()`
cluster-log check; downstream user of the accepted size:
`ext4_handle_clustersize()`.
**Step 5.2: TRACE CALLERS**
Record:
- `ext4_handle_clustersize()` is static; only caller is
`__ext4_fill_super()` at mount (`fs/ext4/super.c:5451`).
- `ext4_fill_super()` is the VFS mount entry (`:5835`).
- Context: filesystem mount, not IRQ/hot path.
**Step 5.3: TRACE CALLEES**
Record: The define is only compared in the mount validator. No new
calls.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace mount of an ext4 image (`mount`, loop, USB, VM disk) →
`ext4_fill_super` → `__ext4_fill_super` → log-cluster check →
`ext4_handle_clustersize()`. Reachable from any ext4 mount of a bigalloc
image. Triggering the overflow needs a crafted superblock (untrusted
image), typically CAP_SYS_ADMIN to mount, still the usual malicious-
filesystem class.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Same series adds two other geometry checks. Same class of
crafted-image bugs. No other `EXT4_MAX_CLUSTER_LOG_SIZE` sites.
===============================================================
PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
===============================================================
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record:
- Tree: **v7.3-rc4-27-gf0100363d8c37**, `VERSION=7 PATCHLEVEL=3
SUBLEVEL=0 EXTRAVERSION=-rc4`.
- `git merge-base --is-ancestor afa3caf2fbc41 HEAD` → commit **is** in
HEAD (landed via ext4-for-linus 7.3-rc1). Current `#define` is already
28.
- That does **not** mean “already selected for stable”; it is the
mainline landing. The **bug mechanism** (32-bit multiply of
`s_clusters_per_group` by cluster ratio, uncapped `s_blocks_per_group`
on bigalloc) is still in `ext4_handle_clustersize()` here; only the
cap is 28.
- Bug class has existed since the 2016 cap (`9e47a4c9fc580` is an
ancestor). Code is not missing.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: Expected apply: **clean**. One-line `#define` change in
`fs/ext4/ext4.h`. Macro and check have been stable since 2016 / the 2022
`ext4_handle_clustersize()` factor-out (`4a8557b094671`).
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: This is the fix. No earlier alternate cap. No follow-up revert
or tightening of this define after `afa3caf2fbc41` in `fs/ext4/ext4.h`.
===============================================================
PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
===============================================================
**Step 7.1: IDENTIFY THE SUBSYSTEM AND ITS CRITICALITY**
Record: **ext4** (`fs/ext4/`). **IMPORTANT** — default Linux filesystem;
crafted-image bugs affect anyone who mounts untrusted ext4 (install
media, USB, VMs, backups).
**Step 7.2: ASSESS SUBSYSTEM ACTIVITY**
Record: Actively developed (ext4_for_linus pulls this cycle). The
overflow is in long-standing mount validation, not new BS>PS work.
===============================================================
PHASE 8: IMPACT AND RISK ASSESSMENT
===============================================================
**Step 8.1: DETERMINE WHO IS AFFECTED**
Record: **Filesystem-specific (ext4)**, extra conditions: **bigalloc +
32-bit `unsigned long` + cluster size > 256MB**. 64-bit does not wrap at
1GB, but the looser cap still allows undocumented cluster sizes. 32-bit
still matters (embedded, some Android/i686).
**Step 8.2: DETERMINE THE TRIGGER CONDITIONS**
Record: Mount of a crafted bigalloc image with large
`s_log_cluster_size`. Not every boot. Not an unprivileged syscall by
itself (mount is privileged), but standard untrusted-image attack. Not a
timing race.
**Step 8.3: DETERMINE THE FAILURE MODE SEVERITY**
Record: Integer overflow bypass of geometry check → inconsistent groups
→ **potential out-of-bounds block allocation**. Severity **HIGH**
(memory corruption / OOB on crafted mount). Not a guaranteed panic on
every mount, but it is a real robustness/security class that stable
routinely takes.
**Step 8.4: CALCULATE RISK-BENEFIT RATIO**
Record:
- **Benefit:** high for 32-bit ext4 mounts of untrusted images; aligns
kernel with documented mkfs limit.
- **Risk:** very low (one constant; Dilger R-b; Ted applied). Only
theoretical regression is rejecting 512MB/1GB cluster filesystems that
mke2fs would not create.
- **Ratio:** strongly favors backport.
===============================================================
PHASE 9: FINAL SYNTHESIS
===============================================================
**Step 9.1: COMPILE THE EVIDENCE**
FOR:
- Real 32-bit overflow in mount-time geometry check; wrapped product can
pass; OOB allocation is the stated consequence.
- One-line, obviously correct cap matching mke2fs (256MB).
- 28 is the max value that keeps `8 * clustersize` ≤ 2^31 on 32-bit.
- Reviewed by Andreas Dilger; applied by Ted Ts'o; Jan Kara did not
object.
- Independent of the rest of the series.
- Affected code present since v4.10; applies cleanly.
- Same class as the original 2016 cluster-size sanity check (that one
was Cc: stable).
AGAINST:
- Trigger needs crafted bigalloc + typically 32-bit.
- No `Fixes:` / `Cc: stable` (not a negative per rules).
- Sashiko is a bot, not a production crash with a reproducer.
- Jan Kara floated 29; that value still overflows (verified); committed
value is 28.
UNRESOLVED:
- Stable@ discussion (lore/stable 403).
- Full Sashiko finding text (SPA page).
**Step 9.2: APPLY THE STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — one-constant change;
maintainer-applied; Dilger R-b.
2. Fixes a real bug that affects users? **PASS** — overflow on crafted
bigalloc mount.
3. Important issue (crash, security, corruption, deadlock)? **PASS** —
HIGH: overflow → bad geometry / possible OOB allocation.
4. Small and contained? **PASS** — 1 line, 1 file.
5. No new features or APIs? **PASS** — tightens an existing limit.
6. Can apply to the local tree? **PASS** — define and check exist; patch
is a single substitution.
**Step 9.3: CHECK FOR EXCEPTION CATEGORIES**
Record: Not an ID/quirk/DT/build/docs exception. It is a normal small
correctness/security-style mount check.
**Step 9.4: MAKE YOUR DECISION**
This tree has the ext4 mount path, the overflow-prone multiply, and the
too-large historical cap. The fix is a one-line tightening to the
documented 256MB limit so the 32-bit product cannot wrap. Independent,
reviewed, and low regression risk. It should be backported here.
------------------------------------------------
What the commit solves and why it matters
------------------------------------------------
Crafted ext4 bigalloc images can set cluster size up to 1GB. On 32-bit,
the groups-per-cluster consistency multiply wraps, so a bogus
`s_blocks_per_group` can look consistent. That can produce wrong group
geometry and out-of-bounds block allocation when the image is mounted.
Capping at 256MB matches mke2fs and keeps the product at 2^31.
------------------------------------------------
Stable rules / risk vs benefit
------------------------------------------------
Meets all six criteria: real overflow, small, no API, applies cleanly.
Benefit is blocking a crafted-image OOB path; risk is rejecting
undocumented huge clusters that tools do not create.
------------------------------------------------
Concerns
------------------------------------------------
No extra series deps. No backport rework expected. 32-bit-centric, but
still a valid stable-class filesystem-image bug.
Verification:
- [Phase 1] Parsed tags: Reported-by Sashiko; Reviewed-by Andreas
Dilger; SOB Baokun Li + Ted Ts'o; Link msgid 20260608111150.827117-3;
no Fixes/Cc:stable
- [Phase 1] Cover letter (openwall/spinics): v2 2/3 independent of 1/3;
overflow on 32-bit bigalloc
- [Phase 2] Diff: 1 line in fs/ext4/ext4.h, 30→28
- [Phase 2] Read ext4_handle_clustersize(): s_blocks_per_group range
check only on non-bigalloc; multiply at lines 4500–4501
- [Phase 2] Types: s_blocks_per_group / s_clusters_per_group are
unsigned long (ext4.h:1595–1596)
- [Phase 2] Math: max product 8*clustersize; 1GB → 2^33 wrap; 256MB →
2^31; 512MB → 2^32 wrap (29 unsafe)
- [Phase 2] Consumer: __ext4_fill_super log-cluster check at
super.c:5196–5202
- [Phase 3] git blame: current 28 from afa3caf2fbc41; 30 from
9e47a4c9fc580 (2016-11-18, v4.10-rc1, Cc:stable)
- [Phase 3] git merge-base --is-ancestor 9e47a4c9fc580 HEAD: yes
- [Phase 3] git merge-base --is-ancestor afa3caf2fbc41 HEAD: yes
(mainline landing, not used as circular “already selected”)
- [Phase 3] Series siblings d42dc7726a003 and a34de48f329b2 are
independent; this patch only touches the define
- [Phase 3] git grep EXT4_MAX_CLUSTER_LOG_SIZE: only ext4.h and super.c
- [Phase 4] b4 dig -c: lore msgid 20260608111150.827117-3
- [Phase 4] b4 dig -a: v2 series 2026-06-08; v1 was patch 1 only
- [Phase 4] b4 dig -w: tytso, dilger, jack, linux-ext4 on original To/Cc
- [Phase 4] Jan Kara: 28 vs 29 up to Ted, no NAK
- [Phase 4] Andreas Dilger: Reviewed-by; safe, nobody using >1GiB
clusters
- [Phase 4] Ted: Applied 2026-07-23 as afa3caf2fbc41
- [Phase 4] lore.kernel.org/stable: 403, UNVERIFIED
- [Phase 4] sashiko.dev hash URL: landing page only, UNVERIFIED full
finding text
- [Phase 5] Only caller of ext4_handle_clustersize: __ext4_fill_super
(mount)
- [Phase 6] git describe HEAD: v7.3-rc4-27-gf0100363d8c37; make
kernelversion 7.3.0-rc4
- [Phase 6] Buggy cap/check present in this lineage since v4.10; apply
is a one-line define swap
- [Phase 8] Failure mode: 32-bit overflow → inconsistent geometry /
possible OOB alloc, severity HIGH
- UNVERIFIED: stable@ thread; full Sashiko report body
**YES**
fs/ext4/ext4.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index da6428312c36b..a9178d63a7d1b 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -334,7 +334,7 @@ struct ext4_io_submit {
#define EXT4_MAX_BLOCK_SIZE 65536
#define EXT4_MIN_BLOCK_LOG_SIZE 10
#define EXT4_MAX_BLOCK_LOG_SIZE 16
-#define EXT4_MAX_CLUSTER_LOG_SIZE 30
+#define EXT4_MAX_CLUSTER_LOG_SIZE 28
#ifdef __KERNEL__
# define EXT4_BLOCK_SIZE(s) ((s)->s_blocksize)
#else
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread