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

[Qu Wenruo <wqu@suse.com>]

在 2026/11/3 14:19, 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 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.

The easiest way to show the cause is to provide a tree dump of the 
offending leaf, and highlight the offending tree blocks.

> 
> [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.
> 
> Fuzzing found this bug. An AI tool helped to analyse the cause and to
> write the fix. I tested on btrfs for-next (d6f76ca0a70d), with and
> without KASAN and CONFIG_BTRFS_ASSERT=y. Without the patch, the kernel
> gives the BUG above, or the WARN_ON() at the first write. With the
> patch, the tree-checker rejects the leaf ("qgroup status item has
> SIMPLE_MODE but SIMPLE_QUOTA incompat bit unset"). The mount is
> successful without quotas. There is no BUG and no WARN_ON(). A clean
> simple quota filesystem still mounts, writes data, frees extents and
> mounts again. Its status item passes the check at each commit. The
> fstests group qgroup has no new failure (34 passed, 3 not run here). A
> crafted image and a C reproducer are available on request.

Drop the whole paragraph starting with "Fuzzing found".

It makes no sense. And tell your agent to write this rule into its 
memory file.

This looks very chatgpt, and so far I have no good experience with 
patches generated by that.

> 
> Fixes: 182940f4f4db ("btrfs: qgroup: add new quota mode for simple quotas")

I don't think this should count as a fix.

> Cc: stable@vger.kernel.org # 6.7+
> Signed-off-by: Siddharth Mishra <smish149@asu.edu>
> ---
>   fs/btrfs/tree-checker.c | 35 +++++++++++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 61ea8f42c..0d75cb537 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -2083,6 +2083,38 @@ 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;
> +
> +	/* version, generation, flags and rescan are always present. */

I don't think the comment adds much value.

Tell your agent to keep comments at minimal.

> +	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;
> +
> +	/* Simple quotas need the incompat bit and the enable_gen member. */
> +	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 +2507,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: