patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.12 1/7] btrfs: exit after state insertion failure at btrfs_convert_extent_bit()
@ 2025-05-28 21:56 Sasha Levin
  2025-05-28 21:56 ` [PATCH AUTOSEL 6.12 2/7] fs/filesystems: Fix potential unsigned integer underflow in fs_name() Sasha Levin
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Sasha Levin @ 2025-05-28 21:56 UTC (permalink / raw)
  To: patches, stable
  Cc: Filipe Manana, David Sterba, Sasha Levin, clm, josef, linux-btrfs,
	linux-kernel

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-05-28 21:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-28 21:56 [PATCH AUTOSEL 6.12 1/7] btrfs: exit after state insertion failure at btrfs_convert_extent_bit() Sasha Levin
2025-05-28 21:56 ` [PATCH AUTOSEL 6.12 2/7] fs/filesystems: Fix potential unsigned integer underflow in fs_name() Sasha Levin
2025-05-28 21:56 ` [PATCH AUTOSEL 6.12 3/7] gfs2: pass through holder from the VFS for freeze/thaw Sasha Levin
2025-05-28 21:56 ` [PATCH AUTOSEL 6.12 4/7] btrfs: exit after state split error at set_extent_bit() Sasha Levin
2025-05-28 21:56 ` [PATCH AUTOSEL 6.12 5/7] nvmet-fcloop: access fcpreq only when holding reqlock Sasha Levin
2025-05-28 21:56 ` [PATCH AUTOSEL 6.12 6/7] perf: Ensure bpf_perf_link path is properly serialized Sasha Levin
2025-05-28 21:56 ` [PATCH AUTOSEL 6.12 7/7] block: use q->elevator with ->elevator_lock held in elv_iosched_show() Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).