[7/8] btrfs: verify checksum for all devices in mount context
diff mbox

Message ID 20180326082742.9235-8-anand.jain@oracle.com
State New
Headers show

Commit Message

Anand Jain March 26, 2018, 8:27 a.m. UTC
During mount context, we aren't verifying the superblock checksum
for all the devices, instead, we verify it only for the
struct btrfs_fs_device::latest_bdev. This patch fixes it
by moving the checksum verification code from the function open_ctree()
into the function btrfs_read_dev_one_super().

By doing this now we are verifying the superblock checksum
in the mount-context, device-replace and, device-delete context.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

Comments

Nikolay Borisov March 27, 2018, 12:21 p.m. UTC | #1
On 26.03.2018 11:27, Anand Jain wrote:
> During mount context, we aren't verifying the superblock checksum
> for all the devices, instead, we verify it only for the
> struct btrfs_fs_device::latest_bdev. This patch fixes it
> by moving the checksum verification code from the function open_ctree()
> into the function btrfs_read_dev_one_super().
> 
> By doing this now we are verifying the superblock checksum
> in the mount-context, device-replace and, device-delete context.

Which call chain provides the device-replace and device-delete contexts?
I think it is worth it documenting them in the changelog.

> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  fs/btrfs/disk-io.c | 35 +++++++++++++++++------------------
>  1 file changed, 17 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 6299ab18da5f..3cc50041c0b9 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2565,24 +2565,6 @@ int open_ctree(struct super_block *sb,
>  	}
>  
>  	/*
> -	 * We want to check superblock checksum, the type is stored inside.
> -	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
> -	 */
> -	err = btrfs_check_super_csum(bh->b_data);
> -	if (err) {
> -		if (err == -EINVAL)
> -			pr_err("BTRFS error (device %pg): "\
> -				"unsupported checksum algorithm",
> -				fs_devices->latest_bdev);
> -		else
> -			pr_err("BTRFS error (device %pg): "\
> -				"superblock checksum mismatch",
> -				fs_devices->latest_bdev);
> -		brelse(bh);
> -		goto fail_alloc;
> -	}
> -
> -	/*
>  	 * super_copy is zeroed at allocation time and we never touch the
>  	 * following bytes up to INFO_SIZE, the checksum is calculated from
>  	 * the whole block of INFO_SIZE
> @@ -3128,6 +3110,7 @@ int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
>  	struct buffer_head *bh;
>  	struct btrfs_super_block *super;
>  	u64 bytenr;
> +	int err;
>  
>  	bytenr = btrfs_sb_offset(copy_num);
>  	if (bytenr + BTRFS_SUPER_INFO_SIZE >= i_size_read(bdev->bd_inode))
> @@ -3148,6 +3131,22 @@ int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
>  		return -EINVAL;
>  	}
>  
> +	/*
> +	 * Check the superblock checksum, the type is stored inside.
> +	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
> +	 */
> +	err = btrfs_check_super_csum(bh->b_data);
> +	if (err) {
> +		if (err == -EINVAL)
> +			pr_err("BTRFS error (device %pg): unsupported checksum algorithm",
> +				bdev);
> +		else if (err == -EUCLEAN)
> +			pr_err("BTRFS error (device %pg): superblock checksum mismatch",
> +				bdev);
> +		brelse(bh);
> +		return err;
> +	}
> +
>  	*bh_ret = bh;
>  	return 0;
>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Anand Jain March 27, 2018, 10:48 p.m. UTC | #2
On 03/27/2018 08:21 PM, Nikolay Borisov wrote:
> 
> 
> On 26.03.2018 11:27, Anand Jain wrote:
>> During mount context, we aren't verifying the superblock checksum
>> for all the devices, instead, we verify it only for the
>> struct btrfs_fs_device::latest_bdev. This patch fixes it
>> by moving the checksum verification code from the function open_ctree()
>> into the function btrfs_read_dev_one_super().
>>
>> By doing this now we are verifying the superblock checksum
>> in the mount-context, device-replace and, device-delete context.
> 
> Which call chain provides the device-replace and device-delete contexts?
> I think it is worth it documenting them in the changelog.

  Will copy it here from the cover-letter.

>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> 
>> ---
>>   fs/btrfs/disk-io.c | 35 +++++++++++++++++------------------
>>   1 file changed, 17 insertions(+), 18 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 6299ab18da5f..3cc50041c0b9 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2565,24 +2565,6 @@ int open_ctree(struct super_block *sb,
>>   	}
>>   
>>   	/*
>> -	 * We want to check superblock checksum, the type is stored inside.
>> -	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
>> -	 */
>> -	err = btrfs_check_super_csum(bh->b_data);
>> -	if (err) {
>> -		if (err == -EINVAL)
>> -			pr_err("BTRFS error (device %pg): "\
>> -				"unsupported checksum algorithm",
>> -				fs_devices->latest_bdev);
>> -		else
>> -			pr_err("BTRFS error (device %pg): "\
>> -				"superblock checksum mismatch",
>> -				fs_devices->latest_bdev);
>> -		brelse(bh);
>> -		goto fail_alloc;
>> -	}
>> -
>> -	/*
>>   	 * super_copy is zeroed at allocation time and we never touch the
>>   	 * following bytes up to INFO_SIZE, the checksum is calculated from
>>   	 * the whole block of INFO_SIZE
>> @@ -3128,6 +3110,7 @@ int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
>>   	struct buffer_head *bh;
>>   	struct btrfs_super_block *super;
>>   	u64 bytenr;
>> +	int err;
>>   
>>   	bytenr = btrfs_sb_offset(copy_num);
>>   	if (bytenr + BTRFS_SUPER_INFO_SIZE >= i_size_read(bdev->bd_inode))
>> @@ -3148,6 +3131,22 @@ int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
>>   		return -EINVAL;
>>   	}
>>   
>> +	/*
>> +	 * Check the superblock checksum, the type is stored inside.
>> +	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
>> +	 */
>> +	err = btrfs_check_super_csum(bh->b_data);
>> +	if (err) {

I am fixing this to if (err < 0) {

>> +		if (err == -EINVAL)
>> +			pr_err("BTRFS error (device %pg): unsupported checksum algorithm",
>> +				bdev);
>> +		else if (err == -EUCLEAN)
>> +			pr_err("BTRFS error (device %pg): superblock checksum mismatch",
>> +				bdev);

  I am also dropping else if to else in v2.

Thanks, Anand


>> +		brelse(bh);
>> +		return err;
>> +	}
>> +
>>   	*bh_ret = bh;
>>   	return 0;
>>   }
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 6299ab18da5f..3cc50041c0b9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2565,24 +2565,6 @@  int open_ctree(struct super_block *sb,
 	}
 
 	/*
-	 * We want to check superblock checksum, the type is stored inside.
-	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
-	 */
-	err = btrfs_check_super_csum(bh->b_data);
-	if (err) {
-		if (err == -EINVAL)
-			pr_err("BTRFS error (device %pg): "\
-				"unsupported checksum algorithm",
-				fs_devices->latest_bdev);
-		else
-			pr_err("BTRFS error (device %pg): "\
-				"superblock checksum mismatch",
-				fs_devices->latest_bdev);
-		brelse(bh);
-		goto fail_alloc;
-	}
-
-	/*
 	 * super_copy is zeroed at allocation time and we never touch the
 	 * following bytes up to INFO_SIZE, the checksum is calculated from
 	 * the whole block of INFO_SIZE
@@ -3128,6 +3110,7 @@  int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
 	struct buffer_head *bh;
 	struct btrfs_super_block *super;
 	u64 bytenr;
+	int err;
 
 	bytenr = btrfs_sb_offset(copy_num);
 	if (bytenr + BTRFS_SUPER_INFO_SIZE >= i_size_read(bdev->bd_inode))
@@ -3148,6 +3131,22 @@  int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num,
 		return -EINVAL;
 	}
 
+	/*
+	 * Check the superblock checksum, the type is stored inside.
+	 * Pass the whole disk block of size BTRFS_SUPER_INFO_SIZE (4k).
+	 */
+	err = btrfs_check_super_csum(bh->b_data);
+	if (err) {
+		if (err == -EINVAL)
+			pr_err("BTRFS error (device %pg): unsupported checksum algorithm",
+				bdev);
+		else if (err == -EUCLEAN)
+			pr_err("BTRFS error (device %pg): superblock checksum mismatch",
+				bdev);
+		brelse(bh);
+		return err;
+	}
+
 	*bh_ret = bh;
 	return 0;
 }