[PATCH v2] handle SB beyond blockcount mkfs.btrfs

[PATCH v2] handle SB beyond blockcount mkfs.btrfs

[Anand Jain]

v2->v1:
 progs: update change log
 Adds kernel check if there is a mismatch in the FSID
   between primary and copy SB

The kernel patch here checks if all the SB copies are of the
same fsid.

The btrfs-progs patch zeros SB if found beyond blockcount.
mkfs.btrfs -b <blockcount>

** CAVEAT **
On the systems with only the below kernel patch but without the
btrfs-progs patch below, it shall return the btrfs scan error
as shown in the example below.

mkfs.btrfs -fq /dev/sdc
mkfs.btrfs -fq -b1G /dev/sdc
ERROR: device scan failed on '/dev/sdc': Invalid argument

The above error says, mkfs is fine but failed to register/scan
the device into the kernel. As the kernel now reports an error
if there is inconsistency in the SB fsid. To overcome this error
user can apply the btrfs-progs patch OR use dd it zero the stale
SB manually.

Anand Jain (1):
  btrfs: check if the fsid in the primary sb and copy sb are same

 fs/btrfs/volumes.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

  btrfs-progs: wipe stale sb copy beyond -b size

 utils.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

-- 
2.7.0

[PATCH v2] btrfs: check if the fsid in the primary sb and copy sb are same

[Anand Jain]

During the btrfs dev scan make sure that other copies of superblock
contain the same fsid as the primary SB. So that we bring to the
users notice if the superblock has been overwritten.

 mkfs.btrfs -fq /dev/sdc
 mkfs.btrfs -fq /dev/sdb
 dd if=/dev/sdb of=/dev/sdc count=4K skip=64K seek=64K obs=1 ibs=1
 mount /dev/sdc /btrfs

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c | 43 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 11 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 87d183accdb2..b79e3ffd2047 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1203,24 +1203,45 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 	struct block_device *bdev;
 	struct page *page;
 	int ret = 0;
-	u64 bytenr;
+	int i;
+	u8 fsid_tmp[BTRFS_FSID_SIZE];
 
-	/*
-	 * we would like to check all the supers, but that would make
-	 * a btrfs mount succeed after a mkfs from a different FS.
-	 * So, we need to add a special mount option to scan for
-	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead
-	 */
-	bytenr = btrfs_sb_offset(0);
 	flags |= FMODE_EXCL;
 
 	bdev = blkdev_get_by_path(path, flags, holder);
 	if (IS_ERR(bdev))
 		return PTR_ERR(bdev);
 
-	if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) {
-		ret = -EINVAL;
-		goto error_bdev_put;
+	/*
+	 * We would like to check all the supers and use one good copy,
+	 * but that would make a btrfs mount succeed after a mkfs from
+	 * a different FS.
+	 * So, we need to add a special mount option to scan for
+	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead.
+	 * So, just validate if all copies of the superblocks are ok
+	 * and have the same fsid.
+	 */
+	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)
+				continue;
+			ret = -EINVAL;
+			goto error_bdev_put;
+		}
+		if (i == 0) {
+			memcpy(fsid_tmp, disk_super->fsid, BTRFS_FSID_SIZE);
+			continue;
+		}
+
+		if (memcmp(fsid_tmp, disk_super->fsid, BTRFS_FSID_SIZE)) {
+			pr_err("BTRFS: (device %pg) SB copy#%d fsid %pU "\
+				"doesn't match with the primary SB fsid %pU\n",
+				bdev, i, disk_super->fsid, fsid_tmp);
+			ret = -EINVAL;
+			goto error_bdev_put;
+		}
 	}
 
 	mutex_lock(&uuid_mutex);
-- 
2.7.0

Re: [PATCH v2] btrfs: check if the fsid in the primary sb and copy sb are same

[Nikolay Borisov]

On 22.03.2018 15:01, Anand Jain wrote:
> During the btrfs dev scan make sure that other copies of superblock
> contain the same fsid as the primary SB. So that we bring to the
> users notice if the superblock has been overwritten.
> 
>  mkfs.btrfs -fq /dev/sdc
>  mkfs.btrfs -fq /dev/sdb
>  dd if=/dev/sdb of=/dev/sdc count=4K skip=64K seek=64K obs=1 ibs=1
>  mount /dev/sdc /btrfs
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/volumes.c | 43 ++++++++++++++++++++++++++++++++-----------
>  1 file changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 87d183accdb2..b79e3ffd2047 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1203,24 +1203,45 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
>  	struct block_device *bdev;
>  	struct page *page;
>  	int ret = 0;
> -	u64 bytenr;
> +	int i;
> +	u8 fsid_tmp[BTRFS_FSID_SIZE];
>  
> -	/*
> -	 * we would like to check all the supers, but that would make
> -	 * a btrfs mount succeed after a mkfs from a different FS.
> -	 * So, we need to add a special mount option to scan for
> -	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead
> -	 */
> -	bytenr = btrfs_sb_offset(0);
>  	flags |= FMODE_EXCL;
>  
>  	bdev = blkdev_get_by_path(path, flags, holder);
>  	if (IS_ERR(bdev))
>  		return PTR_ERR(bdev);
>  
> -	if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) {
> -		ret = -EINVAL;
> -		goto error_bdev_put;
> +	/*
> +	 * We would like to check all the supers and use one good copy,
> +	 * but that would make a btrfs mount succeed after a mkfs from
> +	 * a different FS.
> +	 * So, we need to add a special mount option to scan for
> +	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead.
> +	 * So, just validate if all copies of the superblocks are ok
> +	 * and have the same fsid.
> +	 */
> +	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)
> +				continue;
> +			ret = -EINVAL;
> +			goto error_bdev_put;
> +		}
> +		if (i == 0) {
> +			memcpy(fsid_tmp, disk_super->fsid, BTRFS_FSID_SIZE);
> +			continue;
> +		}
> +
> +		if (memcmp(fsid_tmp, disk_super->fsid, BTRFS_FSID_SIZE)) {
> +			pr_err("BTRFS: (device %pg) SB copy#%d fsid %pU "\
> +				"doesn't match with the primary SB fsid %pU\n",
> +				bdev, i, disk_super->fsid, fsid_tmp);
> +			ret = -EINVAL;
> +			goto error_bdev_put;
> +		}

So the code per-se makes sense. However, have you came across such a
case in the real world or are you just fixing a theoretical (but real)
issue?

>  	}
>  
>  	mutex_lock(&uuid_mutex);
> 

Re: [PATCH v2] btrfs: check if the fsid in the primary sb and copy sb are same

[Anand Jain]

> So the code per-se makes sense. However, have you came across such a
> case in the real world or are you just fixing a theoretical (but real)
> issue?

As discussed in the other thread, the SB csum check is coming to scan
context as well. Now the question I have is, what do we do if the csum
doesn't match, in the inital version we can fail the mount, but in the
long run potentially we could recover it from the good copy.
So if we have to recover it from the good copy we have to ensure copies
are really copies of the primary SB.

Thanks, Anand

Re: [PATCH v2] btrfs: check if the fsid in the primary sb and copy sb are same

[Anand Jain]

  Pls, ignore this. I need to handle the mapped page in a better way.

Thanks, Anand

On 03/22/2018 09:01 PM, Anand Jain wrote:
> During the btrfs dev scan make sure that other copies of superblock
> contain the same fsid as the primary SB. So that we bring to the
> users notice if the superblock has been overwritten.
> 
>   mkfs.btrfs -fq /dev/sdc
>   mkfs.btrfs -fq /dev/sdb
>   dd if=/dev/sdb of=/dev/sdc count=4K skip=64K seek=64K obs=1 ibs=1
>   mount /dev/sdc /btrfs
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>   fs/btrfs/volumes.c | 43 ++++++++++++++++++++++++++++++++-----------
>   1 file changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 87d183accdb2..b79e3ffd2047 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1203,24 +1203,45 @@ int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
>   	struct block_device *bdev;
>   	struct page *page;
>   	int ret = 0;
> -	u64 bytenr;
> +	int i;
> +	u8 fsid_tmp[BTRFS_FSID_SIZE];
>   
> -	/*
> -	 * we would like to check all the supers, but that would make
> -	 * a btrfs mount succeed after a mkfs from a different FS.
> -	 * So, we need to add a special mount option to scan for
> -	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead
> -	 */
> -	bytenr = btrfs_sb_offset(0);
>   	flags |= FMODE_EXCL;
>   
>   	bdev = blkdev_get_by_path(path, flags, holder);
>   	if (IS_ERR(bdev))
>   		return PTR_ERR(bdev);
>   
> -	if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) {
> -		ret = -EINVAL;
> -		goto error_bdev_put;
> +	/*
> +	 * We would like to check all the supers and use one good copy,
> +	 * but that would make a btrfs mount succeed after a mkfs from
> +	 * a different FS.
> +	 * So, we need to add a special mount option to scan for
> +	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead.
> +	 * So, just validate if all copies of the superblocks are ok
> +	 * and have the same fsid.
> +	 */
> +	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)
> +				continue;
> +			ret = -EINVAL;
> +			goto error_bdev_put;
> +		}
> +		if (i == 0) {
> +			memcpy(fsid_tmp, disk_super->fsid, BTRFS_FSID_SIZE);
> +			continue;
> +		}
> +
> +		if (memcmp(fsid_tmp, disk_super->fsid, BTRFS_FSID_SIZE)) {
> +			pr_err("BTRFS: (device %pg) SB copy#%d fsid %pU "\
> +				"doesn't match with the primary SB fsid %pU\n",
> +				bdev, i, disk_super->fsid, fsid_tmp);
> +			ret = -EINVAL;
> +			goto error_bdev_put;
> +		}
>   	}
>   
>   	mutex_lock(&uuid_mutex);
> 

[PATCH v2.1] btrfs: check if the fsid in the primary sb and copy sb are same

[Anand Jain]

During the btrfs dev scan make sure that other copies of superblock
contain the same fsid as the primary SB. So that we bring to the
user notice if the superblock has been overwritten.

 mkfs.btrfs -fq /dev/sdc
 mkfs.btrfs -fq /dev/sdb
 dd if=/dev/sdb of=/dev/sdc count=4K skip=64K seek=64K obs=1 ibs=1
 mount /dev/sdc /btrfs

Also reading all copies of the SB is required to verify their csum
is correct, so this serves as a preparatory as well.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2->v2.1:
 Fix the page release part.
 pr_err string changes.
 add else part for if(btrfs_read_disk_super(..)).
v1:
 Not born yet.

 fs/btrfs/volumes.c | 57 +++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 44 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 87d183accdb2..3e2dae4e8ccb 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1198,41 +1198,72 @@ static int btrfs_read_disk_super(struct block_device *bdev, u64 bytenr,
 int btrfs_scan_one_device(const char *path, fmode_t flags, void *holder,
 			  struct btrfs_fs_devices **fs_devices_ret)
 {
+	struct btrfs_super_block *disk_super_primary;
 	struct btrfs_super_block *disk_super;
 	struct btrfs_device *device;
 	struct block_device *bdev;
 	struct page *page;
 	int ret = 0;
-	u64 bytenr;
+	int i;
 
-	/*
-	 * we would like to check all the supers, but that would make
-	 * a btrfs mount succeed after a mkfs from a different FS.
-	 * So, we need to add a special mount option to scan for
-	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead
-	 */
-	bytenr = btrfs_sb_offset(0);
 	flags |= FMODE_EXCL;
 
 	bdev = blkdev_get_by_path(path, flags, holder);
 	if (IS_ERR(bdev))
 		return PTR_ERR(bdev);
 
-	if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super)) {
-		ret = -EINVAL;
+	/*
+	 * We would like to check all the supers and use one good copy,
+	 * but that would make a btrfs mount succeed after a mkfs from
+	 * a different FS.
+	 * So, we need to add a special mount option to scan for
+	 * later supers, using BTRFS_SUPER_MIRROR_MAX instead.
+	 * So, just validate if all copies of the superblocks are ok
+	 * and have the same fsid.
+	 */
+	disk_super_primary = kzalloc(sizeof(*disk_super_primary), GFP_KERNEL);
+	if (!disk_super_primary) {
+		ret = -ENOMEM;
 		goto error_bdev_put;
 	}
+	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;
+				goto error_kfree;
+			}
+			continue;
+		} else if (i == 0) {
+			memcpy(disk_super_primary, disk_super,
+			       sizeof(*disk_super_primary));
+			btrfs_release_disk_super(page);
+			continue;
+		}
+
+		if (memcmp(disk_super_primary->fsid, disk_super->fsid,
+			   BTRFS_FSID_SIZE)) {
+			pr_err("BTRFS (device %pg): superblock primary:copy#%d fsid missmatch %pU:%pU",
+				bdev, i, disk_super->fsid,
+				disk_super_primary->fsid);
+			ret = -EINVAL;
+			btrfs_release_disk_super(page);
+			goto error_kfree;
+		}
+		btrfs_release_disk_super(page);
+	}
 
 	mutex_lock(&uuid_mutex);
-	device = device_list_add(path, disk_super);
+	device = device_list_add(path, disk_super_primary);
 	if (IS_ERR(device))
 		ret = PTR_ERR(device);
 	else
 		*fs_devices_ret = device->fs_devices;
 	mutex_unlock(&uuid_mutex);
 
-	btrfs_release_disk_super(page);
-
+error_kfree:
+	kfree(disk_super_primary);
 error_bdev_put:
 	blkdev_put(bdev, flags);
 
-- 
2.15.0

Re: [PATCH v2] handle SB beyond blockcount mkfs.btrfs

[Anand Jain]

  Pls ignore this set as I am including these patches
  along with other related changes.

Thanks, Anand


On 03/22/2018 09:01 PM, Anand Jain wrote:
> v2->v1:
>   progs: update change log
>   Adds kernel check if there is a mismatch in the FSID
>     between primary and copy SB
> 
> The kernel patch here checks if all the SB copies are of the
> same fsid.
> 
> The btrfs-progs patch zeros SB if found beyond blockcount.
> mkfs.btrfs -b <blockcount>
> 
> ** CAVEAT **
> On the systems with only the below kernel patch but without the
> btrfs-progs patch below, it shall return the btrfs scan error
> as shown in the example below.
> 
> mkfs.btrfs -fq /dev/sdc
> mkfs.btrfs -fq -b1G /dev/sdc
> ERROR: device scan failed on '/dev/sdc': Invalid argument
> 
> The above error says, mkfs is fine but failed to register/scan
> the device into the kernel. As the kernel now reports an error
> if there is inconsistency in the SB fsid. To overcome this error
> user can apply the btrfs-progs patch OR use dd it zero the stale
> SB manually.
> 
> Anand Jain (1):
>    btrfs: check if the fsid in the primary sb and copy sb are same
> 
>   fs/btrfs/volumes.c | 43 ++++++++++++++++++++++++++++++++-----------
>   1 file changed, 32 insertions(+), 11 deletions(-)
> 
>    btrfs-progs: wipe stale sb copy beyond -b size
> 
>   utils.c | 35 +++++++++++++++++++++++++++++++++++
>   1 file changed, 35 insertions(+)
>