Re: [PATCH 1/3] btrfs: avoid NULL pointer dereference when block group tree root is missing

在 2026/7/11 14:20, Qu Wenruo 写道:
>
>
> 在 2026/7/11 14:49, Dongjiang Zhu 写道:
>> [BUG]
>> For a filesystem using the block group tree feature, corrupting the 
>> block
>> group tree root and mounting with rescue=ibadroots triggers the 
>> following
>> NULL pointer dereference:
>>
>>    BTRFS warning (device loop0 state E): checksum verify failed on 
>> logical 30654464 mirror 1 wanted 0x0f55b851 found 0xd7efdde7 level 0
>>    BTRFS warning (device loop0 state E): checksum verify failed on 
>> logical 30654464 mirror 2 wanted 0x0f55b851 found 0xd7efdde7 level 0
>>    BUG: kernel NULL pointer dereference, address: 0000000000000100
>>    #PF: supervisor read access in kernel mode
>>    #PF: error_code(0x0000) - not-present page
>>    Oops: Oops: 0000 [#1] SMP NOPTI
>>    RIP: 0010:btrfs_update_global_block_rsv+0x9d/0x1c0 [btrfs]
>
> The problem is why we need to use global block rsv in the first place?
>
> The whole fs is never going to be mounted RW as long as "rescue=" 
> mount option is utilized.
>
> In the case of a rescue mount, we should just fill in a dummy block 
> rsv, with block_rsv->full set.
>
> This will also fix the next bug you're fixing.

That makes sense, thanks.

I had considered simply skipping btrfs_update_global_block_rsv() from
btrfs_init_global_block_rsv() when rescue=ibadroots is set, but did not
choose that approach in the end.

Your suggestion is semantically more precise. If I understand it
correctly, would the implementation look like this?

void btrfs_init_global_block_rsv(struct btrfs_fs_info *fs_info)
{
         struct btrfs_space_info *space_info;

         /* Existing space_info initialization. */

         if (unlikely(BTRFS_FS_ERROR(fs_info))) {
                 fs_info->global_block_rsv.full = true;
                 return;
         }

         btrfs_update_global_block_rsv(fs_info);
}

Would you agree with using BTRFS_FS_ERROR() here, or would you prefer the
narrower fs_is_full_ro() or IGNOREBADROOTS condition?

Thanks,
Dongjiang