mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] btrfs: reject zero num_stripes in the system chunk array
@ 2026-10-29 14:12 Cheng Yongkang
  2026-10-29 17:55 ` Filipe Manana
  2026-10-29 21:00 ` Qu Wenruo
  0 siblings, 2 replies; 3+ messages in thread
From: Cheng Yongkang @ 2026-10-29 14:12 UTC (permalink / raw)
  To: David Sterba; +Cc: Chris Mason, linux-btrfs, linux-kernel

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@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().
+		 */
+		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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] btrfs: reject zero num_stripes in the system chunk array
  2026-10-29 14:12 [PATCH] btrfs: reject zero num_stripes in the system chunk array Cheng Yongkang
@ 2026-10-29 17:55 ` Filipe Manana
  2026-10-29 21:00 ` Qu Wenruo
  1 sibling, 0 replies; 3+ messages in thread
From: Filipe Manana @ 2026-10-29 17:55 UTC (permalink / raw)
  To: Cheng Yongkang; +Cc: David Sterba, Chris Mason, linux-btrfs, linux-kernel

On Thu, Oct 29, 2026 at 2:12 PM Cheng Yongkang <teel4res@gmail.com> 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@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

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
>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] btrfs: reject zero num_stripes in the system chunk array
  2026-10-29 14:12 [PATCH] btrfs: reject zero num_stripes in the system chunk array Cheng Yongkang
  2026-10-29 17:55 ` Filipe Manana
@ 2026-10-29 21:00 ` Qu Wenruo
  1 sibling, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2026-10-29 21:00 UTC (permalink / raw)
  To: Cheng Yongkang, David Sterba; +Cc: Chris Mason, linux-btrfs, linux-kernel



在 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);


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-29 21:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-29 14:12 [PATCH] btrfs: reject zero num_stripes in the system chunk array Cheng Yongkang
2026-10-29 17:55 ` Filipe Manana
2026-10-29 21:00 ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®