Re: [PATCH v2 1/2] btrfs: tree-checker: validate qgroup status item

[Qu Wenruo]

在 2026/11/3 16:40, Siddharth Mishra 写道:
> [BUG]
> An image can have a status item with SIMPLE_MODE and a super block
> without the SIMPLE_QUOTA bit.  On a kernel with CONFIG_BTRFS_ASSERT=y,
> the mount of this image causes a BUG :
>
>    assertion failed: btrfs_fs_incompat(fs_info, SIMPLE_QUOTA),
>    in fs/btrfs/qgroup.c:411
>    kernel BUG at fs/btrfs/qgroup.c:411!
>    Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>    RIP: 0010:btrfs_read_qgroup_config.cold (fs/btrfs/qgroup.c:411)
>    Call Trace:
>     open_ctree (fs/btrfs/disk-io.c:3789)
>     btrfs_get_tree.cold (fs/btrfs/super.c:2133)
>     vfs_get_tree (fs/super.c:1943)
>     fc_mount (fs/namespace.c:1282)
>     path_mount (fs/namespace.c:4337)
>     __x64_sys_mount (fs/namespace.c:4542)
>
> Without CONFIG_BTRFS_ASSERT, the mount is successful in simple quota
> mode.  The first data write then adds an owner ref.  The write-time
> tree-checker sees the owner ref without the SIMPLE_QUOTA bit and gives
> a WARN_ON() (tree-checker.c:1782).
>
> [CAUSE]
> The super block does not have the SIMPLE_QUOTA bit, but the status item
> has SIMPLE_MODE :
>
>    incompat_flags  0x361
>                    ( MIXED_BACKREF |
>                      BIG_METADATA |
>                      EXTENDED_IREF |
>                      SKINNY_METADATA |
>                      NO_HOLES )
>
>    item 0 key (0 QGROUP_STATUS 0) itemoff 16243 itemsize 40
>            version 1 generation 12 flags ON|SIMPLE_MODE scan 0
>                                             ^^^^^^^^^^^
>
> The tree-checker does not examine the status item.
> btrfs_read_qgroup_config() reads the flags of the status item.  For
> SIMPLE_MODE, it calls qgroup_read_enable_gen().  This function ASSERT()s
> that the SIMPLE_QUOTA bit is set and that the item contains enable_gen.
> Since commit f2363e6fcc79, the kernel does not write SIMPLE_MODE without
> the SIMPLE_QUOTA bit.  Thus only a damaged or crafted image has this
> state.
>
> [FIX]
> Add a tree-checker rule for the status item :
> - The item must contain the four members of all versions : version,
>    generation, flags and rescan.
> - With SIMPLE_MODE, the super block must have the SIMPLE_QUOTA bit, and
>    the item must also contain enable_gen.
> If a rule fails, the tree-checker rejects the leaf with -EUCLEAN.  Then
> btrfs_read_roots() cannot read the quota root.  It ignores the error, as
> before, and btrfs mounts the filesystem without quotas.
>
> Cc: stable@xxxxxxxxxxxxxxx # 6.7+

No fixes means no CC to stable.

I'll drop it at merge time.

Otherwise looks good to me.

Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>

> Signed-off-by: Siddharth Mishra <smish149@xxxxxxx>
> ---
>   fs/btrfs/tree-checker.c | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 61ea8f42c..bb86c30a2 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -2083,6 +2083,36 @@ static int check_raid_stripe_extent(const struct extent_buffer *leaf,
>   	return 0;
>   }
>   
> +static int check_qgroup_status_item(const struct extent_buffer *leaf, int slot)
> +{
> +	const u32 item_size = btrfs_item_size(leaf, slot);
> +	const u32 min_size = offsetof(struct btrfs_qgroup_status_item, enable_gen);
> +	struct btrfs_qgroup_status_item *ptr;
> +
> +	if (unlikely(item_size < min_size)) {
> +		generic_err(leaf, slot,
> +		"invalid qgroup status item size, have %u expect at least %u",
> +			    item_size, min_size);
> +		return -EUCLEAN;
> +	}
> +	ptr = btrfs_item_ptr(leaf, slot, struct btrfs_qgroup_status_item);
> +	if (!(btrfs_qgroup_status_flags(leaf, ptr) & BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE))
> +		return 0;
> +
> +	if (unlikely(!btrfs_fs_incompat(leaf->fs_info, SIMPLE_QUOTA))) {
> +		generic_err(leaf, slot,
> +	"qgroup status item has SIMPLE_MODE but SIMPLE_QUOTA incompat bit unset");
> +		return -EUCLEAN;
> +	}
> +	if (unlikely(item_size < sizeof(*ptr))) {
> +		generic_err(leaf, slot,
> +	"invalid qgroup status item size for simple quota, have %u expect at least %zu",
> +			    item_size, sizeof(*ptr));
> +		return -EUCLEAN;
> +	}
> +	return 0;
> +}
> +
>   static int check_remap_key(const struct extent_buffer *leaf,
>   			   const struct btrfs_key *key, int slot)
>   {
> @@ -2475,6 +2505,9 @@ static enum btrfs_tree_block_status check_leaf_item(struct extent_buffer *leaf,
>   	case BTRFS_FREE_SPACE_BITMAP_KEY:
>   		ret = check_free_space_bitmap(leaf, key, slot, prev_key);
>   		break;
> +	case BTRFS_QGROUP_STATUS_KEY:
> +		ret = check_qgroup_status_item(leaf, slot);
> +		break;
>   	case BTRFS_IDENTITY_REMAP_KEY:
>   	case BTRFS_REMAP_KEY:
>   	case BTRFS_REMAP_BACKREF_KEY: