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) -