From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Filipe Manana <fdmanana@suse.com>, Qu Wenruo <wqu@suse.com>,
David Sterba <dsterba@suse.com>, Sasha Levin <sashal@kernel.org>,
clm@fb.com, linux-btrfs@vger.kernel.org
Subject: [PATCH AUTOSEL 6.18-6.6] btrfs: abort transaction on item count overflow in __push_leaf_left()
Date: Mon, 8 Dec 2025 19:14:58 -0500 [thread overview]
Message-ID: <20251209001610.611575-6-sashal@kernel.org> (raw)
In-Reply-To: <20251209001610.611575-1-sashal@kernel.org>
From: Filipe Manana <fdmanana@suse.com>
[ Upstream commit 5d8222a50ad37c98455da08b33ce49fe6b726c72 ]
If we try to push an item count from the right leaf that is greater than
the number of items in the leaf, we just emit a warning. This should
never happen but if it does we get an underflow in the new number of
items in the right leaf and chaos follows from it. So replace the warning
with proper error handling, by aborting the transaction and returning
-EUCLEAN, and proper logging by using btrfs_crit() instead of WARN(),
which gives us proper formatting and information about the filesystem.
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Commit Analysis: btrfs: abort transaction on item count overflow in
__push_leaf_left()
### 1. COMMIT MESSAGE ANALYSIS
**Key points:**
- Fixes an item count overflow condition where `push_items >
right_nritems`
- Currently only emits a WARN() but continues execution
- If triggered, causes "an underflow in the new number of items in the
right leaf and chaos follows"
- Replaces warning with proper error handling (abort transaction, return
-EUCLEAN)
**Tags:**
- No `Cc: stable@vger.kernel.org` tag
- No `Fixes:` tag
- Has two `Reviewed-by:` tags (Qu Wenruo and David Sterba - btrfs
maintainer)
### 2. CODE CHANGE ANALYSIS
**Before (problematic):**
```c
if (push_items > right_nritems)
WARN(1, KERN_CRIT "push items %d nr %u\n", push_items,
right_nritems);
// Continues execution despite the error!
```
**After (fixed):**
```c
if (unlikely(push_items > right_nritems)) {
ret = -EUCLEAN;
btrfs_abort_transaction(trans, ret);
btrfs_crit(fs_info, "push items (%d) > right leaf items (%u)",
push_items, right_nritems);
goto out;
}
```
**Technical mechanism of the bug:**
- `__push_leaf_left()` pushes items from right leaf to left leaf in
btrfs B-tree
- If `push_items > right_nritems`, later code does `right_nritems -=
push_items`
- Since `right_nritems` is `u32`, this causes an **integer underflow**
- The underflowed value is then set via `btrfs_set_header_nritems(right,
right_nritems)`
- This corrupts the B-tree structure, leading to filesystem corruption
### 3. CLASSIFICATION
- **Type:** Bug fix - upgrading inadequate error handling to proper
abort
- **Not adding features:** Purely defensive error handling improvement
- **Severity:** Prevents potential filesystem/data corruption
- **Pattern:** Converts warn-and-continue to abort-and-return on
corruption detection
### 4. SCOPE AND RISK ASSESSMENT
| Factor | Assessment |
|--------|------------|
| Lines changed | ~10 lines |
| Files touched | 1 (fs/btrfs/ctree.c) |
| Complexity | Very low |
| Subsystem maturity | btrfs is mature and widely used |
| Regression risk | **Near zero** - if condition never triggers,
behavior unchanged; if it does, abort is safer than corruption |
The change uses existing infrastructure (`btrfs_abort_transaction`,
`goto out` path) that's already present and tested.
### 5. USER IMPACT
- **Affected users:** All btrfs users
- **Code location:** Core B-tree code - fundamental to all btrfs
operations
- **If bug triggers:** Filesystem corruption, potentially unmountable
filesystem
- **Frequency:** Described as "should never happen" but defensive check
was already there
### 6. STABILITY INDICATORS
- Two reviewers from btrfs team
- Signed off by btrfs maintainer (David Sterba)
- Conservative, defensive fix with well-understood behavior
### 7. DEPENDENCY CHECK
- Self-contained fix with no dependencies
- Uses existing error handling patterns present in all stable trees
- This code has existed in btrfs for many years
### DECISION RATIONALE
**For backporting:**
1. **Prevents data corruption** - the most critical category of bugs for
filesystems
2. **Minimal risk** - the fix cannot cause regression in normal
operation
3. **Small and surgical** - ~10 lines, single file, obvious correctness
4. **Well-reviewed** - two experienced btrfs developers signed off
5. **Core subsystem** - btrfs is used in production by enterprise users
relying on stable trees
6. **Integer underflow** - these are exactly the type of bugs stable
trees want fixed
**Against backporting:**
1. No explicit `Cc: stable` tag
2. No `Fixes:` tag
3. Condition described as "should never happen"
**Assessment:** While the maintainer didn't explicitly request stable
backport, the fix meets all stable criteria:
- Obviously correct (straightforward error handling)
- Fixes a real bug (underflow leading to corruption)
- Small and contained (10 lines, 1 file)
- No new features
The severity of the potential consequence (filesystem corruption)
combined with the trivial risk of the fix makes this an appropriate
stable candidate. Filesystems are exactly where defensive hardening
matters most for stable users.
**YES**
fs/btrfs/ctree.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 561658aca018b..3acb3027584d7 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -3398,9 +3398,13 @@ static noinline int __push_leaf_left(struct btrfs_trans_handle *trans,
btrfs_set_header_nritems(left, old_left_nritems + push_items);
/* fixup right node */
- if (push_items > right_nritems)
- WARN(1, KERN_CRIT "push items %d nr %u\n", push_items,
- right_nritems);
+ if (unlikely(push_items > right_nritems)) {
+ ret = -EUCLEAN;
+ btrfs_abort_transaction(trans, ret);
+ btrfs_crit(fs_info, "push items (%d) > right leaf items (%u)",
+ push_items, right_nritems);
+ goto out;
+ }
if (push_items < right_nritems) {
push_space = btrfs_item_offset(right, push_items - 1) -
--
2.51.0
next parent reply other threads:[~2025-12-09 0:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20251209001610.611575-1-sashal@kernel.org>
2025-12-09 0:14 ` Sasha Levin [this message]
2025-12-09 0:15 ` [PATCH AUTOSEL 6.18-5.10] btrfs: scrub: always update btrfs_scrub_progress::last_physical Sasha Levin
2025-12-09 0:15 ` [PATCH AUTOSEL 6.18-6.17] btrfs: use kvcalloc for btrfs_bio::csum allocation 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=20251209001610.611575-6-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=fdmanana@suse.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.org \
--cc=wqu@suse.com \
/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