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

[Qu Wenruo <wqu@suse.com>]

在 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@suse.com>
> 
> Thanks,
> Qu
> 
>> Cc: stable@vger.kernel.org # 5.3+
>> Signed-off-by: ZhengYuan Huang <gality369@gmail.com>
>> ---
>> [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)) {
> 
>