thr3ads.net - Btrfs devel - [PATCH 0/5] Btrfs-progs: coalesce of patches [Jun 2013]

If this information is useful, please help other people find it:
Share via:

Filipe David Borba Manana

2013-Jun-10 19:51 UTC

[PATCH 0/5] Btrfs-progs: coalesce of patches

Some fixes and minor enhancements.

Filipe David Borba Manana (5):
  Btrfs-progs: fix closing of devices
  Btrfs-progs: Add missing free_extent_buffer() call to debug-tree
  Btrfs-progs: Add missing close_ctree() calls to debug-tree
  Btrfs-progs: pretty print dir_item type
  Btrfs-progs: Validate super block checksum

 btrfs-debug-tree.c |   10 ++++---
 disk-io.c          |   80 +++++++++++++++++++++++++++++++++++++++++-----------
 print-tree.c       |   40 +++++++++++++++++++++++++-
 volumes.c          |    4 +--
 4 files changed, 111 insertions(+), 23 deletions(-)

-- 
1.7.9.5

--
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

Filipe David Borba Manana

2013-Jun-10 19:51 UTC

head link

[PATCH 1/5] Btrfs-progs: fix closing of devices

If a device could not be opened in volumes.c:read_one_dev(), a
btrfs_device instance was allocated and added to the list of
devices of the fs - however this device instance had its fd,
name and label fields not initialized. This is problematic in
disk-io.c:close_all_devices() as it tries to close the (invalid)
fd of the device and kfree() its name and label, which point
to random memory locations.

  Thread 1 (Thread 0x7f0a3d2d1740 (LWP 23585)):
  #0  __GI___libc_free (mem=0xa5a5a5a5a5a5a5a5) at malloc.c:2970
  #1  0x000000000042054b in close_all_devices (fs_info=0x1e92bf0) at
disk-io.c:1276
  #2  0x0000000000421dcd in close_ctree (root=<optimized out>) at
disk-io.c:1336
  #3  0x0000000000418cfa in cmd_check (argc=<optimized out>,
argv=<optimized out>) at cmds-check.c:4171
  #4  0x0000000000403ed4 in main (argc=2, argv=0x7fff9a583d28) at btrfs.c:295

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 disk-io.c |    4 ++--
 volumes.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/disk-io.c b/disk-io.c
index 21b410d..bd9cf4e 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1267,12 +1267,12 @@ static int close_all_devices(struct btrfs_fs_info
*fs_info)
 	while (!list_empty(list)) {
 		device = list_entry(list->next, struct btrfs_device, dev_list);
 		list_del_init(&device->dev_list);
-		if (device->fd) {
+		if (device->fd >= 0) {
 			fsync(device->fd);
 			if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED))
 				fprintf(stderr, "Warning, could not drop caches\n");
+			close(device->fd);
 		}
-		close(device->fd);
 		kfree(device->name);
 		kfree(device->label);
 		kfree(device);
diff --git a/volumes.c b/volumes.c
index d6f81f8..a84ded7 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1628,10 +1628,10 @@ static int read_one_dev(struct btrfs_root *root,
 	if (!device) {
 		printk("warning devid %llu not found already\n",
 			(unsigned long long)devid);
-		device = kmalloc(sizeof(*device), GFP_NOFS);
+		device = kzalloc(sizeof(*device), GFP_NOFS);
 		if (!device)
 			return -ENOMEM;
-		device->total_ios = 0;
+		device->fd = -1;
 		list_add(&device->dev_list,
 			 &root->fs_info->fs_devices->devices);
 	}
-- 
1.7.9.5

--
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

Filipe David Borba Manana

2013-Jun-10 19:51 UTC

head link

[PATCH 2/5] Btrfs-progs: Add missing free_extent_buffer() call to debug-tree

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 btrfs-debug-tree.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c
index bae7f94..97459b1 100644
--- a/btrfs-debug-tree.c
+++ b/btrfs-debug-tree.c
@@ -355,6 +355,7 @@ again:
 					btrfs_print_tree(tree_root_scan, buf, 1);
 				}
 			}
+			free_extent_buffer(buf);
 		}
 next:
 		path.slots[0]++;
-- 
1.7.9.5

--
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

Filipe David Borba Manana

2013-Jun-10 19:51 UTC

head link

[PATCH 3/5] Btrfs-progs: Add missing close_ctree() calls to debug-tree

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 btrfs-debug-tree.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c
index 97459b1..74d4d66 100644
--- a/btrfs-debug-tree.c
+++ b/btrfs-debug-tree.c
@@ -195,10 +195,10 @@ int main(int ac, char **av)
 		if (!leaf) {
 			fprintf(stderr, "failed to read %llu\n",
 				(unsigned long long)block_only);
-			return 0;
+			goto close_root;
 		}
 		btrfs_print_tree(root, leaf, 0);
-		return 0;
+		goto close_root;
 	}
 
 	if (!extent_only) {
@@ -370,7 +370,7 @@ no_node:
 	}
 
 	if (extent_only || device_only)
-		return 0;
+		goto close_root;
 
 	if (root_backups)
 		print_old_roots(info->super_copy);
@@ -383,5 +383,6 @@ no_node:
 	uuid_unparse(info->super_copy->fsid, uuidbuf);
 	printf("uuid %s\n", uuidbuf);
 	printf("%s\n", BTRFS_BUILD_VERSION);
-	return 0;
+close_root:
+	return close_ctree(root);
 }
-- 
1.7.9.5

--
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

Filipe David Borba Manana

2013-Jun-10 19:51 UTC

head link

[PATCH 4/5] Btrfs-progs: pretty print dir_item type

Instead of printing an integer, print a symbolic name which
is more human friendly. Particularly useful when using the
program btrfs-debug-tree.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 print-tree.c |   40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/print-tree.c b/print-tree.c
index aae47a9..4401a1a 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -25,6 +25,42 @@
 #include "disk-io.h"
 #include "print-tree.h"
 
+
+static void print_dir_item_type(struct extent_buffer *eb,
+                                struct btrfs_dir_item *di)
+{
+	u8 type = btrfs_dir_type(eb, di);
+
+	switch (type) {
+	case BTRFS_FT_REG_FILE:
+		printf("FILE");
+		break;
+	case BTRFS_FT_DIR:
+		printf("DIR");
+		break;
+	case BTRFS_FT_CHRDEV:
+		printf("CHRDEV");
+		break;
+	case BTRFS_FT_BLKDEV:
+		printf("BLKDEV");
+		break;
+	case BTRFS_FT_FIFO:
+		printf("FIFO");
+		break;
+	case BTRFS_FT_SOCK:
+		printf("SOCK");
+		break;
+	case BTRFS_FT_SYMLINK:
+		printf("SYMLINK");
+		break;
+	case BTRFS_FT_XATTR:
+		printf("XATTR");
+		break;
+	default:
+		printf("%u", type);
+	}
+}
+
 static int print_dir_item(struct extent_buffer *eb, struct btrfs_item *item,
 			  struct btrfs_dir_item *di)
 {
@@ -41,7 +77,9 @@ static int print_dir_item(struct extent_buffer *eb, struct
btrfs_item *item,
 		btrfs_dir_item_key(eb, di, &location);
 		printf("\t\tlocation ");
 		btrfs_print_key(&location);
-		printf(" type %u\n", btrfs_dir_type(eb, di));
+		printf(" type ");
+		print_dir_item_type(eb, di);
+		printf("\n");
 		name_len = btrfs_dir_name_len(eb, di);
 		data_len = btrfs_dir_data_len(eb, di);
 		len = (name_len <= sizeof(namebuf))? name_len: sizeof(namebuf);
-- 
1.7.9.5

--
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

Filipe David Borba Manana

2013-Jun-10 19:51 UTC

head link

[PATCH 5/5] Btrfs-progs: Validate super block checksum

After finding a super block in a device also validate its
checksum. This validation is done in the kernel but it was
missing in btrfs-progs.

The function btrfs_check_super_csum() is imported from the
file fs/btrfs/disk-io.c in the kernel source tree.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 disk-io.c |   76 +++++++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 62 insertions(+), 14 deletions(-)

diff --git a/disk-io.c b/disk-io.c
index bd9cf4e..edd4d52 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1085,47 +1085,95 @@ struct btrfs_root *open_ctree_fd(int fp, const char
*path, u64 sb_bytenr,
 	return info->fs_root;
 }
 
+static int btrfs_check_super_csum(char *raw_disk_sb)
+{
+	struct btrfs_super_block *disk_sb +		(struct btrfs_super_block *)raw_disk_sb;
+	u16 csum_type = btrfs_super_csum_type(disk_sb);
+	int ret = 0;
+
+	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
+		u32 crc = ~(u32)0;
+		const int csum_size = sizeof(crc);
+		char result[csum_size];
+
+		/*
+		 * The super_block structure does not span the whole
+		 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
+		 * is filled with zeros and is included in the checkum.
+		 */
+		crc = btrfs_csum_data(NULL, raw_disk_sb + BTRFS_CSUM_SIZE,
+				crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+		btrfs_csum_final(crc, result);
+
+		if (memcmp(raw_disk_sb, result, csum_size))
+			ret = 1;
+
+		if (ret && btrfs_super_generation(disk_sb) < 10) {
+			fprintf(stderr, "btrfs: super block crcs don''t match, "
+				"older mkfs detected\n");
+			ret = 0;
+		}
+	}
+
+	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
+		fprintf(stderr, "btrfs: unsupported checksum algorithm %u\n",
+			csum_type);
+		ret = 1;
+	}
+
+	return ret;
+}
+
 int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr)
 {
 	u8 fsid[BTRFS_FSID_SIZE];
 	int fsid_is_initialized = 0;
-	struct btrfs_super_block buf;
+	char buf[BTRFS_SUPER_INFO_SIZE];
+	struct btrfs_super_block *tmp_sb;
 	int i;
 	int ret;
 	u64 transid = 0;
 	u64 bytenr;
 
 	if (sb_bytenr != BTRFS_SUPER_INFO_OFFSET) {
-		ret = pread64(fd, &buf, sizeof(buf), sb_bytenr);
+		ret = pread64(fd, buf, sizeof(buf), sb_bytenr);
 		if (ret < sizeof(buf))
 			return -1;
 
-		if (btrfs_super_bytenr(&buf) != sb_bytenr ||
-		    buf.magic != cpu_to_le64(BTRFS_MAGIC))
+		tmp_sb = (struct btrfs_super_block *)buf;
+
+		if (btrfs_super_bytenr(tmp_sb) != sb_bytenr ||
+		    tmp_sb->magic != cpu_to_le64(BTRFS_MAGIC) ||
+		    btrfs_check_super_csum(buf))
 			return -1;
 
-		memcpy(sb, &buf, sizeof(*sb));
+		memcpy(sb, buf, sizeof(*sb));
 		return 0;
 	}
 
 	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
-		ret = pread64(fd, &buf, sizeof(buf), bytenr);
+		ret = pread64(fd, buf, sizeof(buf), bytenr);
 		if (ret < sizeof(buf))
 			break;
 
-		if (btrfs_super_bytenr(&buf) != bytenr )
+		tmp_sb = (struct btrfs_super_block *)buf;
+
+		if (btrfs_super_bytenr(tmp_sb) != bytenr )
 			continue;
 		/* if magic is NULL, the device was removed */
-		if (buf.magic == 0 && i == 0) 
+		if (tmp_sb->magic == 0 && i == 0)
 			return -1;
-		if (buf.magic != cpu_to_le64(BTRFS_MAGIC))
+		if (tmp_sb->magic != cpu_to_le64(BTRFS_MAGIC))
+			continue;
+		if (btrfs_check_super_csum(buf))
 			continue;
 
 		if (!fsid_is_initialized) {
-			memcpy(fsid, buf.fsid, sizeof(fsid));
+			memcpy(fsid, tmp_sb->fsid, sizeof(fsid));
 			fsid_is_initialized = 1;
-		} else if (memcmp(fsid, buf.fsid, sizeof(fsid))) {
+		} else if (memcmp(fsid, tmp_sb->fsid, sizeof(fsid))) {
 			/*
 			 * the superblocks (the original one and
 			 * its backups) contain data of different
@@ -1134,9 +1182,9 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block
*sb, u64 sb_bytenr)
 			continue;
 		}
 
-		if (btrfs_super_generation(&buf) > transid) {
-			memcpy(sb, &buf, sizeof(*sb));
-			transid = btrfs_super_generation(&buf);
+		if (btrfs_super_generation(tmp_sb) > transid) {
+			memcpy(sb, buf, sizeof(*sb));
+			transid = btrfs_super_generation(tmp_sb);
 		}
 	}
 
-- 
1.7.9.5

--
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

Filipe David Borba Manana

2013-Jun-10 20:07 UTC

head link

[PATCH 1/5] Btrfs-progs: fix closing of devices

If a device could not be opened in volumes.c:read_one_dev(), a
btrfs_device instance was allocated and added to the list of
devices of the fs - however this device instance had its fd,
name and label fields not initialized. This is problematic in
disk-io.c:close_all_devices() as it tries to close the (invalid)
fd of the device and kfree() its name and label, which point
to random memory locations.

  Thread 1 (Thread 0x7f0a3d2d1740 (LWP 23585)):
  #0  __GI___libc_free (mem=0xa5a5a5a5a5a5a5a5) at malloc.c:2970
  #1  0x000000000042054b in close_all_devices (fs_info=0x1e92bf0) at
disk-io.c:1276
  #2  0x0000000000421dcd in close_ctree (root=<optimized out>) at
disk-io.c:1336
  #3  0x0000000000418cfa in cmd_check (argc=<optimized out>,
argv=<optimized out>) at cmds-check.c:4171
  #4  0x0000000000403ed4 in main (argc=2, argv=0x7fff9a583d28) at btrfs.c:295

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 disk-io.c |    4 ++--
 volumes.c |    3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/disk-io.c b/disk-io.c
index 21b410d..353e0aa 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1267,12 +1267,12 @@ static int close_all_devices(struct btrfs_fs_info
*fs_info)
 	while (!list_empty(list)) {
 		device = list_entry(list->next, struct btrfs_device, dev_list);
 		list_del_init(&device->dev_list);
-		if (device->fd) {
+		if (device->fd > 0) {
 			fsync(device->fd);
 			if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED))
 				fprintf(stderr, "Warning, could not drop caches\n");
+			close(device->fd);
 		}
-		close(device->fd);
 		kfree(device->name);
 		kfree(device->label);
 		kfree(device);
diff --git a/volumes.c b/volumes.c
index d6f81f8..3f69c37 100644
--- a/volumes.c
+++ b/volumes.c
@@ -1628,10 +1628,9 @@ static int read_one_dev(struct btrfs_root *root,
 	if (!device) {
 		printk("warning devid %llu not found already\n",
 			(unsigned long long)devid);
-		device = kmalloc(sizeof(*device), GFP_NOFS);
+		device = kzalloc(sizeof(*device), GFP_NOFS);
 		if (!device)
 			return -ENOMEM;
-		device->total_ios = 0;
 		list_add(&device->dev_list,
 			 &root->fs_info->fs_devices->devices);
 	}
-- 
1.7.9.5

--
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

Filipe David Borba Manana

2013-Jun-10 23:52 UTC

head link

[PATCH 1/5 v2] Btrfs-progs: fix closing of devices

If a device could not be opened in volumes.c:read_one_dev(), a
btrfs_device instance was allocated and added to the list of
devices of the fs - however this device instance had its fd,
name and label fields not initialized. This is problematic in
disk-io.c:close_all_devices() as it tried to sync, fadvise and
close the (invalid) fd of the device, and kfree() its name and
label, which pointed to random memory locations.

  Thread 1 (Thread 0x7f0a3d2d1740 (LWP 23585)):
  #0  __GI___libc_free (mem=0xa5a5a5a5a5a5a5a5) at malloc.c:2970
  #1  0x000000000042054b in close_all_devices (fs_info=0x1e92bf0) at
disk-io.c:1276
  #2  0x0000000000421dcd in close_ctree (root=<optimized out>) at
disk-io.c:1336
  #3  0x0000000000418cfa in cmd_check (argc=<optimized out>,
argv=<optimized out>) at cmds-check.c:4171
  #4  0x0000000000403ed4 in main (argc=2, argv=0x7fff9a583d28) at btrfs.c:295

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 disk-io.c |    4 ++--
 volumes.c |    5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/disk-io.c b/disk-io.c
index 21b410d..bd9cf4e 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1267,12 +1267,12 @@ static int close_all_devices(struct btrfs_fs_info
*fs_info)
 	while (!list_empty(list)) {
 		device = list_entry(list->next, struct btrfs_device, dev_list);
 		list_del_init(&device->dev_list);
-		if (device->fd) {
+		if (device->fd >= 0) {
 			fsync(device->fd);
 			if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED))
 				fprintf(stderr, "Warning, could not drop caches\n");
+			close(device->fd);
 		}
-		close(device->fd);
 		kfree(device->name);
 		kfree(device->label);
 		kfree(device);
diff --git a/volumes.c b/volumes.c
index d6f81f8..061f094 100644
--- a/volumes.c
+++ b/volumes.c
@@ -116,6 +116,7 @@ static int device_list_add(const char *path,
 			/* we can safely leave the fs_devices entry around */
 			return -ENOMEM;
 		}
+		device->fd = -1;
 		device->devid = devid;
 		memcpy(device->uuid, disk_super->dev_item.uuid,
 		       BTRFS_UUID_SIZE);
@@ -1628,10 +1629,10 @@ static int read_one_dev(struct btrfs_root *root,
 	if (!device) {
 		printk("warning devid %llu not found already\n",
 			(unsigned long long)devid);
-		device = kmalloc(sizeof(*device), GFP_NOFS);
+		device = kzalloc(sizeof(*device), GFP_NOFS);
 		if (!device)
 			return -ENOMEM;
-		device->total_ios = 0;
+		device->fd = -1;
 		list_add(&device->dev_list,
 			 &root->fs_info->fs_devices->devices);
 	}
-- 
1.7.9.5

--
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

David Sterba

2013-Jun-11 15:00 UTC

head link

Re: [PATCH 1/5] Btrfs-progs: fix closing of devices

On Mon, Jun 10, 2013 at 09:07:55PM +0100, Filipe David Borba Manana
wrote:> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1267,12 +1267,12 @@ static int close_all_devices(struct btrfs_fs_info
*fs_info)
>  	while (!list_empty(list)) {
>  		device = list_entry(list->next, struct btrfs_device, dev_list);
>  		list_del_init(&device->dev_list);
> -		if (device->fd) {
> +		if (device->fd > 0) {
> -		device = kmalloc(sizeof(*device), GFP_NOFS);
> +		device = kzalloc(sizeof(*device), GFP_NOFS);
>  		if (!device)
>  			return -ENOMEM;
> -		device->total_ios = 0;
Can you please describe the incremental change between the two patches?
Why do you exclude fd == 0 ?

david
--
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

Filipe Manana

2013-Jun-11 16:09 UTC

head link

Re: [PATCH 1/5] Btrfs-progs: fix closing of devices

On 11/06/2013, at 16:00, David Sterba <dsterba@suse.cz> wrote:
> On Mon, Jun 10, 2013 at 09:07:55PM +0100, Filipe David Borba Manana wrote:
>> --- a/disk-io.c
>> +++ b/disk-io.c
>> @@ -1267,12 +1267,12 @@ static int close_all_devices(struct
btrfs_fs_info *fs_info)
>>    while (!list_empty(list)) {
>>        device = list_entry(list->next, struct btrfs_device,
dev_list);
>>        list_del_init(&device->dev_list);
>> -        if (device->fd) {
>> +        if (device->fd > 0) {
> 
>> -        device = kmalloc(sizeof(*device), GFP_NOFS);
>> +        device = kzalloc(sizeof(*device), GFP_NOFS);
>>        if (!device)
>>            return -ENOMEM;
>> -        device->total_ios = 0;
> 
> Can you please describe the incremental change between the two patches?
> Why do you exclude fd == 0 ?
Sorry, I messed up my git send-email foo.

I excluded 0 because the original code did it too, however it was logically
incorrect. The last patch version (v2, 3rd email) does not exclude 0 and
ensures that -1 is used everywhere as a marker for invalid fd.

(Hopefully now I figured how to use git send-email to update a patch
correctly)

Thanks

> 
> david--
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

Filipe David Manana

2013-Jun-20 17:08 UTC

head link

Re: [PATCH 5/5] Btrfs-progs: Validate super block checksum

Ping.

Is there any reason why the btrfs progs (except for btrfs-show-super)
don''t validate the super block''s checksum?

thanks

On Mon, Jun 10, 2013 at 8:51 PM, Filipe David Borba Manana
<fdmanana@gmail.com> wrote:> After finding a super block in a device also validate its
> checksum. This validation is done in the kernel but it was
> missing in btrfs-progs.
>
> The function btrfs_check_super_csum() is imported from the
> file fs/btrfs/disk-io.c in the kernel source tree.
>
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
>  disk-io.c |   76
+++++++++++++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 62 insertions(+), 14 deletions(-)
>
> diff --git a/disk-io.c b/disk-io.c
> index bd9cf4e..edd4d52 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1085,47 +1085,95 @@ struct btrfs_root *open_ctree_fd(int fp, const char
*path, u64 sb_bytenr,
>         return info->fs_root;
>  }
>
> +static int btrfs_check_super_csum(char *raw_disk_sb)
> +{
> +       struct btrfs_super_block *disk_sb > +               (struct
btrfs_super_block *)raw_disk_sb;
> +       u16 csum_type = btrfs_super_csum_type(disk_sb);
> +       int ret = 0;
> +
> +       if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
> +               u32 crc = ~(u32)0;
> +               const int csum_size = sizeof(crc);
> +               char result[csum_size];
> +
> +               /*
> +                * The super_block structure does not span the whole
> +                * BTRFS_SUPER_INFO_SIZE range, we expect that the unused
space
> +                * is filled with zeros and is included in the checkum.
> +                */
> +               crc = btrfs_csum_data(NULL, raw_disk_sb + BTRFS_CSUM_SIZE,
> +                               crc, BTRFS_SUPER_INFO_SIZE -
BTRFS_CSUM_SIZE);
> +               btrfs_csum_final(crc, result);
> +
> +               if (memcmp(raw_disk_sb, result, csum_size))
> +                       ret = 1;
> +
> +               if (ret && btrfs_super_generation(disk_sb) < 10)
{
> +                       fprintf(stderr, "btrfs: super block crcs
don''t match, "
> +                               "older mkfs detected\n");
> +                       ret = 0;
> +               }
> +       }
> +
> +       if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
> +               fprintf(stderr, "btrfs: unsupported checksum algorithm
%u\n",
> +                       csum_type);
> +               ret = 1;
> +       }
> +
> +       return ret;
> +}
> +
>  int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64
sb_bytenr)
>  {
>         u8 fsid[BTRFS_FSID_SIZE];
>         int fsid_is_initialized = 0;
> -       struct btrfs_super_block buf;
> +       char buf[BTRFS_SUPER_INFO_SIZE];
> +       struct btrfs_super_block *tmp_sb;
>         int i;
>         int ret;
>         u64 transid = 0;
>         u64 bytenr;
>
>         if (sb_bytenr != BTRFS_SUPER_INFO_OFFSET) {
> -               ret = pread64(fd, &buf, sizeof(buf), sb_bytenr);
> +               ret = pread64(fd, buf, sizeof(buf), sb_bytenr);
>                 if (ret < sizeof(buf))
>                         return -1;
>
> -               if (btrfs_super_bytenr(&buf) != sb_bytenr ||
> -                   buf.magic != cpu_to_le64(BTRFS_MAGIC))
> +               tmp_sb = (struct btrfs_super_block *)buf;
> +
> +               if (btrfs_super_bytenr(tmp_sb) != sb_bytenr ||
> +                   tmp_sb->magic != cpu_to_le64(BTRFS_MAGIC) ||
> +                   btrfs_check_super_csum(buf))
>                         return -1;
>
> -               memcpy(sb, &buf, sizeof(*sb));
> +               memcpy(sb, buf, sizeof(*sb));
>                 return 0;
>         }
>
>         for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
>                 bytenr = btrfs_sb_offset(i);
> -               ret = pread64(fd, &buf, sizeof(buf), bytenr);
> +               ret = pread64(fd, buf, sizeof(buf), bytenr);
>                 if (ret < sizeof(buf))
>                         break;
>
> -               if (btrfs_super_bytenr(&buf) != bytenr )
> +               tmp_sb = (struct btrfs_super_block *)buf;
> +
> +               if (btrfs_super_bytenr(tmp_sb) != bytenr )
>                         continue;
>                 /* if magic is NULL, the device was removed */
> -               if (buf.magic == 0 && i == 0)
> +               if (tmp_sb->magic == 0 && i == 0)
>                         return -1;
> -               if (buf.magic != cpu_to_le64(BTRFS_MAGIC))
> +               if (tmp_sb->magic != cpu_to_le64(BTRFS_MAGIC))
> +                       continue;
> +               if (btrfs_check_super_csum(buf))
>                         continue;
>
>                 if (!fsid_is_initialized) {
> -                       memcpy(fsid, buf.fsid, sizeof(fsid));
> +                       memcpy(fsid, tmp_sb->fsid, sizeof(fsid));
>                         fsid_is_initialized = 1;
> -               } else if (memcmp(fsid, buf.fsid, sizeof(fsid))) {
> +               } else if (memcmp(fsid, tmp_sb->fsid, sizeof(fsid))) {
>                         /*
>                          * the superblocks (the original one and
>                          * its backups) contain data of different
> @@ -1134,9 +1182,9 @@ int btrfs_read_dev_super(int fd, struct
btrfs_super_block *sb, u64 sb_bytenr)
>                         continue;
>                 }
>
> -               if (btrfs_super_generation(&buf) > transid) {
> -                       memcpy(sb, &buf, sizeof(*sb));
> -                       transid = btrfs_super_generation(&buf);
> +               if (btrfs_super_generation(tmp_sb) > transid) {
> +                       memcpy(sb, buf, sizeof(*sb));
> +                       transid = btrfs_super_generation(tmp_sb);
>                 }
>         }
>
> --
> 1.7.9.5
>


-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That''s why all progress depends on unreasonable men."
--
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

Liu Bo

2013-Jun-21 16:03 UTC

head link

Re: [PATCH 1/5 v2] Btrfs-progs: fix closing of devices

On Tue, Jun 11, 2013 at 12:52:36AM +0100, Filipe David Borba Manana
wrote:> If a device could not be opened in volumes.c:read_one_dev(), a
> btrfs_device instance was allocated and added to the list of
> devices of the fs - however this device instance had its fd,
> name and label fields not initialized. This is problematic in
> disk-io.c:close_all_devices() as it tried to sync, fadvise and
> close the (invalid) fd of the device, and kfree() its name and
> label, which pointed to random memory locations.
> 
>   Thread 1 (Thread 0x7f0a3d2d1740 (LWP 23585)):
>   #0  __GI___libc_free (mem=0xa5a5a5a5a5a5a5a5) at malloc.c:2970
>   #1  0x000000000042054b in close_all_devices (fs_info=0x1e92bf0) at
disk-io.c:1276
>   #2  0x0000000000421dcd in close_ctree (root=<optimized out>) at
disk-io.c:1336
>   #3  0x0000000000418cfa in cmd_check (argc=<optimized out>,
argv=<optimized out>) at cmds-check.c:4171
>   #4  0x0000000000403ed4 in main (argc=2, argv=0x7fff9a583d28) at
btrfs.c:295
Good catch!  This addresses one of my problems.

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>

thanks,
liubo
> 
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
>  disk-io.c |    4 ++--
>  volumes.c |    5 +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/disk-io.c b/disk-io.c
> index 21b410d..bd9cf4e 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1267,12 +1267,12 @@ static int close_all_devices(struct btrfs_fs_info
*fs_info)
>  	while (!list_empty(list)) {
>  		device = list_entry(list->next, struct btrfs_device, dev_list);
>  		list_del_init(&device->dev_list);
> -		if (device->fd) {
> +		if (device->fd >= 0) {
>  			fsync(device->fd);
>  			if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED))
>  				fprintf(stderr, "Warning, could not drop caches\n");
> +			close(device->fd);
>  		}
> -		close(device->fd);
>  		kfree(device->name);
>  		kfree(device->label);
>  		kfree(device);
> diff --git a/volumes.c b/volumes.c
> index d6f81f8..061f094 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -116,6 +116,7 @@ static int device_list_add(const char *path,
>  			/* we can safely leave the fs_devices entry around */
>  			return -ENOMEM;
>  		}
> +		device->fd = -1;
>  		device->devid = devid;
>  		memcpy(device->uuid, disk_super->dev_item.uuid,
>  		       BTRFS_UUID_SIZE);
> @@ -1628,10 +1629,10 @@ static int read_one_dev(struct btrfs_root *root,
>  	if (!device) {
>  		printk("warning devid %llu not found already\n",
>  			(unsigned long long)devid);
> -		device = kmalloc(sizeof(*device), GFP_NOFS);
> +		device = kzalloc(sizeof(*device), GFP_NOFS);
>  		if (!device)
>  			return -ENOMEM;
> -		device->total_ios = 0;
> +		device->fd = -1;
>  		list_add(&device->dev_list,
>  			 &root->fs_info->fs_devices->devices);
>  	}
> -- 
> 1.7.9.5
> 
> --
> 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

Liu Bo

2013-Jun-21 16:11 UTC

head link

Re: [PATCH 1/5 v2] Btrfs-progs: fix closing of devices

On Tue, Jun 11, 2013 at 12:52:36AM +0100, Filipe David Borba Manana
wrote:> If a device could not be opened in volumes.c:read_one_dev(), a
> btrfs_device instance was allocated and added to the list of
> devices of the fs - however this device instance had its fd,
> name and label fields not initialized. This is problematic in
> disk-io.c:close_all_devices() as it tried to sync, fadvise and
> close the (invalid) fd of the device, and kfree() its name and
> label, which pointed to random memory locations.
> 
>   Thread 1 (Thread 0x7f0a3d2d1740 (LWP 23585)):
>   #0  __GI___libc_free (mem=0xa5a5a5a5a5a5a5a5) at malloc.c:2970
>   #1  0x000000000042054b in close_all_devices (fs_info=0x1e92bf0) at
disk-io.c:1276
>   #2  0x0000000000421dcd in close_ctree (root=<optimized out>) at
disk-io.c:1336
>   #3  0x0000000000418cfa in cmd_check (argc=<optimized out>,
argv=<optimized out>) at cmds-check.c:4171
>   #4  0x0000000000403ed4 in main (argc=2, argv=0x7fff9a583d28) at
btrfs.c:295
> 
> Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
> ---
>  disk-io.c |    4 ++--
>  volumes.c |    5 +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/disk-io.c b/disk-io.c
> index 21b410d..bd9cf4e 100644
> --- a/disk-io.c
> +++ b/disk-io.c
> @@ -1267,12 +1267,12 @@ static int close_all_devices(struct btrfs_fs_info
*fs_info)
>  	while (!list_empty(list)) {
>  		device = list_entry(list->next, struct btrfs_device, dev_list);
>  		list_del_init(&device->dev_list);
> -		if (device->fd) {
> +		if (device->fd >= 0) {
>  			fsync(device->fd);
>  			if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED))
>  				fprintf(stderr, "Warning, could not drop caches\n");
> +			close(device->fd);
>  		}
> -		close(device->fd);
>  		kfree(device->name);
>  		kfree(device->label);
>  		kfree(device);
> diff --git a/volumes.c b/volumes.c
> index d6f81f8..061f094 100644
> --- a/volumes.c
> +++ b/volumes.c
> @@ -116,6 +116,7 @@ static int device_list_add(const char *path,
>  			/* we can safely leave the fs_devices entry around */
>  			return -ENOMEM;
>  		}
> +		device->fd = -1;
One nit here,
I think it''s ok without this since if we get the right device->name,
it
means this device is not missing and its fd will be valid.

thanks,
liubo
>  		device->devid = devid;
>  		memcpy(device->uuid, disk_super->dev_item.uuid,
>  		       BTRFS_UUID_SIZE);
> @@ -1628,10 +1629,10 @@ static int read_one_dev(struct btrfs_root *root,
>  	if (!device) {
>  		printk("warning devid %llu not found already\n",
>  			(unsigned long long)devid);
> -		device = kmalloc(sizeof(*device), GFP_NOFS);
> +		device = kzalloc(sizeof(*device), GFP_NOFS);
>  		if (!device)
>  			return -ENOMEM;
> -		device->total_ios = 0;
> +		device->fd = -1;
>  		list_add(&device->dev_list,
>  			 &root->fs_info->fs_devices->devices);
>  	}
> -- 
> 1.7.9.5
> 
> --
> 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

David Sterba

2013-Jun-21 16:18 UTC

head link

Re: [PATCH 5/5] Btrfs-progs: Validate super block checksum

On Thu, Jun 20, 2013 at 06:08:29PM +0100, Filipe David Manana
wrote:> Is there any reason why the btrfs progs (except for btrfs-show-super)
> don''t validate the super block''s checksum?
btrfsck accepts broken filesystems, so open_ctree_broken should possibly
detect but skip the checksum mismatch.

david
--
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

Filipe David Manana

2013-Jun-21 16:38 UTC

head link

Re: [PATCH 5/5] Btrfs-progs: Validate super block checksum

On Fri, Jun 21, 2013 at 5:18 PM, David Sterba <dsterba@suse.cz>
wrote:> On Thu, Jun 20, 2013 at 06:08:29PM +0100, Filipe David Manana wrote:
>> Is there any reason why the btrfs progs (except for btrfs-show-super)
>> don''t validate the super block''s checksum?
>
> btrfsck accepts broken filesystems, so open_ctree_broken should possibly
> detect but skip the checksum mismatch.
So, when running btrfsck just report if there''s a super block checksum
mismatch (like the patch does) but use it (don''t skip it). For any
other tools in btrfs-progs just report the mismatch and skip to the
next super block (like what the patch does now) ?

thanks
>
> david


--
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That''s why all progress depends on unreasonable men."
--
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

Filipe David Manana

2013-Jun-21 16:46 UTC

head link

Re: [PATCH 1/5 v2] Btrfs-progs: fix closing of devices

On Fri, Jun 21, 2013 at 5:11 PM, Liu Bo <bo.li.liu@oracle.com>
wrote:>> --- a/volumes.c
>> +++ b/volumes.c
>> @@ -116,6 +116,7 @@ static int device_list_add(const char *path,
>>                       /* we can safely leave the fs_devices entry
around */
>>                       return -ENOMEM;
>>               }
>> +             device->fd = -1;
>
> One nit here,
> I think it''s ok without this since if we get the right
device->name, it
> means this device is not missing and its fd will be valid.
Good point.
Only reason I left it like that, is because besides being more clear
(subjective) there seems to be possible to have device.name as
non-null but no valid fd - i.e. the open() call in
volumes.c:btrfs_open_devices() failed.

thanks

>
> thanks,
> liubo--
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

Filipe David Borba Manana

2013-Jun-26 16:41 UTC

head link

[PATCH v2 1/5] Btrfs-progs: fix closing of devices

If a device could not be opened in volumes.c:read_one_dev(), a
btrfs_device instance was allocated and added to the list of
devices of the fs - however this device instance had its fd,
name and label fields not initialized. This is problematic in
disk-io.c:close_all_devices() as it tried to sync, fadvise and
close the (invalid) fd of the device, and kfree() its name and
label, which pointed to random memory locations.

  Thread 1 (Thread 0x7f0a3d2d1740 (LWP 23585)):
  #0  __GI___libc_free (mem=0xa5a5a5a5a5a5a5a5) at malloc.c:2970
  #1  0x000000000042054b in close_all_devices (fs_info=0x1e92bf0) at
disk-io.c:1276
  #2  0x0000000000421dcd in close_ctree (root=<optimized out>) at
disk-io.c:1336
  #3  0x0000000000418cfa in cmd_check (argc=<optimized out>,
argv=<optimized out>) at cmds-check.c:4171
  #4  0x0000000000403ed4 in main (argc=2, argv=0x7fff9a583d28) at btrfs.c:295

v2: Added Liu Bo''s review mention.

Reviewed-by: Liu Bo <bo.li.liu@oracle.com>
Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 disk-io.c |    4 ++--
 volumes.c |    5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/disk-io.c b/disk-io.c
index 9ffe6e4..768adda 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -1270,12 +1270,12 @@ static int close_all_devices(struct btrfs_fs_info
*fs_info)
 	while (!list_empty(list)) {
 		device = list_entry(list->next, struct btrfs_device, dev_list);
 		list_del_init(&device->dev_list);
-		if (device->fd) {
+		if (device->fd >= 0) {
 			fsync(device->fd);
 			if (posix_fadvise(device->fd, 0, 0, POSIX_FADV_DONTNEED))
 				fprintf(stderr, "Warning, could not drop caches\n");
+			close(device->fd);
 		}
-		close(device->fd);
 		kfree(device->name);
 		kfree(device->label);
 		kfree(device);
diff --git a/volumes.c b/volumes.c
index d6f81f8..061f094 100644
--- a/volumes.c
+++ b/volumes.c
@@ -116,6 +116,7 @@ static int device_list_add(const char *path,
 			/* we can safely leave the fs_devices entry around */
 			return -ENOMEM;
 		}
+		device->fd = -1;
 		device->devid = devid;
 		memcpy(device->uuid, disk_super->dev_item.uuid,
 		       BTRFS_UUID_SIZE);
@@ -1628,10 +1629,10 @@ static int read_one_dev(struct btrfs_root *root,
 	if (!device) {
 		printk("warning devid %llu not found already\n",
 			(unsigned long long)devid);
-		device = kmalloc(sizeof(*device), GFP_NOFS);
+		device = kzalloc(sizeof(*device), GFP_NOFS);
 		if (!device)
 			return -ENOMEM;
-		device->total_ios = 0;
+		device->fd = -1;
 		list_add(&device->dev_list,
 			 &root->fs_info->fs_devices->devices);
 	}
-- 
1.7.9.5

--
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

Filipe David Borba Manana

2013-Jun-26 16:42 UTC

head link

[PATCH v2 5/5] Btrfs-progs: Validate super block checksum

After finding a super block in a device also validate its
checksum. This validation is done in the kernel but it was
missing in btrfs-progs.

The function btrfs_check_super_csum() is imported from the
file fs/btrfs/disk-io.c in the kernel source tree.

v2:

When finding the super block for a device ignore checksum
validation failures if we''re running the btrfsck tool, the
restore or check commands of the btrfs tool, btrfs-debug-tree
or btrfs-find-root tools. For all cases, print a warning
message whenever the checksum validation fails, mentioning
the superblock offset and device path.

Signed-off-by: Filipe David Borba Manana <fdmanana@gmail.com>
---
 btrfs-debug-tree.c |    2 +-
 btrfs-find-root.c  |    8 ++--
 cmds-check.c       |    2 +-
 cmds-device.c      |    2 +-
 cmds-filesystem.c  |    2 +-
 cmds-replace.c     |    2 +-
 cmds-restore.c     |    2 +-
 disk-io.c          |  103 ++++++++++++++++++++++++++++++++++++++++------------
 disk-io.h          |    7 +++-
 utils.c            |   15 ++++----
 utils.h            |    4 +-
 volumes.c          |    6 ++-
 volumes.h          |    3 +-
 13 files changed, 110 insertions(+), 48 deletions(-)

diff --git a/btrfs-debug-tree.c b/btrfs-debug-tree.c
index 74d4d66..6fa9e83 100644
--- a/btrfs-debug-tree.c
+++ b/btrfs-debug-tree.c
@@ -166,7 +166,7 @@ int main(int ac, char **av)
 	if (ac != 1)
 		print_usage();
 
-	info = open_ctree_fs_info(av[optind], 0, 0, 0, 1);
+	info = open_ctree_fs_info(av[optind], 0, 0, 0, 1, 1);
 	if (!info) {
 		fprintf(stderr, "unable to open %s\n", av[optind]);
 		exit(1);
diff --git a/btrfs-find-root.c b/btrfs-find-root.c
index 810d835..d7abc8a 100644
--- a/btrfs-find-root.c
+++ b/btrfs-find-root.c
@@ -102,7 +102,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const
char *device)
 	u64 features;
 
 	ret = btrfs_scan_one_device(fd, device, &fs_devices,
-				    &total_devs, BTRFS_SUPER_INFO_OFFSET);
+				    &total_devs, BTRFS_SUPER_INFO_OFFSET, 1);
 
 	if (ret) {
 		fprintf(stderr, "No valid Btrfs found on %s\n", device);
@@ -110,7 +110,7 @@ static struct btrfs_root *open_ctree_broken(int fd, const
char *device)
 	}
 
 	if (total_devs != 1) {
-		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1);
+		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1, 1);
 		if (ret)
 			goto out;
 	}
@@ -149,8 +149,8 @@ static struct btrfs_root *open_ctree_broken(int fd, const
char *device)
 
 	fs_info->super_bytenr = BTRFS_SUPER_INFO_OFFSET;
 	disk_super = fs_info->super_copy;
-	ret = btrfs_read_dev_super(fs_devices->latest_bdev,
-				   disk_super, BTRFS_SUPER_INFO_OFFSET);
+	ret = btrfs_read_dev_super(fs_devices->latest_bdev, device,
+				   disk_super, BTRFS_SUPER_INFO_OFFSET, 1);
 	if (ret) {
 		printk("No valid btrfs found\n");
 		goto out_devices;
diff --git a/cmds-check.c b/cmds-check.c
index 68cdd52..7aa5e82 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -5185,7 +5185,7 @@ int cmd_check(int argc, char **argv)
 		return -EBUSY;
 	}
 
-	info = open_ctree_fs_info(argv[optind], bytenr, 0, rw, 1);
+	info = open_ctree_fs_info(argv[optind], bytenr, 0, rw, 1, 1);
 	if (!info) {
 		fprintf(stderr, "Couldn''t open file system\n");
 		return -EIO;
diff --git a/cmds-device.c b/cmds-device.c
index 41e79d3..723ce2e 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -203,7 +203,7 @@ static int cmd_scan_dev(int argc, char **argv)
 
 		printf("Scanning for Btrfs filesystems\n");
 		if(checklist)
-			ret = btrfs_scan_block_devices(1);
+			ret = btrfs_scan_block_devices(1, 0);
 		else
 			ret = btrfs_scan_one_dir("/dev", 1);
 		if (ret){
diff --git a/cmds-filesystem.c b/cmds-filesystem.c
index f41a72a..2178986 100644
--- a/cmds-filesystem.c
+++ b/cmds-filesystem.c
@@ -257,7 +257,7 @@ static int cmd_show(int argc, char **argv)
 		usage(cmd_show_usage);
 
 	if(checklist)
-		ret = btrfs_scan_block_devices(0);
+		ret = btrfs_scan_block_devices(0, 0);
 	else
 		ret = btrfs_scan_one_dir("/dev", 0);
 
diff --git a/cmds-replace.c b/cmds-replace.c
index 6397bb5..16b17f0 100644
--- a/cmds-replace.c
+++ b/cmds-replace.c
@@ -280,7 +280,7 @@ static int cmd_start_replace(int argc, char **argv)
 		goto leave_with_error;
 	}
 	ret = btrfs_scan_one_device(fddstdev, dstdev, &fs_devices_mnt,
-				    &total_devs, BTRFS_SUPER_INFO_OFFSET);
+				    &total_devs, BTRFS_SUPER_INFO_OFFSET, 0);
 	if (ret >= 0 && !force_using_targetdev) {
 		fprintf(stderr,
 			"Error, target device %s contains filesystem, use
''-f'' to force overwriting.\n",
diff --git a/cmds-restore.c b/cmds-restore.c
index eca528d..39f19cd 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -842,7 +842,7 @@ static struct btrfs_root *open_fs(const char *dev, u64
root_location,
 
 	for (i = super_mirror; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
-		fs_info = open_ctree_fs_info(dev, bytenr, root_location, 0, 1);
+		fs_info = open_ctree_fs_info(dev, bytenr, root_location, 0, 1, 1);
 		if (fs_info)
 			break;
 		fprintf(stderr, "Could not open root, trying backup super\n");
diff --git a/disk-io.c b/disk-io.c
index 768adda..350daa1 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -801,7 +801,8 @@ struct btrfs_root *btrfs_read_fs_root(struct btrfs_fs_info
*fs_info,
 static struct btrfs_fs_info *__open_ctree_fd(int fp, const char *path,
 					     u64 sb_bytenr,
 					     u64 root_tree_bytenr, int writes,
-					     int partial)
+					     int partial,
+					     int ignore_sb_csum)
 {
 	u32 sectorsize;
 	u32 nodesize;
@@ -830,7 +831,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const
char *path,
 		fprintf(stderr, "Warning, could not drop caches\n");
 
 	ret = btrfs_scan_one_device(fp, path, &fs_devices,
-				    &total_devs, sb_bytenr);
+				    &total_devs, sb_bytenr, ignore_sb_csum);
 
 	if (ret) {
 		fprintf(stderr, "No valid Btrfs found on %s\n", path);
@@ -838,7 +839,7 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const
char *path,
 	}
 
 	if (total_devs != 1) {
-		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1);
+		ret = btrfs_scan_for_fsid(fs_devices, total_devs, 1, ignore_sb_csum);
 		if (ret)
 			goto out;
 	}
@@ -881,8 +882,8 @@ static struct btrfs_fs_info *__open_ctree_fd(int fp, const
char *path,
 
 	fs_info->super_bytenr = sb_bytenr;
 	disk_super = fs_info->super_copy;
-	ret = btrfs_read_dev_super(fs_devices->latest_bdev,
-				   disk_super, sb_bytenr);
+	ret = btrfs_read_dev_super(fs_devices->latest_bdev, path,
+				   disk_super, sb_bytenr, ignore_sb_csum);
 	if (ret) {
 		printk("No valid btrfs found\n");
 		goto out_devices;
@@ -1048,7 +1049,8 @@ out:
 
 struct btrfs_fs_info *open_ctree_fs_info(const char *filename,
 					 u64 sb_bytenr, u64 root_tree_bytenr,
-					 int writes, int partial)
+					 int writes, int partial,
+					 int ignore_sb_csum)
 {
 	int fp;
 	struct btrfs_fs_info *info;
@@ -1063,7 +1065,7 @@ struct btrfs_fs_info *open_ctree_fs_info(const char
*filename,
 		return NULL;
 	}
 	info = __open_ctree_fd(fp, filename, sb_bytenr, root_tree_bytenr,
-			       writes, partial);
+			       writes, partial, ignore_sb_csum);
 	close(fp);
 	return info;
 }
@@ -1072,7 +1074,7 @@ struct btrfs_root *open_ctree(const char *filename, u64
sb_bytenr, int writes)
 {
 	struct btrfs_fs_info *info;
 
-	info = open_ctree_fs_info(filename, sb_bytenr, 0, writes, 0);
+	info = open_ctree_fs_info(filename, sb_bytenr, 0, writes, 0, 0);
 	if (!info)
 		return NULL;
 	return info->fs_root;
@@ -1082,53 +1084,106 @@ struct btrfs_root *open_ctree_fd(int fp, const char
*path, u64 sb_bytenr,
 				 int writes)
 {
 	struct btrfs_fs_info *info;
-	info = __open_ctree_fd(fp, path, sb_bytenr, 0, writes, 0);
+	info = __open_ctree_fd(fp, path, sb_bytenr, 0, writes, 0, 0);
 	if (!info)
 		return NULL;
 	return info->fs_root;
 }
 
-int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr)
+static int btrfs_check_super_csum(char *raw_disk_sb, const char *path)
+{
+	struct btrfs_super_block *disk_sb +		(struct btrfs_super_block *)raw_disk_sb;
+	u16 csum_type = btrfs_super_csum_type(disk_sb);
+	int ret = 0;
+
+	if (csum_type == BTRFS_CSUM_TYPE_CRC32) {
+		u32 crc = ~(u32)0;
+		const int csum_size = sizeof(crc);
+		char result[csum_size];
+
+		/*
+		 * The super_block structure does not span the whole
+		 * BTRFS_SUPER_INFO_SIZE range, we expect that the unused space
+		 * is filled with zeros and is included in the checkum.
+		 */
+		crc = btrfs_csum_data(NULL, raw_disk_sb + BTRFS_CSUM_SIZE,
+				crc, BTRFS_SUPER_INFO_SIZE - BTRFS_CSUM_SIZE);
+		btrfs_csum_final(crc, result);
+
+		if (memcmp(raw_disk_sb, result, csum_size))
+			ret = 1;
+
+		if (ret && btrfs_super_generation(disk_sb) < 1) {
+			fprintf(stderr, "Super block CRCs don''t match, "
+				"older mkfs detected\n");
+			ret = 0;
+		}
+	}
+
+	if (ret) {
+		fprintf(stderr,
+			"Super block checksum validation failed, offset %llu,"
+			" device %s\n",
+			(unsigned long long)btrfs_super_bytenr(disk_sb), path);
+	}
+
+	if (csum_type >= ARRAY_SIZE(btrfs_csum_sizes)) {
+		fprintf(stderr, "Unsupported super block checksum algorithm %u\n",
+			csum_type);
+		ret = 1;
+	}
+
+	return ret;
+}
+
+int btrfs_read_dev_super(int fd, const char *path,
+			 struct btrfs_super_block *sb, u64 sb_bytenr,
+			 int ignore_sb_csum)
 {
 	u8 fsid[BTRFS_FSID_SIZE];
 	int fsid_is_initialized = 0;
-	struct btrfs_super_block buf;
+	char buf[BTRFS_SUPER_INFO_SIZE];
+	struct btrfs_super_block *tmp_sb = (struct btrfs_super_block *)buf;
 	int i;
 	int ret;
 	u64 transid = 0;
 	u64 bytenr;
 
 	if (sb_bytenr != BTRFS_SUPER_INFO_OFFSET) {
-		ret = pread64(fd, &buf, sizeof(buf), sb_bytenr);
+		ret = pread64(fd, buf, sizeof(buf), sb_bytenr);
 		if (ret < sizeof(buf))
 			return -1;
 
-		if (btrfs_super_bytenr(&buf) != sb_bytenr ||
-		    buf.magic != cpu_to_le64(BTRFS_MAGIC))
+		if (btrfs_super_bytenr(tmp_sb) != sb_bytenr ||
+		    tmp_sb->magic != cpu_to_le64(BTRFS_MAGIC) ||
+		    (btrfs_check_super_csum(buf, path) && !ignore_sb_csum))
 			return -1;
 
-		memcpy(sb, &buf, sizeof(*sb));
+		memcpy(sb, buf, sizeof(*sb));
 		return 0;
 	}
 
 	for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
 		bytenr = btrfs_sb_offset(i);
-		ret = pread64(fd, &buf, sizeof(buf), bytenr);
+		ret = pread64(fd, buf, sizeof(buf), bytenr);
 		if (ret < sizeof(buf))
 			break;
 
-		if (btrfs_super_bytenr(&buf) != bytenr )
+		if (btrfs_super_bytenr(tmp_sb) != bytenr )
 			continue;
 		/* if magic is NULL, the device was removed */
-		if (buf.magic == 0 && i == 0) 
+		if (tmp_sb->magic == 0 && i == 0)
 			return -1;
-		if (buf.magic != cpu_to_le64(BTRFS_MAGIC))
+		if (tmp_sb->magic != cpu_to_le64(BTRFS_MAGIC))
+			continue;
+		if (btrfs_check_super_csum(buf, path) && !ignore_sb_csum)
 			continue;
 
 		if (!fsid_is_initialized) {
-			memcpy(fsid, buf.fsid, sizeof(fsid));
+			memcpy(fsid, tmp_sb->fsid, sizeof(fsid));
 			fsid_is_initialized = 1;
-		} else if (memcmp(fsid, buf.fsid, sizeof(fsid))) {
+		} else if (memcmp(fsid, tmp_sb->fsid, sizeof(fsid))) {
 			/*
 			 * the superblocks (the original one and
 			 * its backups) contain data of different
@@ -1137,9 +1192,9 @@ int btrfs_read_dev_super(int fd, struct btrfs_super_block
*sb, u64 sb_bytenr)
 			continue;
 		}
 
-		if (btrfs_super_generation(&buf) > transid) {
-			memcpy(sb, &buf, sizeof(*sb));
-			transid = btrfs_super_generation(&buf);
+		if (btrfs_super_generation(tmp_sb) > transid) {
+			memcpy(sb, buf, sizeof(*sb));
+			transid = btrfs_super_generation(tmp_sb);
 		}
 	}
 
diff --git a/disk-io.h b/disk-io.h
index c29ee8e..f061998 100644
--- a/disk-io.h
+++ b/disk-io.h
@@ -52,12 +52,15 @@ struct btrfs_root *open_ctree_fd(int fp, const char *path,
u64 sb_bytenr,
 				 int writes);
 struct btrfs_fs_info *open_ctree_fs_info(const char *filename,
 					 u64 sb_bytenr, u64 root_tree_bytenr,
-					 int writes, int partial);
+					 int writes, int partial,
+					 int ignore_sb_csum);
 int close_ctree(struct btrfs_root *root);
 int write_all_supers(struct btrfs_root *root);
 int write_ctree_super(struct btrfs_trans_handle *trans,
 		      struct btrfs_root *root);
-int btrfs_read_dev_super(int fd, struct btrfs_super_block *sb, u64 sb_bytenr);
+int btrfs_read_dev_super(int fd, const char *path,
+			 struct btrfs_super_block *sb, u64 sb_bytenr,
+			 int ignore_sb_csum);
 int btrfs_map_bh_to_logical(struct btrfs_root *root, struct extent_buffer *bh,
 			    u64 logical);
 struct extent_buffer *btrfs_find_tree_block(struct btrfs_root *root,
diff --git a/utils.c b/utils.c
index 7b4cd74..4c9e89a 100644
--- a/utils.c
+++ b/utils.c
@@ -923,12 +923,12 @@ int check_mounted_where(int fd, const char *file, char
*where, int size,
 
 	/* scan the initial device */
 	ret = btrfs_scan_one_device(fd, file, &fs_devices_mnt,
-				    &total_devs, BTRFS_SUPER_INFO_OFFSET);
+				    &total_devs, BTRFS_SUPER_INFO_OFFSET, 1);
 	is_btrfs = (ret >= 0);
 
 	/* scan other devices */
 	if (is_btrfs && total_devs > 1) {
-		if((ret = btrfs_scan_for_fsid(fs_devices_mnt, total_devs, 1)))
+		if((ret = btrfs_scan_for_fsid(fs_devices_mnt, total_devs, 1, 1)))
 			return ret;
 	}
 
@@ -1085,7 +1085,7 @@ again:
 		}
 		ret = btrfs_scan_one_device(fd, fullpath, &tmp_devices,
 					    &num_devices,
-					    BTRFS_SUPER_INFO_OFFSET);
+					    BTRFS_SUPER_INFO_OFFSET, 0);
 		if (ret == 0 && run_ioctl > 0) {
 			btrfs_register_one_device(fullpath);
 		}
@@ -1111,11 +1111,11 @@ fail:
 }
 
 int btrfs_scan_for_fsid(struct btrfs_fs_devices *fs_devices, u64 total_devs,
-			int run_ioctls)
+			int run_ioctls, int ignore_sb_csum)
 {
 	int ret;
 
-	ret = btrfs_scan_block_devices(run_ioctls);
+	ret = btrfs_scan_block_devices(run_ioctls, ignore_sb_csum);
 	if (ret)
 		ret = btrfs_scan_one_dir("/dev", run_ioctls);
 	return ret;
@@ -1350,7 +1350,7 @@ int set_label(const char *btrfs_dev, const char *label)
 		set_label_mounted(btrfs_dev, label);
 }
 
-int btrfs_scan_block_devices(int run_ioctl)
+int btrfs_scan_block_devices(int run_ioctl, int ignore_sb_csum)
 {
 
 	struct stat st;
@@ -1416,7 +1416,8 @@ scan_again:
 		}
 		ret = btrfs_scan_one_device(fd, fullpath, &tmp_devices,
 					    &num_devices,
-					    BTRFS_SUPER_INFO_OFFSET);
+					    BTRFS_SUPER_INFO_OFFSET,
+					    ignore_sb_csum);
 		if (ret == 0 && run_ioctl > 0) {
 			btrfs_register_one_device(fullpath);
 		}
diff --git a/utils.h b/utils.h
index 3c17e14..9c1a98e 100644
--- a/utils.h
+++ b/utils.h
@@ -36,7 +36,7 @@ int btrfs_add_to_fsid(struct btrfs_trans_handle *trans,
 		      u64 block_count, u32 io_width, u32 io_align,
 		      u32 sectorsize);
 int btrfs_scan_for_fsid(struct btrfs_fs_devices *fs_devices, u64 total_devs,
-			int run_ioctls);
+			int run_ioctls, int ignore_sb_csum);
 void btrfs_register_one_device(char *fname);
 int btrfs_scan_one_dir(char *dirname, int run_ioctl);
 int check_mounted(const char *devicename);
@@ -46,7 +46,7 @@ int btrfs_device_already_in_root(struct btrfs_root *root, int
fd,
 				 int super_offset);
 char *pretty_sizes(u64 size);
 int get_mountpt(char *dev, char *mntpt, size_t size);
-int btrfs_scan_block_devices(int run_ioctl);
+int btrfs_scan_block_devices(int run_ioctl, int ignore_sb_csum);
 u64 parse_size(char *s);
 int open_file_or_dir(const char *fname);
 int get_device_info(int fd, u64 devid,
diff --git a/volumes.c b/volumes.c
index 061f094..3178281 100644
--- a/volumes.c
+++ b/volumes.c
@@ -213,7 +213,8 @@ fail:
 
 int btrfs_scan_one_device(int fd, const char *path,
 			  struct btrfs_fs_devices **fs_devices_ret,
-			  u64 *total_devs, u64 super_offset)
+			  u64 *total_devs, u64 super_offset,
+			  int ignore_sb_csum)
 {
 	struct btrfs_super_block *disk_super;
 	char *buf;
@@ -227,7 +228,8 @@ int btrfs_scan_one_device(int fd, const char *path,
 		goto error;
 	}
 	disk_super = (struct btrfs_super_block *)buf;
-	ret = btrfs_read_dev_super(fd, disk_super, super_offset);
+	ret = btrfs_read_dev_super(fd, path,
+				   disk_super, super_offset, ignore_sb_csum);
 	if (ret < 0) {
 		ret = -EIO;
 		goto error_brelse;
diff --git a/volumes.h b/volumes.h
index 911f788..e8d1534 100644
--- a/volumes.h
+++ b/volumes.h
@@ -179,7 +179,8 @@ int btrfs_update_device(struct btrfs_trans_handle *trans,
 			struct btrfs_device *device);
 int btrfs_scan_one_device(int fd, const char *path,
 			  struct btrfs_fs_devices **fs_devices_ret,
-			  u64 *total_devs, u64 super_offset);
+			  u64 *total_devs, u64 super_offset,
+			  int ignore_sb_csum);
 int btrfs_num_copies(struct btrfs_mapping_tree *map_tree, u64 logical, u64
len);
 int btrfs_bootstrap_super_map(struct btrfs_mapping_tree *map_tree,
 			      struct btrfs_fs_devices *fs_devices);
-- 
1.7.9.5

--
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

Btrfs devel - Jun 2013 - [PATCH 0/5] Btrfs-progs: coalesce of patches

[PATCH 0/5] Btrfs-progs: coalesce of patches

[PATCH 1/5] Btrfs-progs: fix closing of devices

[PATCH 2/5] Btrfs-progs: Add missing free_extent_buffer() call to debug-tree

[PATCH 3/5] Btrfs-progs: Add missing close_ctree() calls to debug-tree

[PATCH 4/5] Btrfs-progs: pretty print dir_item type

[PATCH 5/5] Btrfs-progs: Validate super block checksum

[PATCH 1/5] Btrfs-progs: fix closing of devices

[PATCH 1/5 v2] Btrfs-progs: fix closing of devices

Re: [PATCH 1/5] Btrfs-progs: fix closing of devices

Re: [PATCH 1/5] Btrfs-progs: fix closing of devices

Re: [PATCH 5/5] Btrfs-progs: Validate super block checksum

Re: [PATCH 1/5 v2] Btrfs-progs: fix closing of devices

Re: [PATCH 1/5 v2] Btrfs-progs: fix closing of devices

Re: [PATCH 5/5] Btrfs-progs: Validate super block checksum

Re: [PATCH 5/5] Btrfs-progs: Validate super block checksum

Re: [PATCH 1/5 v2] Btrfs-progs: fix closing of devices

[PATCH v2 1/5] Btrfs-progs: fix closing of devices

[PATCH v2 5/5] Btrfs-progs: Validate super block checksum