Re: [PATCH 2/3] btrfs: Add csum type check for btrfs_check_super_valid()

David Sterba
2018-04-19T10:12:04Z

On Thu, Apr 19, 2018 at 05:38:15PM +0800, Qu Wenruo wrote:
> Just like incompat flags check, although we have already done super csum
> type check before calling btrfs_check_super_valid(), we can still add
> such check for later write time check.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/disk-io.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index ec123158f051..b189ec086bc2 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -3981,6 +3981,15 @@ static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
> u64 sectorsize = btrfs_super_sectorsize(sb);
> int ret = 0;
>
> + /*
> + * For write time check, as for mount time we have checked csum before
> + * calling btrfs_check_super_valid(), so it must be a corruption
> + */
> + if (btrfs_super_csum_type(sb) >= ARRAY_SIZE(btrfs_csum_sizes)) {
> + btrfs_err(fs_info, "corrupted csum type %u",
> + btrfs_super_csum_type(sb));
> + ret = -EINVAL;
> + }
> if (btrfs_super_magic(sb) != BTRFS_MAGIC) {

The test for magic should be IMO the first one, that's how the
filesystem type is identified before anything else. Granted that the
magic must be correct before it can be even mounted, but the code should
follow that logic too. Otherwise ok.