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

[Filipe Manana]

On Thu, Oct 29, 2026 at 2:12 PM Cheng Yongkang <teel4res@xxxxxxxxx> wrote:
>
> 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:
>
>   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@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Cheng Yongkang <teel4res@xxxxxxxxx>
> ---
>  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

I find this puzzling, using the "represent" word since
btrfs_chunk_item_size() does not represent anything, it just does a
calculation.

> +                * stripes; such a chunk is also invalid in the system chunk
> +                * array (system chunks are never remapped).  Check before

Also the "remapped" detail is also confusing and I can't grasp the
intention or why it matters.

> +                * btrfs_chunk_item_size() so a crafted super block is rejected
> +                * with -EUCLEAN instead of tripping its ASSERT().

Something simpler and more clear for the whole comment:

"We can not have 0 stripes, and passing 0 to btrfs_chunk_item_size()
triggers an ASSERT(), so fail immediately if we have 0 stripes."

Codewise it looks good, thanks.

> +                */
> +               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);
> --
> 2.34.1
>
>