Re: [PATCH v2] btrfs: tree-checker: introduce checks for FREE_SPACE_BITMAP

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2026/3/10 21:26, ZhengYuan Huang 写道:
> Introduce checks for FREE_SPACE_BITMAP item, which include:
> 
> - Key alignment check
>    Same as FREE_SPACE_EXTENT, the objectid is the logical bytenr of the
>    free space, and offset is the length of the free space, so both
>    should be aligned to the fs block size.
> 
> - Non-zero range check
>    A zero key->offset would describe an empty bitmap, which is invalid.
> 
> - Item size check
>    The item must hold exactly DIV_ROUND_UP(key->offset >> sectorsize_bits,
>    BITS_PER_BYTE) bytes.  A mismatch indicates a truncated or otherwise
>    corrupt bitmap item; without this check, the bitmap loading path would
>    walk past the end of the leaf and trigger a NULL dereference in
>    assert_eb_folio_uptodate().
> 
> Signed-off-by: ZhengYuan Huang <gality369@gmail.com>

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

And pushed to for-next branch with minor changes to resolve the 
conflicts with my patches on FREE_SPACE_INFO and FREE_SPACE_EXTENT.

Finally to reflect the review from Johannes on my patches, split the 
objectid and offset check into two, so that they will have a more 
explicit message on exactly which member is not aligned.

Thanks,
Qu

> ---
> [CHANGELOG]
> v2:
> - Move the FREE_SPACE_BITMAP item size validation from
>    load_free_space_bitmaps() in free-space-tree.c into tree-checker, so
>    corrupt bitmap items are rejected when the leaf is read from disk.
> - Drop the extent_buffer_test_bit() range check added in v1.
> - Rework the fix to follow Qu Wenruo's suggested tree-checker based
>    validation.
> ---
>   fs/btrfs/tree-checker.c | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index c10b4c242acf..0f12fe462b6c 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -1901,6 +1901,42 @@ static int check_dev_extent_item(const struct extent_buffer *leaf,
>   	return 0;
>   }
>   
> +static int check_free_space_bitmap(struct extent_buffer *leaf,
> +				   struct btrfs_key *key, int slot)
> +{
> +	struct btrfs_fs_info *fs_info = leaf->fs_info;
> +	const u32 blocksize = fs_info->sectorsize;
> +	u32 expected_item_size;
> +
> +	if (unlikely(!IS_ALIGNED(key->objectid, blocksize) ||
> +		     !IS_ALIGNED(key->offset, blocksize))) {
> +		generic_err(leaf, slot,
> +			    "free space bitmap key range is not aligned to %u, has (%llu %u %llu)",
> +			    blocksize, key->objectid, key->type, key->offset);
> +		return -EUCLEAN;
> +	}
> +	if (unlikely(key->offset == 0)) {
> +		generic_err(leaf, slot,
> +			    "free space bitmap range is 0");
> +		return -EUCLEAN;
> +	}
> +	/*
> +	 * The item must hold exactly the right number of bitmap bytes for the
> +	 * range described by key->offset.  A mismatch means the item was
> +	 * truncated or the key is corrupt; either way the bitmap data is not
> +	 * safe to access.
> +	 */
> +	expected_item_size = DIV_ROUND_UP(key->offset >> fs_info->sectorsize_bits,
> +					  BITS_PER_BYTE);
> +	if (unlikely(btrfs_item_size(leaf, slot) != expected_item_size)) {
> +		generic_err(leaf, slot,
> +			    "invalid item size for free space bitmap, has %u expect %u",
> +			    btrfs_item_size(leaf, slot), expected_item_size);
> +		return -EUCLEAN;
> +	}
> +	return 0;
> +}
> +
>   /*
>    * Common point to switch the item-specific validation.
>    */
> @@ -1964,6 +2000,9 @@ static enum btrfs_tree_block_status check_leaf_item(struct extent_buffer *leaf,
>   	case BTRFS_RAID_STRIPE_KEY:
>   		ret = check_raid_stripe_extent(leaf, key, slot);
>   		break;
> +	case BTRFS_FREE_SPACE_BITMAP_KEY:
> +		ret = check_free_space_bitmap(leaf, key, slot);
> +		break;
>   	}
>   
>   	if (unlikely(ret))