From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de ([195.135.220.15]:41560 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751211AbeC2M5e (ORCPT ); Thu, 29 Mar 2018 08:57:34 -0400 Subject: Re: [PATCH v3 5/7] btrfs: verify superblock checksum during scan To: Anand Jain , linux-btrfs@vger.kernel.org References: <20180329103705.13566-1-anand.jain@oracle.com> <20180329103705.13566-6-anand.jain@oracle.com> From: Nikolay Borisov Message-ID: <9e0cf43b-44e6-02fc-532b-a602b413f41b@suse.com> Date: Thu, 29 Mar 2018 15:57:31 +0300 MIME-Version: 1.0 In-Reply-To: <20180329103705.13566-6-anand.jain@oracle.com> Content-Type: text/plain; charset=utf-8 Sender: linux-btrfs-owner@vger.kernel.org List-ID: On 29.03.2018 13:37, Anand Jain wrote: > During the scan context, we aren't verifying the superblock- > checksum when read. > This patch fixes it by adding the checksum verification function > btrfs_check_super_csum() in the function btrfs_read_disk_super(). > And makes device scan to error fail if the primary superblock csum > is wrong, whereas if the copy-superblock csum is wrong it will just > just report mismatch and continue mount/scan as usual. When the > mount is successful We anyway overwrite all superblocks upon unmount. > > The context in which this will be called is - device scan, device ready, > and mount -o device option. > > Test script: > > Corrupt primary superblock and check if device scan and mount > fails: > mkfs.btrfs -fq /dev/sdc > dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K > btrfs dev scan /dev/sdc > mount /dev/sdc /btrfs > > Corrupt secondary superblock and check if device scan and mount > is succcessful, check for the dmesg for errors. > mkfs.btrfs -fq /dev/sdc > dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=67108864 > btrfs dev scan /dev/sdc > mount /dev/sdc /btrfs > > Signed-off-by: Anand Jain > v3->v2: > Also squash 4/8 in here. > 4/8: btrfs: make btrfs_check_super_csum() non-static > v1->v2: > changed title. > use explicit (< 0) check for %errr. > Un-split pr_err() string. > Fix typo in the git commit log. > Move the csum check after bytenr and btrfs magic verified. > > btrfs: make btrfs_check_super_csum() non-static > > In preparation to add the superblock csum verification for the > scan context, make btrfs_check_super_csum() non-static. > > Signed-off-by: Anand Jain > Reviewed-by: Nikolay Borisov This patch's changelog is garbaged. Duplicated signed off by , changelog above scissors etc. Please review your patches carefully before sending upstream. > --- > fs/btrfs/disk-io.c | 2 +- > fs/btrfs/disk-io.h | 1 + > fs/btrfs/volumes.c | 13 +++++++++++++ > 3 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 4c573602480c..35dbbdc613cd 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -395,7 +395,7 @@ static int verify_parent_transid(struct extent_io_tree *io_tree, > * Otherwise: -EINVAL if csum type is not found > * -EUCLEAN if csum does not match > */ > -static int btrfs_check_super_csum(char *raw_disk_sb) > +int btrfs_check_super_csum(char *raw_disk_sb) > { > struct btrfs_super_block *disk_sb = > (struct btrfs_super_block *)raw_disk_sb; > diff --git a/fs/btrfs/disk-io.h b/fs/btrfs/disk-io.h > index 70a88d61b547..c400cc68f913 100644 > --- a/fs/btrfs/disk-io.h > +++ b/fs/btrfs/disk-io.h > @@ -69,6 +69,7 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors); > struct buffer_head *btrfs_read_dev_super(struct block_device *bdev); > int btrfs_read_dev_one_super(struct block_device *bdev, int copy_num, > struct buffer_head **bh_ret); > +int btrfs_check_super_csum(char *raw_disk_sb); > int btrfs_commit_super(struct btrfs_fs_info *fs_info); > struct btrfs_root *btrfs_read_fs_root(struct btrfs_root *tree_root, > struct btrfs_key *location); > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 035affa447fa..f1f074fbfee1 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1149,6 +1149,7 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr, > struct page **page, > struct btrfs_super_block **disk_super) > { > + int err; > void *p; > pgoff_t index; > > @@ -1183,6 +1184,18 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr, > return -EINVAL; > } > > + err = btrfs_check_super_csum((char *) *disk_super); > + if (err < 0) { > + if (err == -EINVAL) > + pr_err("BTRFS error (device %pg): unsupported checksum type, bytenr=%llu", > + bdev, bytenr); > + else > + pr_err("BTRFS error (device %pg): superblock checksum failed, bytenr=%llu", > + bdev, bytenr); > + btrfs_release_disk_super(*page); > + return err; > + } > + > if ((*disk_super)->label[0] && > (*disk_super)->label[BTRFS_LABEL_SIZE - 1]) > (*disk_super)->label[BTRFS_LABEL_SIZE - 1] = '\0'; >