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

[Qu Wenruo <wqu@suse.com>]

在 2025/3/12 03:09, Mark Harmstone 写道:
> 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()")

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks for catching this.

Although I agree with Filipe that it's better to return the error 
immediately in the future.

Yes, currently it will report all the errors, but if the first check 
against magic number fails, the super block can just be garbage and all 
the remaining checks will be triggered.

Other than a single "no valid fs found" message, we got all kinds of 
errors, which is not that easy for end users to understand.

Thanks,
Qu

> ---
>   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);
>   
>   	/*