public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Ziyi Guo <n7l8m4@u.northwestern.edu>, Jan Kara <jack@suse.cz>,
	Sasha Levin <sashal@kernel.org>,
	jack@suse.com, linux-ext4@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 7.0-6.6] ext2: avoid drop_nlink() during unlink of zero-nlink inode in ext2_unlink()
Date: Mon, 20 Apr 2026 09:17:26 -0400	[thread overview]
Message-ID: <20260420132314.1023554-52-sashal@kernel.org> (raw)
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>

From: Ziyi Guo <n7l8m4@u.northwestern.edu>

[ Upstream commit 19134a133184fcc49c41cf42797cb2e7fef76065 ]

ext2_unlink() calls inode_dec_link_count() unconditionally, which
invokes drop_nlink(). If the inode was loaded from a corrupted disk
image with i_links_count == 0, drop_nlink()
triggers WARN_ON(inode->i_nlink == 0)

Follow the ext4 pattern from __ext4_unlink(): check i_nlink before
decrementing. If already zero, skip the decrement.

Signed-off-by: Ziyi Guo <n7l8m4@u.northwestern.edu>
Link: https://patch.msgid.link/20260211022052.973114-1-n7l8m4@u.northwestern.edu
Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed for a complete analysis. Let me
compile the full assessment.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem:** ext2 (filesystem)
- **Action verb:** "avoid" - indicates preventing an incorrect behavior
- **Summary:** Prevent `drop_nlink()` WARN_ON when unlinking an inode
  that already has zero link count

### Step 1.2: Tags
- **Link:** `https://patch.msgid.link/20260211022052.973114-1-
  n7l8m4@u.northwestern.edu` - original patch submission
- **Signed-off-by:** Ziyi Guo (author), Jan Kara (ext2/ext4 maintainer)
- No Fixes: tag (expected for this review pipeline)
- No Reported-by: tag, but the commit describes a specific WARN_ON
  trigger from corrupted disk images
- No Cc: stable (expected)

### Step 1.3: Commit Body
- **Bug:** `ext2_unlink()` unconditionally calls
  `inode_dec_link_count()`, which calls `drop_nlink()`. If the inode was
  loaded from a corrupted disk with `i_links_count == 0`, `drop_nlink()`
  triggers `WARN_ON(inode->i_nlink == 0)`.
- **Failure mode:** Kernel WARN_ON triggered, and then `i_nlink`
  underflows to `(unsigned int)-1`.
- **Fix approach:** Follow the ext4 pattern from `__ext4_unlink()`:
  check `i_nlink` before decrementing.

### Step 1.4: Hidden Bug Fix?
This is an explicit bug fix, not disguised. It directly addresses a
WARN_ON trigger and an nlink underflow from corrupted disk images.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
- **File:** `fs/ext2/namei.c` — 1 line removed, 3 lines added (net +2
  lines)
- **Function modified:** `ext2_unlink()`
- **Scope:** Single-file, single-function, surgical fix

### Step 2.2: Code Flow Change
**Before:** `inode_dec_link_count(inode)` is called unconditionally
after a successful directory entry deletion.

**After:** `inode_dec_link_count(inode)` is only called if
`inode->i_nlink > 0`.

### Step 2.3: Bug Mechanism
Category: **Logic/correctness fix + defensive coding against
corruption**

The call chain is:
1. `ext2_unlink()` → `inode_dec_link_count()` (fs.h inline)
2. `inode_dec_link_count()` → `drop_nlink()` (fs/inode.c)
3. `drop_nlink()` has `WARN_ON(inode->i_nlink == 0)` followed by
   `inode->__i_nlink--`

Verified from `fs/inode.c` lines 416-422:
```c
void drop_nlink(struct inode *inode)
{
    WARN_ON(inode->i_nlink == 0);
    inode->__i_nlink--;
    ...
}
```

With a corrupted disk where `i_links_count == 0`, this triggers the WARN
and underflows the nlink counter.

### Step 2.4: Fix Quality
- **Obviously correct:** Yes — if nlink is already 0, don't decrement
  further.
- **Minimal/surgical:** Yes — 2 lines of logic added.
- **Regression risk:** Extremely low — only affects corrupted inodes
  with zero nlink. Normal inodes always have nlink >= 1 during unlink.
- **Precedent:** The ext4 filesystem has had the identical check since
  2019 (commit c7df4a1ecb857, by Theodore Ts'o, Cc: stable@kernel.org).

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
The unconditional `inode_dec_link_count(inode)` at the unlink path
traces to `a513b035eadf80` (2006, Alexey Dobriyan - introduced the
`inode_dec_link_count` wrapper) but the underlying unlink logic is from
`1da177e4c3f41` (2005, Linus Torvalds, Linux 2.6.12-rc2). **This buggy
code has been present since the very first kernel in git.**

### Step 3.2: Fixes Tag
No Fixes: tag present. This is expected for the review pipeline. The bug
has existed since the origin of the codebase.

### Step 3.3: File History
Recent changes to `fs/ext2/namei.c` are all refactoring (folio
conversion, ctime accessors, idmap). None are related to nlink handling.
The fix is standalone with no prerequisites.

### Step 3.4: Author
Ziyi Guo is not a regular ext2 contributor. However, the commit was
signed by **Jan Kara** (`jack@suse.cz`), who is the ext2/ext4
maintainer. This gives the fix high credibility.

### Step 3.5: Dependencies
The fix has **zero dependencies**. It only adds an `if` guard around an
existing function call. No new functions, no new data structures.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1-4.2: Patch Discussion
Lore was not accessible (Anubis protection). b4 dig could not match
because the HEAD SHA was incorrectly used. However, the Link: tag
confirms the patch was submitted and applied through Jan Kara's tree.

### Step 4.3: Bug Context
- The ext4 equivalent fix (c7df4a1ecb857) references bugzilla.kernel.org
  bug 205433 — a real user-reported bug from corrupted disk images.
- The minix equivalent fixes (`009a2ba40303c`, `d3e0e8661ceb4`) were
  **syzbot-reported**, confirming this class of bug is found by fuzzers
  against ext2-like filesystems.

### Step 4.4: Related Patches
Multiple filesystems have received the exact same fix:
- **ext4:** c7df4a1ecb857 (2019, Cc: stable, by Theodore Ts'o)
- **minix rename:** 009a2ba40303c (syzbot-reported, Reviewed-by: Jan
  Kara)
- **minix rmdir:** d3e0e8661ceb4 (syzbot-reported, Reviewed-by: Jan
  Kara)
- **fat:** 8cafcb881364a (parent link count underflow in rmdir)
- **f2fs:** 42cb74a92adaf (prevent kernel warning from corrupted image)

This is a well-understood class of bug. Ext2 was the last remaining
major filesystem without the guard.

### Step 4.5: Stable History
The ext4 equivalent was explicitly tagged `Cc: stable@kernel.org` by
Theodore Ts'o, establishing a precedent that this class of fix belongs
in stable.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1-5.2: Functions
- **Modified function:** `ext2_unlink()` — called from:
  - VFS unlink path (`.unlink = ext2_unlink` in
    `ext2_dir_inode_operations`)
  - `ext2_rmdir()` (line 308)
  - `ext2_rename()` is not a direct caller
- VFS unlink is triggered by the `unlink()` syscall — this is a
  **common, userspace-reachable path**.

### Step 5.3-5.4: Call Chain
`unlink()` syscall → `do_unlinkat()` → `vfs_unlink()` → `ext2_unlink()`
→ `inode_dec_link_count()` → `drop_nlink()` → **WARN_ON**

The buggy path is directly reachable from userspace with any corrupted
ext2 filesystem image.

### Step 5.5: Similar Patterns
`ext2_rmdir()` also has an unprotected `inode_dec_link_count(inode)` at
line 311 (after calling `ext2_unlink`). This is a separate path that
could benefit from a similar guard, but the current fix addresses the
most direct and common case.

---

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Code Exists in Stable
Verified: the exact same code exists in the 6.19.12 stable tree —
`inode_dec_link_count(inode)` at the same location in `ext2_unlink()`.
The buggy code has been present since Linux 2.6.12 and is in **every
active stable tree**.

### Step 6.2: Backport Complications
The code in the 7.0 tree and the 6.19.12 stable tree is **identical**
around `ext2_unlink()`. The patch will apply cleanly. Older stable trees
(pre-6.6) that use page-based rather than folio-based code will have the
same surrounding logic — the fix only touches the `inode_dec_link_count`
line, which hasn't changed.

### Step 6.3: Related Fixes in Stable
No equivalent ext2 fix is already in stable. The ext4 fix
(c7df4a1ecb857) went to stable in 2019.

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem Criticality
- **Subsystem:** ext2 filesystem (fs/ext2/)
- **Criticality:** IMPORTANT — ext2 is widely used in embedded systems,
  USB drives, small partitions, and as a simple filesystem for testing.
  Any machine that mounts an ext2 filesystem is affected.

### Step 7.2: Activity
ext2 is a mature, stable subsystem with infrequent changes. Bug has been
present for 20+ years, making the fix more important for stable (more
deployed systems affected).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Affected Population
All users who mount ext2 filesystems — this includes:
- Embedded systems, USB drives, legacy partitions
- Any system handling potentially corrupted or malicious ext2 images

### Step 8.2: Trigger Conditions
- **Trigger:** Mount a corrupted ext2 filesystem with an inode that has
  `i_links_count == 0`, then unlink that file.
- **Likelihood:** Uncommon in normal usage, but straightforward with
  corrupted/malicious disk images.
- **Unprivileged user:** Yes — can be triggered by any user with write
  access to the mounted filesystem (or via auto-mounted USB devices).
- **Security relevance:** Mounting crafted filesystem images is a known
  attack vector.

### Step 8.3: Failure Mode Severity
- **WARN_ON trigger:** Produces a kernel warning with full stack trace
  (log spam, potential for denial-of-service if warnings cause system
  slowdown or panic-on-warn)
- **nlink underflow:** `i_nlink` wraps to `(unsigned int)-1`, which
  corrupts the inode state in memory
- **Severity:** MEDIUM-HIGH (WARN_ON + data corruption in inode state)
- On systems with `panic_on_warn`, this becomes a **kernel panic**
  (CRITICAL).

### Step 8.4: Risk-Benefit Ratio
- **Benefit:** Prevents WARN_ON, nlink underflow, and potential panic on
  corrupted ext2 images. Established pattern across 5+ filesystems.
- **Risk:** Near-zero. The fix is a 2-line `if` guard that only
  activates on corrupted inodes. Normal operations are completely
  unaffected.
- **Ratio:** Very favorable for backporting.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence Summary

**FOR backporting:**
- Fixes a real bug: WARN_ON + nlink underflow on corrupted disk images
- Extremely small and surgical: 2 lines of code
- Obviously correct: simple `if (inode->i_nlink)` guard
- Follows established pattern from ext4 (which was CC'd stable by Ted
  Ts'o)
- Same class of fix applied to minix (syzbot-reported), fat, f2fs
- Signed by ext2 maintainer Jan Kara
- Buggy code exists in ALL stable trees (since 2005)
- Patch applies cleanly to stable trees
- No dependencies on other commits
- Zero regression risk for normal operations
- On `panic_on_warn` systems, this prevents a kernel panic

**AGAINST backporting:**
- No explicit Fixes: tag (expected, not a negative signal)
- Only the unlink path is fixed; ext2_rmdir has a similar unprotected
  path (but this fix is still standalone and valuable)
- Trigger requires corrupted disk image (not common in normal usage)

### Step 9.2: Stable Rules Checklist
1. **Obviously correct and tested?** YES — trivial guard, same pattern
   as ext4 (in stable since 2019)
2. **Fixes a real bug?** YES — WARN_ON and nlink underflow from
   corrupted images
3. **Important issue?** YES — kernel warning, potential data corruption,
   panic-on-warn
4. **Small and contained?** YES — 2 lines, single function, single file
5. **No new features/APIs?** YES — purely defensive check
6. **Applies to stable?** YES — verified identical code in 6.19.12

### Step 9.3: Exception Categories
Not needed — this meets standard stable criteria as a bug fix.

### Step 9.4: Decision
Clear YES. This is a textbook stable backport: a tiny, obviously correct
fix that prevents a kernel warning and nlink corruption on mounted
corrupted ext2 filesystems, following an established pattern across
multiple filesystems, signed by the subsystem maintainer.

---

## Verification

- [Phase 1] Parsed tags: Link to patch.msgid.link, Signed-off-by Jan
  Kara (ext2 maintainer)
- [Phase 2] Diff analysis: 2 lines added — `if (inode->i_nlink)` guard
  around `inode_dec_link_count(inode)` in `ext2_unlink()`
- [Phase 2] Confirmed `drop_nlink()` in `fs/inode.c:416-422` has
  `WARN_ON(inode->i_nlink == 0)` followed by `inode->__i_nlink--`
- [Phase 2] Confirmed `inode_dec_link_count()` in
  `include/linux/fs.h:2251-2255` calls `drop_nlink()` then
  `mark_inode_dirty()`
- [Phase 3] git blame: buggy `inode_dec_link_count` introduced in
  a513b035eadf80 (2006), underlying logic from 1da177e4c3f41 (2005,
  original kernel)
- [Phase 3] No prerequisites found; fix is standalone
- [Phase 3] git log: no related ext2 unlink fixes in recent history
- [Phase 4] b4 dig: could not match due to commit not being in this
  tree; lore.kernel.org blocked by Anubis
- [Phase 4] ext4 equivalent fix c7df4a1ecb857 (Theodore Ts'o, 2019)
  verified — has `Cc: stable@kernel.org` and `Reviewed-by: Andreas
  Dilger`
- [Phase 4] minix equivalents 009a2ba40303c and d3e0e8661ceb4 verified —
  syzbot-reported, Reviewed-by Jan Kara
- [Phase 5] Callers: ext2_unlink is VFS `.unlink` handler, reachable
  from `unlink()` syscall — common path
- [Phase 5] Also called from ext2_rmdir (which has its own unprotected
  inode_dec_link_count)
- [Phase 6] Verified: 6.19.12 stable tree has identical unfixed code at
  same location in ext2_unlink()
- [Phase 6] Patch applies cleanly — surrounding context is identical
- [Phase 7] ext2 is a mature, widely-deployed filesystem — IMPORTANT
  criticality
- [Phase 8] Failure mode: WARN_ON + nlink underflow; CRITICAL on
  panic_on_warn systems
- [Phase 8] Risk: near-zero (2-line if guard, only activates on
  corrupted inodes)

**YES**

 fs/ext2/namei.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
index bde617a66cecd..728c487308baf 100644
--- a/fs/ext2/namei.c
+++ b/fs/ext2/namei.c
@@ -293,7 +293,10 @@ static int ext2_unlink(struct inode *dir, struct dentry *dentry)
 		goto out;
 
 	inode_set_ctime_to_ts(inode, inode_get_ctime(dir));
-	inode_dec_link_count(inode);
+
+	if (inode->i_nlink)
+		inode_dec_link_count(inode);
+
 	err = 0;
 out:
 	return err;
-- 
2.53.0


       reply	other threads:[~2026-04-20 13:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260420132314.1023554-1-sashal@kernel.org>
2026-04-20 13:17 ` Sasha Levin [this message]
2026-04-20 13:18 ` [PATCH AUTOSEL 7.0-5.10] ext2: replace BUG_ON with WARN_ON_ONCE in ext2_get_blocks Sasha Levin
2026-04-20 13:22 ` [PATCH AUTOSEL 7.0-6.1] ext4: unmap invalidated folios from page tables in mpage_release_unused_pages() Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260420132314.1023554-52-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=jack@suse.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=n7l8m4@u.northwestern.edu \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox