From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7D53E3E2766; Mon, 20 Apr 2026 13:24:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776691476; cv=none; b=WuR5725UJGO9ZuL1fn0OwVpX/hD2wh42AyW+1dddwiSS4KB8layv7jZ4IDK5sY1vRP1z+Ae02yvZCiqoFOeVRQOk7F0CmWS6deLn2uQj7v+S5TMFuSuz+J8po70P9Zf8ZMYaNKTr1Uo/uWRNZ/2E4e5EcNCU7kjgs36pFfantT4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776691476; c=relaxed/simple; bh=kHtxgp4FvRLcfUMJwmWJAE/lpLA6/LPhfFr3uy8U9vw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Gokf+dflHKNCdjlYDlZKloILSnV4yNQ2Uu7xKnIvNW7yCjwTo0+P1lQ/swq+GY9IKEn8QVWN+JvQyQn4eFbNAiM2XHCiiNvKnfYvHatvN/tZypoqwvDb28L3enLF+6yy2ndp2hoVIOv0VkCqfUMQPyRTptN43Odo04MMBF81hFM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=mWqBE/aw; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="mWqBE/aw" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 221E7C2BCB6; Mon, 20 Apr 2026 13:24:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1776691476; bh=kHtxgp4FvRLcfUMJwmWJAE/lpLA6/LPhfFr3uy8U9vw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mWqBE/awQq5+O6ERazlV8E40EZ0MhBCqm/mkoASmaQ4fR/SOwJkWmsgQ325wozvtB RUSfaCMtpmKL6nmQhfjPBhwWBGvBauqjJydu7YabGACui2yZu9CPiVJ2iLjdg1cWw0 QXTHOILYkbsTfcyp4KkCJGAv4UtnruY7nmBj2TUcFJXE19M5FyVNdkUVEIPsH7OKY7 6De+5SG6UEawgDpaTyAAXHng3HHr6LMGV9QPBDwnPMo3u4kTHrJ0IsSDIIiAW84ZwZ QNj81bgCSwAav1t53DBXuw9rYc8KUOH09Heq0eXuPceCYqs/xFu170WSwiqKZBSFRn GG5A6vGYNgDUA== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Ziyi Guo , Jan Kara , Sasha Levin , 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 Message-ID: <20260420132314.1023554-52-sashal@kernel.org> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org> References: <20260420132314.1023554-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-ext4@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 7.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From: Ziyi Guo [ 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 Link: https://patch.msgid.link/20260211022052.973114-1-n7l8m4@u.northwestern.edu Signed-off-by: Jan Kara Signed-off-by: Sasha Levin --- 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