Re: [PATCH v4 3/3] btrfs: Do super block verification before writing it to disk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 25, 2018 at 12:43:25PM +0800, Qu Wenruo wrote:
> Reported-by: Ken Swenson <flat@xxxxxxxxxxx>
> Reported-by: Ben Parsons <9parsonsb@xxxxxxxxx>
> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
> ---
>  fs/btrfs/disk-io.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index b981ecc4b6f9..d6c0cee627d9 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2610,6 +2610,41 @@ static int btrfs_validate_mount_super(struct btrfs_fs_info *fs_info)
>  	return __validate_super(fs_info, fs_info->super_copy, 0);
>  }
>  
> +/*
> + * Check the validation of super block at write time.
> + * Some checks like bytenr check will be skipped as their values will be
> + * overwritten soon.
> + * Extra checks like csum type and incompact flags will be executed here.
> + */
> +static int btrfs_validate_write_super(struct btrfs_fs_info *fs_info,
> +				      struct btrfs_super_block *sb)
> +{
> +	int ret;
> +
> +	ret = __validate_super(fs_info, sb, -1);

Please note that the patches have been slightly updated in the committed
version. Updates to patches that come that late after the initial
iterations are more convenient as incremental updates.

> +	if (ret < 0)
> +		goto out;
> +	if (btrfs_super_csum_type(sb) != BTRFS_CSUM_TYPE_CRC32) {
> +		ret = -EUCLEAN;
> +		btrfs_err(fs_info, "invalid csum type, has %u want %u",
> +			  btrfs_super_csum_type(sb), BTRFS_CSUM_TYPE_CRC32);
> +		goto out;
> +	}
> +	if (btrfs_super_incompat_flags(sb) & ~BTRFS_FEATURE_INCOMPAT_SUPP) {
> +		ret = -EUCLEAN;
> +		btrfs_err(fs_info,
> +		"invalid incompact flags, has 0x%llu valid mask 0x%llu",
> +			  btrfs_super_incompat_flags(sb),
> +			  BTRFS_FEATURE_INCOMPAT_SUPP);
> +		goto out;
> +	}
> +out:
> +	if (ret < 0)
> +		btrfs_err(fs_info,
> +		"super block corruption detected before writing it to disk");
> +	return ret;
> +}
> +
>  int open_ctree(struct super_block *sb,
>  	       struct btrfs_fs_devices *fs_devices,
>  	       char *options)
> @@ -3770,6 +3805,14 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
>  		flags = btrfs_super_flags(sb);
>  		btrfs_set_super_flags(sb, flags | BTRFS_HEADER_FLAG_WRITTEN);
>  
> +		ret = btrfs_validate_write_super(fs_info, sb);
> +		if (ret < 0) {
> +			mutex_unlock(&fs_info->fs_devices->device_list_mutex);
> +			btrfs_handle_fs_error(fs_info, -EUCLEAN,
> +				"unexpected superblock corruption detected");
> +			return -EUCLEAN;

The shortcut from here makes sense but we still have to wait for any
previous superblock writes that could have happened (write_dev_supers).

The sequence would be:

at device 1
	check sb
	write

at device 2
	check sb
	failed write

now the bh from device 1 write would have one more reference.

As the change is only in the 3rd patch, please send only that one and
based on the patch in misc-next. Thanks.

> +		}
> +
>  		ret = write_dev_supers(dev, sb, max_mirrors);

(Here the reference gets incremented.)
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux