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

[Qu Wenruo]

在 2026/5/10 17:19, Zhang Cen 写道:
> btrfs_load_free_space_tree() picks bitmap or extent mode from the
> FREE_SPACE_INFO flags and then lets load_free_space_bitmaps() or
> load_free_space_extents() walk the following records until the next info
> item. Those walkers only verify the record type and range with ASSERT(),
> so production builds can decode an EXTENT item as bitmap data or accept
> a BITMAP item as a whole free extent.
>
> Add a shared runtime check for each post-info key and call it from both
> loaders before decoding the current record. Reject keys whose type does
> not match the mode selected by FREE_SPACE_INFO and keys whose range falls
> outside the block group with -EUCLEAN, instead of reaching
> btrfs_free_space_test_bit() or btrfs_add_new_free_space() with an
> unexpected record.
>
> Sanitizer validation reported a fatal fault in extent_buffer_test_bit()
> (fs/btrfs/extent_io.c:4313) through btrfs_free_space_test_bit()
> (fs/btrfs/free-space-tree.c:518), reached from load_free_space_bitmaps()
> (fs/btrfs/free-space-tree.c:1603) after an extent item was decoded as
> bitmap data.
>
> Sanitizer validation reported:
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
> Call trace:
>    assert_eb_folio_uptodate() (fs/btrfs/extent_io.c:4134)
>    extent_buffer_test_bit() (?:?)
>    btrfs_free_space_test_bit() (fs/btrfs/free-space-tree.c:518)
>    srso_alias_return_thunk() (arch/x86/include/asm/nospec-branch.h:375)
>    __entry_text_end() (?:?)
>    __asan_memcpy() (mm/kasan/shadow.c:103)
>    read_extent_buffer() (?:?)
>    load_free_space_bitmaps() (fs/btrfs/free-space-tree.c:1548)
>    btrfs_get_32() (fs/btrfs/free-space-tree.c:?)
>    btrfs_set_16() (fs/btrfs/free-space-tree.c:?)
>    kmem_cache_alloc_noprof() (?:?)
>    btrfs_load_free_space_tree() (fs/btrfs/free-space-tree.c:1685)
>    load_free_space_tree_for_test() (?:?)
>    rcu_disable_urgency_upon_qs() (kernel/rcu/tree.c:721)
>    vprintk_emit() (?:?)
>    __up_write() (kernel/locking/rwsem.c:1401)
>    clone_commit_root_for_test() (?:?)
>    test_extent_as_bitmap_mode_mismatch() (?:?)
>    kmem_cache_free() (?:?)
>    btrfs_free_path() (fs/btrfs/free-space-tree.c:1449)
>    __add_block_group_free_space() (fs/btrfs/free-space-tree.c:20)
>    run_test() (?:?)
>    do_raw_spin_unlock() (?:?)
>    btrfs_test_free_space_tree() (fs/btrfs/tests/free-space-tree-tests.c:547)
>    btrfs_test_qgroups() (fs/btrfs/tests/qgroup-tests.c:462)
>    btrfs_run_sanity_tests() (fs/btrfs/free-space-tree.c:?)
>    init_btrfs_fs() (fs/btrfs/super.c:2690)
>    do_one_initcall() (init/main.c:1382)
>    __kasan_kmalloc() (?:?)
>    rcu_is_watching() (?:?)
>    do_initcalls() (init/main.c:1457)
>    kernel_init_freeable() (init/main.c:1674)
>    kernel_init() (init/main.c:1584)
>    ret_from_fork() (?:?)
>    __switch_to() (?:?)
>    ret_from_fork_asm() (?:?)
>
> Signed-off-by: Zhang Cen <rollkingzzc@xxxxxxxxx>
>
> ---
> diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
> index 472b3060e5ac..e7fed8041eb1 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 (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 (key->objectid >= end || key->offset > end - key->objectid) {

The later half "key->offset > end - key->objectid" is unsafe and very 
hard to read.

"end - key->objectid" can underflow.

Change it to "key->objectid + key->offset > end" will be easier to read.

Furthermore, "key->offset" should never be zero, thus in that case
a single "key->objectid + key->offset > end" will be more than enough.

For the key->offset != 0 part, it can be validated inside tree-checker.


> +		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 (ret)

Please use unlikely() for every validate_free_space_key() failure.