Re: [PATCH] btrfs: reject zero num_stripes in the system chunk array

[Qu Wenruo <wqu@suse.com>]

在 2026/10/30 00:42, Cheng Yongkang 写道:
> validate_sys_chunk_array() reads the on-disk num_stripes and passes it to
> btrfs_chunk_item_size() to check that the chunk item fits in the system
> chunk array.  btrfs_chunk_item_size() has ASSERT(num_stripes), and the
> only num_stripes == 0 rejection is in btrfs_check_chunk_valid(), which is
> called later.  A crafted superblock with a BTRFS_CHUNK_ITEM_KEY entry
> whose btrfs_chunk has num_stripes == 0 therefore trips the assertion
> before the invalid chunk is rejected:

Since it will be checked by btrfs_check_chunk_valid(), why not remove 
the ASSERT(num_stripes) and let it continue to btrfs_check_chunk_valid()?

The whole idea of tree-checker is to concentrate the checks, not 
distribute them again.

> 
>    assertion failed: num_stripes, in fs/btrfs/volumes.h:714
>    kernel BUG at fs/btrfs/volumes.h:714!
>    RIP: 0010:btrfs_validate_super+0x3ce7/0x4210
>    Call Trace:
>     open_ctree
>     btrfs_get_tree
>     vfs_get_tree
>     do_new_mount
> 
> Commit 2a9bb78cfd36 ("btrfs: validate system chunk array at
> btrfs_validate_super()") moved the validation out of
> btrfs_read_sys_array(), which rejected num_stripes == 0 explicitly.
> Reject zero stripes in validate_sys_chunk_array() before calling
> btrfs_chunk_item_size(), so the crafted superblock is rejected with
> -EUCLEAN before the assertion is evaluated.  open_ctree() turns the
> validation failure into -EINVAL for the mount.
> 
> Fixes: 2a9bb78cfd36 ("btrfs: validate system chunk array at btrfs_validate_super()")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
> ---
>   fs/btrfs/disk-io.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index c628c50..920ca4b 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2389,6 +2389,19 @@ static int validate_sys_chunk_array(const struct btrfs_fs_info *fs_info,
>   
>   		chunk = (struct btrfs_chunk *)(sb->sys_chunk_array + cur);
>   		num_stripes = btrfs_stack_chunk_num_stripes(chunk);
> +		/*
> +		 * btrfs_chunk_item_size() cannot represent a chunk without
> +		 * stripes; such a chunk is also invalid in the system chunk
> +		 * array (system chunks are never remapped).  Check before
> +		 * btrfs_chunk_item_size() so a crafted super block is rejected
> +		 * with -EUCLEAN instead of tripping its ASSERT().

Please let your agent to drop unnecessary comments.

It only wastes code space without providing any real benefit.

> +		 */
> +		if (unlikely(num_stripes == 0)) {
> +			btrfs_err(fs_info,
> +				  "invalid chunk num_stripes 0 in sys_array at offset %u",
> +				  cur);
> +			return -EUCLEAN;
> +		}
>   		if (unlikely(cur + btrfs_chunk_item_size(num_stripes) > sys_array_size))
>   			goto short_read;
>   		type = btrfs_stack_chunk_type(chunk);