Re: [PATCH] btrfs: don't clobber ret in btrfs_validate_super()

[Mark Harmstone <maharmstone@meta.com>]

On 11/3/25 16:54, Filipe Manana wrote:
> > 
> On Tue, Mar 11, 2025 at 4:39 PM Mark Harmstone <maharmstone@fb.com> wrote:
>>
>> Commit 2a9bb78cfd36 introduces a call to validate_sys_chunk_array() in
>> btrfs_validate_super(), which clobbers the value of ret set earlier.
>> This has the effect of negating the validity checks done earlier, making
>> it so btrfs could potentially try to mount invalid filesystems.
>>
>> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
>> Cc: Qu Wenruo <wqu@suse.com>
>> Fixes: 2a9bb78cfd36 ("btrfs: validate system chunk array at btrfs_validate_super()")
>> ---
>>   fs/btrfs/disk-io.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 0afd3c0f2fab..4421c946a53c 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2562,6 +2562,9 @@ int btrfs_validate_super(const struct btrfs_fs_info *fs_info,
>>                  ret = -EINVAL;
>>          }
>>
>> +       if (ret)
>> +               return ret;
>> +
>>          ret = validate_sys_chunk_array(fs_info, sb);
> 
> While this fixes the problem, the function is structured in a way that
> is easy to get into this sort of issue.
> Rather than set 'ret' to -EINVAL every time some check fails and then
> continue, I'd rather have it return -EINVAL immediately.
> 
> Anyway, this fixes a bug, so:
> 
> Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks. Yes, I did think that, but if there's multiple reasons why your 
FS won't mount you probably want to know all of them.

> 
> Thanks.
> 
>>
>>          /*
>> --
>> 2.45.3
>>
>>