[PATCH 1/2] btrfs-progs: dump-super: check array_size in print_sys_chunk_array

[PATCH 1/2] btrfs-progs: dump-super: check array_size in print_sys_chunk_array

[Lu Fengqi]

Without validation of array_size, the dump-super may lead to a bad
memory access.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 cmds-inspect-dump-super.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
index ee2c8e3a..48b5219c 100644
--- a/cmds-inspect-dump-super.c
+++ b/cmds-inspect-dump-super.c
@@ -62,16 +62,23 @@ static void print_sys_chunk_array(struct btrfs_super_block *sb)
 	struct btrfs_key key;
 	int item;
 
-	buf = malloc(sizeof(*buf) + sizeof(*sb));
+	buf = malloc(sizeof(*buf) + BTRFS_SUPER_INFO_SIZE);
 	if (!buf) {
 		error("not enough memory");
-		goto out;
+		return;
 	}
-	write_extent_buffer(buf, sb, 0, sizeof(*sb));
+	write_extent_buffer(buf, sb, 0, BTRFS_SUPER_INFO_SIZE);
 	array_size = btrfs_super_sys_array_size(sb);
 
 	array_ptr = sb->sys_chunk_array;
 	sb_array_offset = offsetof(struct btrfs_super_block, sys_chunk_array);
+
+	if (array_size > BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
+		error("sys_array_size %u shouldn't exceed %u bytes",
+				array_size, BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
+		goto out;
+	}
+
 	cur_offset = 0;
 	item = 0;
 
@@ -124,8 +131,8 @@ static void print_sys_chunk_array(struct btrfs_super_block *sb)
 		item++;
 	}
 
-	free(buf);
 out:
+	free(buf);
 	return;
 
 out_short_read:
-- 
2.12.1

[PATCH 2/2] btrfs-progs: print-tree: add validation to print_chunk

[Lu Fengqi]

In print_chunk, validate the value of uuid_offset when read the dev_uuid of
stripe.

Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
---
 cmds-inspect-dump-super.c |  1 +
 print-tree.c              | 20 +++++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
index 48b5219c..a89fe451 100644
--- a/cmds-inspect-dump-super.c
+++ b/cmds-inspect-dump-super.c
@@ -68,6 +68,7 @@ static void print_sys_chunk_array(struct btrfs_super_block *sb)
 		return;
 	}
 	write_extent_buffer(buf, sb, 0, BTRFS_SUPER_INFO_SIZE);
+	buf->len = BTRFS_SUPER_INFO_SIZE;
 	array_size = btrfs_super_sys_array_size(sb);
 
 	array_ptr = sb->sys_chunk_array;
diff --git a/print-tree.c b/print-tree.c
index 5af80e87..8352e03d 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -199,8 +199,14 @@ void print_chunk(struct extent_buffer *eb, struct btrfs_chunk *chunk)
 {
 	int num_stripes = btrfs_chunk_num_stripes(eb, chunk);
 	int i;
+	u32 chunk_item_size = btrfs_chunk_item_size(num_stripes);
 	char chunk_flags_str[32] = {0};
 
+	if ((unsigned long)chunk + chunk_item_size > eb->len) {
+		printf("\t\tchunk item invalid\n");
+		return;
+	}
+
 	bg_flags_to_str(btrfs_chunk_type(eb, chunk), chunk_flags_str);
 	printf("\t\tlength %llu owner %llu stripe_len %llu type %s\n",
 	       (unsigned long long)btrfs_chunk_length(eb, chunk),
@@ -216,9 +222,21 @@ void print_chunk(struct extent_buffer *eb, struct btrfs_chunk *chunk)
 	for (i = 0 ; i < num_stripes ; i++) {
 		unsigned char dev_uuid[BTRFS_UUID_SIZE];
 		char str_dev_uuid[BTRFS_UUID_UNPARSED_SIZE];
+		u64 uuid_offset;
+		u64 stripe_offset;
+
+		uuid_offset = (unsigned long)btrfs_stripe_dev_uuid_nr(chunk, i);
+		stripe_offset = (unsigned long)btrfs_stripe_nr(chunk, i);
+
+		if (uuid_offset < stripe_offset ||
+			(uuid_offset + BTRFS_UUID_SIZE) >
+				(stripe_offset + sizeof(struct btrfs_stripe))) {
+			printf("\t\t\tstripe %d invalid\n", i);
+			break;
+		}
 
 		read_extent_buffer(eb, dev_uuid,
-			(unsigned long)btrfs_stripe_dev_uuid_nr(chunk, i),
+			uuid_offset,
 			BTRFS_UUID_SIZE);
 		uuid_unparse(dev_uuid, str_dev_uuid);
 		printf("\t\t\tstripe %d devid %llu offset %llu\n", i,
-- 
2.12.1

Re: [PATCH 1/2] btrfs-progs: dump-super: check array_size in print_sys_chunk_array

[David Sterba]

On Wed, Apr 05, 2017 at 04:59:14PM +0800, Lu Fengqi wrote:
> Without validation of array_size, the dump-super may lead to a bad
> memory access.
> 
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> ---
>  cmds-inspect-dump-super.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c
> index ee2c8e3a..48b5219c 100644
> --- a/cmds-inspect-dump-super.c
> +++ b/cmds-inspect-dump-super.c
> @@ -62,16 +62,23 @@ static void print_sys_chunk_array(struct btrfs_super_block *sb)
>  	struct btrfs_key key;
>  	int item;
>  
> -	buf = malloc(sizeof(*buf) + sizeof(*sb));
> +	buf = malloc(sizeof(*buf) + BTRFS_SUPER_INFO_SIZE);

This seems to be unnecessary, the super block structure should contain
entier sys_array.

>  	if (!buf) {
>  		error("not enough memory");
> -		goto out;
> +		return;
>  	}
> -	write_extent_buffer(buf, sb, 0, sizeof(*sb));
> +	write_extent_buffer(buf, sb, 0, BTRFS_SUPER_INFO_SIZE);
>  	array_size = btrfs_super_sys_array_size(sb);
>  
>  	array_ptr = sb->sys_chunk_array;
>  	sb_array_offset = offsetof(struct btrfs_super_block, sys_chunk_array);
> +
> +	if (array_size > BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
> +		error("sys_array_size %u shouldn't exceed %u bytes",
> +				array_size, BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
> +		goto out;
> +	}
> +
>  	cur_offset = 0;
>  	item = 0;
>  
> @@ -124,8 +131,8 @@ static void print_sys_chunk_array(struct btrfs_super_block *sb)
>  		item++;
>  	}
>  
> -	free(buf);
>  out:
> +	free(buf);
>  	return;
>  
>  out_short_read:
> -- 
> 2.12.1
> 
> 
> 
> --
> 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

Re: [PATCH 1/2] btrfs-progs: dump-super: check array_size in print_sys_chunk_array

[Lu Fengqi]

On 2017年04月19日 23:41, David Sterba wrote:
>>   
>> -	buf = malloc(sizeof(*buf) + sizeof(*sb));
>> +	buf = malloc(sizeof(*buf) + BTRFS_SUPER_INFO_SIZE);
> This seems to be unnecessary, the super block structure should contain
> entier sys_array.
> 
Alright, I will remove it.

-- 
Thanks,
Lu