[PATCH 0/2] verify superblock checksum

[PATCH 0/2] verify superblock checksum

[Anand Jain]

Here are the threads/context [1] in which we read the superblock(s).
And this patchset will make sure the superblock csum are checked
when they are read in the respective context as show below [1].

This patchset is on top of
  [PATCH v2.1] btrfs: check if the fsid in the primary sb and copy sb are same
which is on top of
  [PATCH 0/3] Preparatory to add the csum check in the scan context

[1]

A. dev delete/replace
(its would have been better if we had just used uuid from the userland,
reading the superblock does not make sense to me, but for legacy purpose
we have to continue to support its ioctl args.).

btrfs_rm_device() || btrfs_dev_replace_by_ioctl()
|_btrfs_find_device_by_devspec()
  |_btrfs_find_device_missing_or_by_path()
    |_btrfs_find_device_by_path()
      |_btrfs_get_bdev_and_sb()
        |_btrfs_read_dev_super()
          |_btrfs_read_dev_one_super()
            |___bread()

B. mount

btrfs_mount_root()
 |
 |_btrfs_parse_early_options (-o device only)
 | |_btrfs_scan_one_device
 |   |_btrfs_read_disk_super()
 |     |_read_cache_page_gfp()
 |
 |_btrfs_scan_one_device(mount-arg-dev only)
 | |_btrfs_read_disk_super()
 |   |_read_cache_page_gfp()
 |
 |
 |_btrfs_open_devices(fsid:all)
 |  |_btrfs_open_one_device()
 |    |_btrfs_get_bdev_and_sb()  <--- invalidate_bdev(fsid:all)
 |      |_btrfs_read_dev_super()
 |        |_btrfs_read_dev_one_super()
 |          |___bread()
 |
 |_btrfs_fill_super()
   |_btrfs_open_ctree()   <-- invalidate_bdev(latest_bdev) <-- redundant
     |_btrfs_read_dev_super(latest_bdev only)
     | |_btrfs_read_dev_one_super(latest_bdev only)
     |   |___bread(latest_bdev)
     |
     |_btrfs_check_super_csum(latest_bdev only) [*]
     |
     |_btrfs_read_chunk_tree
       |_read_one_dev()
         |_open_seed_devices()
           |_btrfs_open_devices(fs_devices->seed only)

C. scan/ready

scan/ready
|_btrfs_scan_one_device(ioctl-arg-dev only)
   |_btrfs_read_disk_super()
     |_read_cache_page_gfp()


Anand Jain (2):
  btrfs: verify checksum when superblock is read for mount
  btrfs: verify checksum when superblock is read for scan

 fs/btrfs/disk-io.c | 35 ++++++++++++++++++-----------------
 fs/btrfs/disk-io.h |  1 +
 fs/btrfs/volumes.c | 32 +++++++++++++++++++++++---------
 3 files changed, 42 insertions(+), 26 deletions(-)

-- 
2.15.0

[PATCH 1/2] btrfs: verify checksum when superblock is read for mount

[Anand Jain]

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 | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 062b3e0c7fd1..c4600d5eca9c 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2564,22 +2564,6 @@ int open_ctree(struct super_block *sb,
 		goto fail_alloc;
 	}
 
-	/*
-	 * 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 if (err == -EUCLEAN)
-			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
@@ -3126,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))
@@ -3146,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;
 }
-- 
2.15.0

[PATCH 2/2] btrfs: verify checksum when superblock is read for scan

[Anand Jain]

During the scan context, we aren't verifying if the superblock-
checksum is correct for the primary and its copies.
This patch fixes it by adding the checksum verification function
btrfs_check_super_csum() in the function btrfs_read_disk_super().

It would fail the scan only if the primary SB csum is wrong,
whereas if the copy-SB csum is wrong it would just warn and
still makes the scan successful. Since because the mount reads
only primary SB. And further, all SB gets overwritten. Thus
fixing the bad SB.

The context in which this will be called are, scan, ready,
for the device in the mount argument, for the devices in the
mount -o device option.

Test script:
mkfs.btrfs -fq /dev/sdc
dd if=/dev/urandom of=/dev/sdc ibs=1 obs=1 count=1 seek=64K
btrfs dev scan

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c |  2 +-
 fs/btrfs/disk-io.h |  1 +
 fs/btrfs/volumes.c | 32 +++++++++++++++++++++++---------
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c4600d5eca9c..3cc50041c0b9 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 3e2dae4e8ccb..e15d19d04722 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1149,38 +1149,53 @@ 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;
 
 	/* make sure our super fits in the device */
 	if (bytenr + PAGE_SIZE >= i_size_read(bdev->bd_inode))
-		return 1;
+		return -EINVAL;
 
 	/* make sure our super fits in the page */
 	if (sizeof(**disk_super) > PAGE_SIZE)
-		return 1;
+		return -EINVAL;
 
 	/* make sure our super doesn't straddle pages on disk */
 	index = bytenr >> PAGE_SHIFT;
 	if ((bytenr + sizeof(**disk_super) - 1) >> PAGE_SHIFT != index)
-		return 1;
+		return -EINVAL;
 
 	/* pull in the page with our super */
 	*page = read_cache_page_gfp(bdev->bd_inode->i_mapping,
 				   index, GFP_KERNEL);
 
 	if (IS_ERR_OR_NULL(*page))
-		return 1;
+		return -EINVAL;
 
 	p = kmap(*page);
 
 	/* align our pointer to the offset of the super block */
 	*disk_super = p + (bytenr & ~PAGE_MASK);
 
+	err = btrfs_check_super_csum((char *) *disk_super);
+	if (err) {
+		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 (btrfs_super_bytenr(*disk_super) != bytenr ||
 	    btrfs_super_magic(*disk_super) != BTRFS_MAGIC) {
 		btrfs_release_disk_super(*page);
-		return 1;
+		return -EINVAL;
 	}
 
 	if ((*disk_super)->label[0] &&
@@ -1229,11 +1244,10 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		u64 bytenr = btrfs_sb_offset(i);
 
-		if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) {
-			if (i == 0) {
-				ret = -EINVAL;
+		ret = btrfs_read_disk_super(bdev, bytenr, &page, &disk_super);
+		if (ret) {
+			if (i == 0)
 				goto error_kfree;
-			}
 			continue;
 		} else if (i == 0) {
 			memcpy(disk_super_primary, disk_super,
-- 
2.15.0

Re: [PATCH 0/2] verify superblock checksum

[Anand Jain]

  Pls ignore this set as I am including these patches
  in the bigger set.

Thanks, Anand

On 03/23/2018 08:53 PM, Anand Jain wrote:
> Here are the threads/context [1] in which we read the superblock(s).
> And this patchset will make sure the superblock csum are checked
> when they are read in the respective context as show below [1].
> 
> This patchset is on top of
>    [PATCH v2.1] btrfs: check if the fsid in the primary sb and copy sb are same
> which is on top of
>    [PATCH 0/3] Preparatory to add the csum check in the scan context
> 
> [1]
> 
> A. dev delete/replace
> (its would have been better if we had just used uuid from the userland,
> reading the superblock does not make sense to me, but for legacy purpose
> we have to continue to support its ioctl args.).
> 
> btrfs_rm_device() || btrfs_dev_replace_by_ioctl()
> |_btrfs_find_device_by_devspec()
>    |_btrfs_find_device_missing_or_by_path()
>      |_btrfs_find_device_by_path()
>        |_btrfs_get_bdev_and_sb()
>          |_btrfs_read_dev_super()
>            |_btrfs_read_dev_one_super()
>              |___bread()
> 
> B. mount
> 
> btrfs_mount_root()
>   |
>   |_btrfs_parse_early_options (-o device only)
>   | |_btrfs_scan_one_device
>   |   |_btrfs_read_disk_super()
>   |     |_read_cache_page_gfp()
>   |
>   |_btrfs_scan_one_device(mount-arg-dev only)
>   | |_btrfs_read_disk_super()
>   |   |_read_cache_page_gfp()
>   |
>   |
>   |_btrfs_open_devices(fsid:all)
>   |  |_btrfs_open_one_device()
>   |    |_btrfs_get_bdev_and_sb()  <--- invalidate_bdev(fsid:all)
>   |      |_btrfs_read_dev_super()
>   |        |_btrfs_read_dev_one_super()
>   |          |___bread()
>   |
>   |_btrfs_fill_super()
>     |_btrfs_open_ctree()   <-- invalidate_bdev(latest_bdev) <-- redundant
>       |_btrfs_read_dev_super(latest_bdev only)
>       | |_btrfs_read_dev_one_super(latest_bdev only)
>       |   |___bread(latest_bdev)
>       |
>       |_btrfs_check_super_csum(latest_bdev only) [*]
>       |
>       |_btrfs_read_chunk_tree
>         |_read_one_dev()
>           |_open_seed_devices()
>             |_btrfs_open_devices(fs_devices->seed only)
> 
> C. scan/ready
> 
> scan/ready
> |_btrfs_scan_one_device(ioctl-arg-dev only)
>     |_btrfs_read_disk_super()
>       |_read_cache_page_gfp()
> 
> 
> Anand Jain (2):
>    btrfs: verify checksum when superblock is read for mount
>    btrfs: verify checksum when superblock is read for scan
> 
>   fs/btrfs/disk-io.c | 35 ++++++++++++++++++-----------------
>   fs/btrfs/disk-io.h |  1 +
>   fs/btrfs/volumes.c | 32 +++++++++++++++++++++++---------
>   3 files changed, 42 insertions(+), 26 deletions(-)
>