[PATCH AUTOSEL 6.12 1/7] btrfs: exit after state insertion failure at btrfs_convert_extent_bit()

[PATCH AUTOSEL 6.12 1/7] btrfs: exit after state insertion failure at btrfs_convert_extent_bit()

[Sasha Levin]

From: Filipe Manana <fdmanana@suse.com>

[ Upstream commit 3bf179e36da917c5d9bec71c714573ed1649b7c1 ]

If insert_state() state failed it returns an error pointer and we call
extent_io_tree_panic() which will trigger a BUG() call. However if
CONFIG_BUG is disabled, which is an uncommon and exotic scenario, then
we fallthrough and call cache_state() which will dereference the error
pointer, resulting in an invalid memory access.

So jump to the 'out' label after calling extent_io_tree_panic(), it also
makes the code more clear besides dealing with the exotic scenario where
CONFIG_BUG is disabled.

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

**YES** This commit should be backported to stable kernel trees.
**Detailed Analysis:** **1. Bug Description and Impact:** The commit
fixes a serious potential memory corruption bug in the btrfs filesystem.
When `insert_state()` fails in `btrfs_convert_extent_bit()`, it returns
an error pointer (ERR_PTR). The current code calls
`extent_io_tree_panic()` which triggers `BUG()`, but if `CONFIG_BUG` is
disabled, the code continues executing and calls
`cache_state(inserted_state, cached_state)` with the error pointer,
causing invalid memory access. **2. Code Changes Analysis:** The fix is
minimal and surgical - just adding a single `goto out;` statement after
the `extent_io_tree_panic()` call: ```c if (IS_ERR(inserted_state)) {
ret = PTR_ERR(inserted_state); extent_io_tree_panic(tree, prealloc,
"insert", ret); + goto out; // <-- The fix } ``` This ensures that when
`CONFIG_BUG` is disabled, execution jumps to the cleanup code instead of
continuing with an invalid pointer. **3. Comparison with Similar
Commits:** This commit aligns with the pattern seen in "Similar Commit
#2" (Status: YES), which also: - Removes reliance on `BUG_ON()` behavior
- Provides graceful error handling - Has minimal risk - Fixes a
potential crash/corruption scenario Similar to commit #3 and #5 (both
Status: NO), this touches BUG() handling, but unlike those commits which
make broader architectural changes to error handling patterns, this fix
is much more contained. **4. Stable Tree Criteria Assessment:** ✅
**Fixes important bug**: Prevents potential memory corruption/crashes ✅
**Small and contained**: Single line addition ✅ **Minimal risk**: Only
affects error path when insert_state() fails AND CONFIG_BUG is disabled
✅ **No new features**: Pure bug fix ✅ **No architectural changes**:
Preserves existing error handling, just prevents fallthrough ✅
**Critical subsystem**: btrfs filesystem corruption prevention ✅ **Clear
side effects**: No unintended consequences beyond fixing the bug **5.
Risk Assessment:** - **Very Low Risk**: The change only affects an error
condition that's already problematic - **Exotic scenario**: Only impacts
systems with `CONFIG_BUG` disabled (uncommon but not impossible) - **No
regression potential**: The change only prevents executing invalid code,
doesn't change normal operation - **Well-contained**: Affects only one
function in one file **6. Security Implications:** While `CONFIG_BUG`
disabled is uncommon, this could potentially be exploited if an attacker
can trigger the `insert_state()` failure condition, leading to memory
corruption. The fix prevents this attack vector. This is a clear
candidate for stable backporting - it fixes a real bug with minimal risk
and follows the stable tree rules perfectly.

 fs/btrfs/extent-io-tree.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 6d08c100b01de..bb3aaf610652a 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -1456,6 +1456,7 @@ int convert_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		if (IS_ERR(inserted_state)) {
 			ret = PTR_ERR(inserted_state);
 			extent_io_tree_panic(tree, prealloc, "insert", ret);
+			goto out;
 		}
 		cache_state(inserted_state, cached_state);
 		if (inserted_state == prealloc)
-- 
2.39.5

[PATCH AUTOSEL 6.12 4/7] btrfs: exit after state split error at set_extent_bit()

[Sasha Levin]

From: Filipe Manana <fdmanana@suse.com>

[ Upstream commit 41d69d4d78d8b179bf3bcdfc56d28a12b3a608d2 ]

If split_state() returned an error we call extent_io_tree_panic() which
will trigger a BUG() call. However if CONFIG_BUG is disabled, which is an
uncommon and exotic scenario, then we fallthrough and hit a use after free
when calling set_state_bits() since the extent state record which the
local variable 'prealloc' points to was freed by split_state().

So jump to the label 'out' after calling extent_io_tree_panic() and set
the 'prealloc' pointer to NULL since split_state() has already freed it
when it hit an error.

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

**YES** This commit should be backported to stable kernel trees. Here's
my extensive analysis: ## Analysis of the Code Changes The commit fixes
a critical use-after-free bug in the btrfs filesystem's extent I/O tree
management. Examining the specific code changes: **Problem Location**:
In `fs/btrfs/extent-io-tree.c`, function `set_extent_bit()` around lines
1254-1256: ```c ret = split_state(tree, state, prealloc, end + 1); if
(ret) extent_io_tree_panic(tree, state, "split", ret); ``` **The Bug**:
After `split_state()` fails and `extent_io_tree_panic()` is called, the
code continues to execute `set_state_bits(tree, prealloc, bits,
changeset)` on line 1258. However, when `split_state()` fails, it frees
the `prealloc` extent state, making this a use-after-free vulnerability.
**The Fix**: The commit adds proper error handling: ```c ret =
split_state(tree, state, prealloc, end + 1); if (ret) {
extent_io_tree_panic(tree, state, "split", ret); prealloc = NULL; goto
out; } ``` ## Why This Should Be Backported ### 1. **Critical Security
Issue** - **Use-after-free vulnerability**: This is a serious memory
safety issue that can lead to kernel crashes, data corruption, or
potentially exploitable conditions - **Affects btrfs filesystem**: A
widely used filesystem in production environments ### 2. **Specific Edge
Case Scenario** From my examination of the kernel code: -
`extent_io_tree_panic()` calls `btrfs_panic()` which calls `BUG()` -
When `CONFIG_BUG` is disabled (uncommon but possible), `BUG()` becomes a
no-op loop instead of halting execution - This allows execution to
continue to the use-after-free code ### 3. **Minimal Risk Change** -
**Small, targeted fix**: Only adds 3 lines of code - **Clear logic**:
Sets pointer to NULL and jumps to cleanup - **No functional changes**:
Doesn't alter normal operation paths - **No architectural changes**:
Pure bug fix without affecting subsystem design ### 4. **Comparison with
Similar Commits** Looking at the reference commits: - Similar to
"Similar Commit #2" (marked YES) which also improved error handling in
extent bit operations - Unlike commits marked NO, this doesn't add
features or make architectural changes - Follows the same pattern as
other accepted backports for memory safety fixes ### 5. **Clear Bug Fix
Criteria** - **Fixes a real bug**: Use-after-free is a concrete,
exploitable issue - **Minimal scope**: Only affects error path in one
function - **No side effects**: Change only affects already-failing code
paths - **Well-understood impact**: Risk is contained to btrfs extent
I/O operations ### 6. **Production Impact** - While `CONFIG_BUG` being
disabled is rare, when it occurs this creates a serious vulnerability -
btrfs is used in production environments where memory safety is critical
- The fix prevents potential kernel crashes or memory corruption ### 7.
**Stable Tree Compatibility** - **Self-contained**: Doesn't depend on
other recent changes - **Backwards compatible**: No API changes - **Low
regression risk**: Only affects error handling path that was already
broken The commit perfectly fits stable tree criteria: it's a small,
important bugfix with minimal risk that addresses a real security issue
in a widely-used filesystem.

 fs/btrfs/extent-io-tree.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index bb3aaf610652a..5f9a43734812e 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -1252,8 +1252,11 @@ static int __set_extent_bit(struct extent_io_tree *tree, u64 start, u64 end,
 		if (!prealloc)
 			goto search_again;
 		ret = split_state(tree, state, prealloc, end + 1);
-		if (ret)
+		if (ret) {
 			extent_io_tree_panic(tree, state, "split", ret);
+			prealloc = NULL;
+			goto out;
+		}
 
 		set_state_bits(tree, prealloc, bits, changeset);
 		cache_state(prealloc, cached_state);
-- 
2.39.5