| Message ID | 20180326082742.9235-8-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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; }
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(-)