Re: [PATCH 2/2] btrfs: tree-checker: reject extent owner ref without simple quota

[Qu Wenruo <wqu@suse.com>]

在 2026/11/3 14:19, Siddharth Mishra 写道:
> [BUG]
> An extent item can contain an owner ref.  The owner ref is correct only
> when the super block has the SIMPLE_QUOTA bit.  If the bit is not set,
> check_extent_item() gives a WARN_ON() and then accepts the leaf :
> 
> 	case BTRFS_EXTENT_OWNER_REF_KEY:
> 		WARN_ON(!btrfs_fs_incompat(fs_info, SIMPLE_QUOTA));
> 		break;
> 
> For all other incorrect inline refs, check_extent_item() returns
> -EUCLEAN.  After the tree-checker accepts the leaf, a free of the extent
> calls btrfs_get_extent_inline_ref_type().  This function ASSERT()s the
> SIMPLE_QUOTA bit.
> 
> I made a crafted image : an owner ref, no SIMPLE_QUOTA bit, and no quota
> tree.  On a kernel with CONFIG_BTRFS_ASSERT=y, I removed the file of the
> extent.  At the next commit, btrfs frees the extent and gives :

I don't think this should go into BUG section, but CAUSE.

> 
>    WARNING: fs/btrfs/tree-checker.c:1782 at __btrfs_check_leaf
>    assertion failed: btrfs_fs_incompat(fs_info, SIMPLE_QUOTA),
>    in fs/btrfs/extent-tree.c:354
>    kernel BUG at fs/btrfs/extent-tree.c:354!
>    Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>    RIP: 0010:btrfs_get_extent_inline_ref_type.cold
>    (fs/btrfs/extent-tree.c:354)
>    Call Trace:
>     lookup_inline_extent_backref (fs/btrfs/extent-tree.c:919)
>     lookup_extent_backref (fs/btrfs/extent-tree.c:1100)
>     __btrfs_free_extent.isra.0 (fs/btrfs/extent-tree.c:3319)
>     __btrfs_run_delayed_refs (fs/btrfs/extent-tree.c:2134)
>     btrfs_run_delayed_refs (fs/btrfs/extent-tree.c:2246)
>     btrfs_commit_transaction (fs/btrfs/transaction.c:2278)
>     ksys_sync (fs/sync.c:104)
> 
> Without CONFIG_BTRFS_ASSERT, the kernel gives only the WARN_ON().  Then
> btrfs continues to use the owner ref.
> 
> [CAUSE]
> For an owner ref without the SIMPLE_QUOTA bit, the tree-checker gives a
> WARN_ON() and keeps the leaf.  For all other inline ref types, it
> rejects the leaf.  Two sources can put owner refs without the
> SIMPLE_QUOTA bit on a disk : a damaged or crafted image, and a kernel
> without commit f2363e6fcc79 ("btrfs: fix transaction atomicity bug when
> enabling simple quotas").
> 
> [FIX]
> Reject the leaf with -EUCLEAN, as for all other incorrect inline refs.
> Then btrfs finds the problem when it reads the leaf, and the ASSERT() is
> not reached.
> 
> syzbot reported the WARN_ON() and the ASSERT() in 2024 without a
> reproducer.  Both reports were closed for inactivity.  At that time, the
> WARN_ON() was noted as something that "should be handled in a better
> way".  I found the cause during the analysis of a fuzzing report.  An AI
> tool helped.  I tested with a crafted image on btrfs for-next
> (d6f76ca0a70d).  Without the patch, the kernel gives the WARN_ON(), and
> the BUG above with CONFIG_BTRFS_ASSERT=y.  With the patch, the
> tree-checker rejects the leaf ("extent owner ref without simple quota")
> and the mount fails with -EIO.  There is no WARN_ON() and no BUG.  A
> clean simple quota filesystem still writes data, frees extents and
> mounts again without errors.  The fstests group qgroup has no new
> failure (34 passed, 3 not run here).  The crafted image and the commands
> are available on request.

Again, remove the last paragraph.

> 
> Link: https://syzkaller.appspot.com/bug?extid=d7d1fc7e21835ca19219
> Link: https://syzkaller.appspot.com/bug?extid=4983e68a6d77180bfe33
> Fixes: d9a620f77e33 ("btrfs: new inline ref storing owning subvol of data extents")

Again, I don't think this really counts as a fixes.

Or all hardening would be considered as a fix, which makes no sense.

> Cc: stable@vger.kernel.org # 6.7+
> Signed-off-by: Siddharth Mishra <smish149@asu.edu>
> ---
>   fs/btrfs/tree-checker.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 0d75cb537..5c44bed99 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -1779,7 +1779,11 @@ static int check_extent_item(struct extent_buffer *leaf,
>   			inline_refs += btrfs_shared_data_ref_count(leaf, sref);
>   			break;
>   		case BTRFS_EXTENT_OWNER_REF_KEY:
> -			WARN_ON(!btrfs_fs_incompat(fs_info, SIMPLE_QUOTA));
> +			if (unlikely(!btrfs_fs_incompat(fs_info, SIMPLE_QUOTA))) {
> +				extent_err(leaf, slot,
> +					   "extent owner ref without simple quota");
> +				return -EUCLEAN;
> +			}
>   			break;
>   		default:
>   			extent_err(leaf, slot, "unknown inline ref type: %u",