Re: [PATCH v2] btrfs: reject root items with drop_progress and zero drop_level

[Qu Wenruo]

在 2026/3/12 10:53, Qu Wenruo 写道:
[...]
> > Fixes: 259ee7754b67 ("btrfs: tree-checker: Add ROOT_ITEM check")
>
> Again, it's not a bug fix. You're just adding a new check.

More info about when to use fixes tag:

https://www.kernel.org/doc/html/v6.19/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes

It's more common to use it for a regression, aka, before that commit 
everything works, but at that commit something is broken.

And in your case, the commit is not causing the BUG_ON() thus it's 
incorrect.
Furthermore you'd better not put the commit introducing the BUG_ON() as 
the fixes target.


In your case, you're just enhancing the tree-checker to address a fuzzed 
image (which I guess you'll continue submitting such patches), thus 
getting the fixes tag done correctly will save everyone time.

Thanks,
Qu

> This fixes tag should only go with the error message fix.
>
> You don't need to send a new update, I'll do all the update at merge time.
>
> Otherwise looks good to me.
>
> Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
>
> Thanks,
> Qu
>
> > Cc: stable@xxxxxxxxxxxxxxx # 5.3+
> > Signed-off-by: ZhengYuan Huang <gality369@xxxxxxxxx>
> > ---
> > [CHANGELOG]
> > v2:
> > - Split out the error message fix from the previous patch, as requested
> >    during review.
> > ---
> >   fs/btrfs/tree-checker.c | 17 +++++++++++++++++
> >   1 file changed, 17 insertions(+)
> >
> > diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> > index dd274f67ad7f..1e052c3303b3 100644
> > --- a/fs/btrfs/tree-checker.c
> > +++ b/fs/btrfs/tree-checker.c
> > @@ -1260,6 +1260,23 @@ static int check_root_item(struct extent_buffer 
> > *leaf, struct btrfs_key *key,
> >                   btrfs_root_drop_level(&ri), BTRFS_MAX_LEVEL - 1);
> >           return -EUCLEAN;
> >       }
> > +    /*
> > +     * If drop_progress.objectid is non-zero, a btrfs_drop_snapshot() 
> > was
> > +     * interrupted and the resume point was recorded in drop_progress 
> > and
> > +     * drop_level.  In that case drop_level must be >= 1: level 0 is the
> > +     * leaf level and drop_snapshot never saves a checkpoint there (it
> > +     * only records checkpoints at internal node levels in 
> > DROP_REFERENCE
> > +     * stage).  A zero drop_level combined with a non-zero drop_progress
> > +     * objectid indicates on-disk corruption and would cause a BUG_ON in
> > +     * merge_reloc_root() and btrfs_drop_snapshot() at mount time.
> > +     */
> > +    if (unlikely(btrfs_disk_key_objectid(&ri.drop_progress) != 0 &&
> > +             btrfs_root_drop_level(&ri) == 0)) {
> > +        generic_err(leaf, slot,
> > +                "invalid root drop_level 0 with non-zero 
> > drop_progress objectid %llu",
> > +                btrfs_disk_key_objectid(&ri.drop_progress));
> > +        return -EUCLEAN;
> > +    }
> >       /* Flags check */
> >       if (unlikely(btrfs_root_flags(&ri) & ~valid_root_flags)) {