[PATCH v3] btrfs: free-space-tree: reject mismatched extent and bitmap items

Zhang Cen posted 1 patch 1 month ago
There is a newer version of this series
fs/btrfs/free-space-tree.c | 37 ++++++++++++++++++++++++++++++++-----
fs/btrfs/tree-checker.c    |  4 ++++
2 files changed, 36 insertions(+), 5 deletions(-)
[PATCH v3] btrfs: free-space-tree: reject mismatched extent and bitmap items
Posted by Zhang Cen 1 month ago
btrfs_load_free_space_tree() reads FREE_SPACE_INFO once and then chooses
the bitmap or extent loader for all following free-space records until the
next FREE_SPACE_INFO item. Those loaders currently enforce the selected
record type only with ASSERT().

On production builds without CONFIG_BTRFS_ASSERT, a malformed free-space
tree can therefore be decoded in the wrong mode. An EXTENT item can reach
btrfs_free_space_test_bit() as bitmap data, while a BITMAP item can be
added as a full free extent. The latter corrupts the in-memory free-space
cache and the former can read beyond the item payload.

Validate every post-info key before decoding it. Reject keys whose type
does not match the mode selected by FREE_SPACE_INFO, and reject keys
whose range extends past the block group, returning -EUCLEAN instead of
feeding the wrong record type to the bitmap or extent decoder.

Also reject zero-length FREE_SPACE_EXTENT items in tree-checker, matching
the existing FREE_SPACE_BITMAP zero-length check. This keeps the loader
range check simple and prevents a zero-length extent item from being a
valid on-disk free-space record.

A malformed extent-as-bitmap record was observed as a KASAN fault in
extent_buffer_test_bit() (fs/btrfs/extent_io.c:4313), reached through
btrfs_free_space_test_bit() (fs/btrfs/free-space-tree.c:518) from
load_free_space_bitmaps() (fs/btrfs/free-space-tree.c:1603).

Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
---
Changes since v2:
- Regenerate the mail-ready patch without the nested mbox wrapper.
- No code changes beyond the v2 fix.

 fs/btrfs/free-space-tree.c | 37 ++++++++++++++++++++++++++++++++-----
 fs/btrfs/tree-checker.c    |  4 ++++
 2 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 472b3060e5ac..c1af70761919 100644
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1545,6 +1545,30 @@ int btrfs_remove_block_group_free_space(struct btrfs_trans_handle *trans,
 	return 0;
 }
 
+static int validate_free_space_key(struct btrfs_block_group *block_group,
+				   const struct btrfs_key *key,
+				   u8 expected_type)
+{
+	const u64 end = btrfs_block_group_end(block_group);
+
+	if (unlikely(key->type != expected_type)) {
+		btrfs_err(block_group->fs_info,
+			  "block group %llu has unexpected free space key type %u, expected %u",
+			  block_group->start, key->type, expected_type);
+		return -EUCLEAN;
+	}
+
+	if (unlikely(key->objectid + key->offset > end)) {
+		btrfs_err(block_group->fs_info,
+			  "block group %llu has invalid free space key (%llu %u %llu)",
+			  block_group->start, key->objectid, key->type,
+			  key->offset);
+		return -EUCLEAN;
+	}
+
+	return 0;
+}
+
 static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
 				   struct btrfs_path *path,
 				   u32 expected_extent_count)
@@ -1576,8 +1600,10 @@ static int load_free_space_bitmaps(struct btrfs_caching_control *caching_ctl,
 		if (key.type == BTRFS_FREE_SPACE_INFO_KEY)
 			break;
 
-		ASSERT(key.type == BTRFS_FREE_SPACE_BITMAP_KEY);
-		ASSERT(key.objectid < end && key.objectid + key.offset <= end);
+		ret = validate_free_space_key(block_group, &key,
+					      BTRFS_FREE_SPACE_BITMAP_KEY);
+		if (unlikely(ret))
+			return ret;
 
 		offset = key.objectid;
 		while (offset < key.objectid + key.offset) {
@@ -1633,7 +1659,6 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
 	struct btrfs_fs_info *fs_info = block_group->fs_info;
 	struct btrfs_root *root;
 	struct btrfs_key key;
-	const u64 end = btrfs_block_group_end(block_group);
 	u64 total_found = 0;
 	u32 extent_count = 0;
 	int ret;
@@ -1654,8 +1679,10 @@ static int load_free_space_extents(struct btrfs_caching_control *caching_ctl,
 		if (key.type == BTRFS_FREE_SPACE_INFO_KEY)
 			break;
 
-		ASSERT(key.type == BTRFS_FREE_SPACE_EXTENT_KEY);
-		ASSERT(key.objectid < end && key.objectid + key.offset <= end);
+		ret = validate_free_space_key(block_group, &key,
+					      BTRFS_FREE_SPACE_EXTENT_KEY);
+		if (unlikely(ret))
+			return ret;
 
 		ret = btrfs_add_new_free_space(block_group, key.objectid,
 					       key.objectid + key.offset,
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 1f15d0793a9c..ec24ffb6641d 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -2129,6 +2129,10 @@ static int check_free_space_extent(struct extent_buffer *leaf, struct btrfs_key
 			    blocksize, BTRFS_KEY_FMT_VALUE(key));
 		return -EUCLEAN;
 	}
+	if (unlikely(key->offset == 0)) {
+		generic_err(leaf, slot, "free space extent length is 0");
+		return -EUCLEAN;
+	}
 	if (unlikely(btrfs_item_size(leaf, slot) != 0)) {
 		generic_err(leaf, slot,
 			    "invalid item size for free space info, has %u expect 0",
-- 
2.43.0
Re: [PATCH v3] btrfs: free-space-tree: reject mismatched extent and bitmap items
Posted by David Sterba 1 month ago
On Sun, May 10, 2026 at 11:03:22PM +0800, Zhang Cen wrote:
> +static int validate_free_space_key(struct btrfs_block_group *block_group,
> +				   const struct btrfs_key *key,
> +				   u8 expected_type)
> +{
> +	const u64 end = btrfs_block_group_end(block_group);
> +
> +	if (unlikely(key->type != expected_type)) {
> +		btrfs_err(block_group->fs_info,
> +			  "block group %llu has unexpected free space key type %u, expected %u",
> +			  block_group->start, key->type, expected_type);
> +		return -EUCLEAN;
> +	}
> +
> +	if (unlikely(key->objectid + key->offset > end)) {

This has a review comment https://sashiko.dev/#/patchset/20260510152848.3844894-1-rollkingzzc%40gmail.com
that the expression can overflow. I did a light check if it is valid at
all (we're still evaluating the quality of AI reviews) but I think in
this case more checks to the key.offset and key.objectid could be added.
As this would is in checker it's meant to be extensive so partially
overlapping conditions are not a problem.
Re: [PATCH v3] btrfs: free-space-tree: reject mismatched extent and bitmap items
Posted by Qu Wenruo 1 month ago

在 2026/5/13 00:48, David Sterba 写道:
> On Sun, May 10, 2026 at 11:03:22PM +0800, Zhang Cen wrote:
>> +static int validate_free_space_key(struct btrfs_block_group *block_group,
>> +				   const struct btrfs_key *key,
>> +				   u8 expected_type)
>> +{
>> +	const u64 end = btrfs_block_group_end(block_group);
>> +
>> +	if (unlikely(key->type != expected_type)) {
>> +		btrfs_err(block_group->fs_info,
>> +			  "block group %llu has unexpected free space key type %u, expected %u",
>> +			  block_group->start, key->type, expected_type);
>> +		return -EUCLEAN;
>> +	}
>> +
>> +	if (unlikely(key->objectid + key->offset > end)) {
> 
> This has a review comment https://sashiko.dev/#/patchset/20260510152848.3844894-1-rollkingzzc%40gmail.com
> that the expression can overflow. I did a light check if it is valid at
> all (we're still evaluating the quality of AI reviews) but I think in
> this case more checks to the key.offset and key.objectid could be added.
> As this would is in checker it's meant to be extensive so partially
> overlapping conditions are not a problem.
> 
I have already submitted tree-checker enhancement to address this:
https://lore.kernel.org/linux-btrfs/cover.1778460959.git.wqu@suse.com/T/

Which adds extra overflow and overlap checks for fst.

Thanks,
Qu
Re: [PATCH v3] btrfs: free-space-tree: reject mismatched extent and bitmap items
Posted by Qu Wenruo 1 month ago

在 2026/5/11 00:33, Zhang Cen 写道:
> btrfs_load_free_space_tree() reads FREE_SPACE_INFO once and then chooses
> the bitmap or extent loader for all following free-space records until the
> next FREE_SPACE_INFO item. Those loaders currently enforce the selected
> record type only with ASSERT().
> 
> On production builds without CONFIG_BTRFS_ASSERT, a malformed free-space
> tree can therefore be decoded in the wrong mode. An EXTENT item can reach
> btrfs_free_space_test_bit() as bitmap data, while a BITMAP item can be
> added as a full free extent. The latter corrupts the in-memory free-space
> cache and the former can read beyond the item payload.
> 
> Validate every post-info key before decoding it. Reject keys whose type
> does not match the mode selected by FREE_SPACE_INFO, and reject keys
> whose range extends past the block group, returning -EUCLEAN instead of
> feeding the wrong record type to the bitmap or extent decoder.
> 
> Also reject zero-length FREE_SPACE_EXTENT items in tree-checker, matching
> the existing FREE_SPACE_BITMAP zero-length check. This keeps the loader
> range check simple and prevents a zero-length extent item from being a
> valid on-disk free-space record.
> 
> A malformed extent-as-bitmap record was observed as a KASAN fault in
> extent_buffer_test_bit() (fs/btrfs/extent_io.c:4313), reached through
> btrfs_free_space_test_bit() (fs/btrfs/free-space-tree.c:518) from
> load_free_space_bitmaps() (fs/btrfs/free-space-tree.c:1603).
> 
> Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
> ---
> Changes since v2:
> - Regenerate the mail-ready patch without the nested mbox wrapper.
> - No code changes beyond the v2 fix.

If you want to put a changelog, please include all modification, not 
only the last modification but from the very beginning.

A proper example:

https://lore.kernel.org/linux-btrfs/335133ce1989ac89a6de007d4db05f5f4a6c1be2.1775491985.git.boris@bur.io/

Otherwise looks good to me.

Reviewed-by: Qu Wenruo <wqu@suse.com>

I'll give it a full fstests run before merging.

Thanks,
Qu

Re: [PATCH v3] btrfs: free-space-tree: reject mismatched extent and bitmap items
Posted by Cen Zhang 1 month ago
>
> If you want to put a changelog, please include all modification, not
> only the last modification but from the very beginning.
>
> A proper example:
>
> https://lore.kernel.org/linux-btrfs/335133ce1989ac89a6de007d4db05f5f4a6c1be2.1775491985.git.boris@bur.io/
>
> Otherwise looks good to me.
>
> Reviewed-by: Qu Wenruo <wqu@suse.com>
>
> I'll give it a full fstests run before merging.
>

Understood, thanks. I will include the full changelog from the
beginning in future.

Thank you for the review and guidence.

Thanks,
Zhang Cen