[PATCH 1/3] btrfs-progs: Remove fs_info parameter from btrfs_leaf_free_space()

[PATCH 1/3] btrfs-progs: Remove fs_info parameter from btrfs_leaf_free_space()

[Qu Wenruo]

For btrfs_leaf_free_space(), to get leaf data size, we have two way to
get it:

1) leaf->fs_info->nodesize
2) leaf->len

Anyway, we could get rid of @fs_info parameter for
btrfs_leaf_free_space().
And here we choose method 2), as it provides extra benefit to get leaf
free space without initializing a real fs_info.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 btrfs-corrupt-block.c |  2 +-
 check/main.c          |  2 +-
 check/mode-lowmem.c   |  2 +-
 ctree.c               | 49 ++++++++++++++++++++++---------------------
 ctree.h               |  3 +--
 print-tree.c          |  2 +-
 6 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
index da0ec8c51e5a..fc06204b6481 100644
--- a/btrfs-corrupt-block.c
+++ b/btrfs-corrupt-block.c
@@ -726,7 +726,7 @@ out:
 static void shift_items(struct btrfs_root *root, struct extent_buffer *eb)
 {
 	int nritems = btrfs_header_nritems(eb);
-	int shift_space = btrfs_leaf_free_space(root->fs_info, eb) / 2;
+	int shift_space = btrfs_leaf_free_space(eb) / 2;
 	int slot = nritems / 2;
 	int i = 0;
 	unsigned int data_end = btrfs_item_offset_nr(eb, nritems - 1);
diff --git a/check/main.c b/check/main.c
index c4a1801fb0ef..2030fe663e1b 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5970,7 +5970,7 @@ static int run_next_block(struct btrfs_root *root,
 		goto out;
 
 	if (btrfs_is_leaf(buf)) {
-		btree_space_waste += btrfs_leaf_free_space(fs_info, buf);
+		btree_space_waste += btrfs_leaf_free_space(buf);
 		for (i = 0; i < nritems; i++) {
 			struct btrfs_file_extent_item *fi;
 
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index bfe45abab6cc..227ecf87cf97 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -390,7 +390,7 @@ static void account_bytes(struct btrfs_root *root, struct btrfs_path *path,
 		total_extent_tree_bytes += eb->len;
 
 	if (level == 0) {
-		btree_space_waste += btrfs_leaf_free_space(root->fs_info, eb);
+		btree_space_waste += btrfs_leaf_free_space(eb);
 	} else {
 		free_nrs = (BTRFS_NODEPTRS_PER_BLOCK(root->fs_info) -
 			    btrfs_header_nritems(eb));
diff --git a/ctree.c b/ctree.c
index d1c41925d71c..2c3ba70b000c 100644
--- a/ctree.c
+++ b/ctree.c
@@ -480,11 +480,11 @@ btrfs_check_leaf(struct btrfs_root *root, struct btrfs_disk_key *parent_key,
 		       (unsigned long long)btrfs_header_bytenr(buf));
 		goto fail;
 	}
-	if (btrfs_leaf_free_space(root->fs_info, buf) < 0) {
+	if (btrfs_leaf_free_space(buf) < 0) {
 		ret = BTRFS_TREE_BLOCK_INVALID_FREE_SPACE;
 		fprintf(stderr, "leaf free space incorrect %llu %d\n",
 			(unsigned long long)btrfs_header_bytenr(buf),
-			btrfs_leaf_free_space(root->fs_info, buf));
+			btrfs_leaf_free_space(buf));
 		goto fail;
 	}
 
@@ -1193,7 +1193,7 @@ again:
 		} else {
 			p->slots[level] = slot;
 			if (ins_len > 0 &&
-			    ins_len > btrfs_leaf_free_space(root->fs_info, b)) {
+			    ins_len > btrfs_leaf_free_space(b)) {
 				int sret = split_leaf(trans, root, key,
 						      p, ins_len, ret == 0);
 				BUG_ON(sret > 0);
@@ -1634,17 +1634,19 @@ static int leaf_space_used(struct extent_buffer *l, int start, int nr)
  * the start of the leaf data.  IOW, how much room
  * the leaf has left for both items and data
  */
-int btrfs_leaf_free_space(struct btrfs_fs_info *fs_info,
-			  struct extent_buffer *leaf)
+int btrfs_leaf_free_space(struct extent_buffer *leaf)
 {
 	int nritems = btrfs_header_nritems(leaf);
+	u32 leaf_data_size;
 	int ret;
 
-	ret = BTRFS_LEAF_DATA_SIZE(fs_info) - leaf_space_used(leaf, 0, nritems);
+	BUG_ON(leaf->fs_info && leaf->fs_info->nodesize != leaf->len);
+	leaf_data_size = __BTRFS_LEAF_DATA_SIZE(leaf->len);
+	ret = leaf_data_size - leaf_space_used(leaf, 0 ,nritems);
 	if (ret < 0) {
-		printk("leaf free space ret %d, leaf data size %lu, used %d nritems %d\n",
-		       ret, BTRFS_LEAF_DATA_SIZE(fs_info),
-		       leaf_space_used(leaf, 0, nritems), nritems);
+		printk("leaf free space ret %d, leaf data size %u, used %d nritems %d\n",
+		       ret, leaf_data_size, leaf_space_used(leaf, 0, nritems),
+		       nritems);
 	}
 	return ret;
 }
@@ -1692,7 +1694,7 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root
 			return PTR_ERR(right);
 		return -EIO;
 	}
-	free_space = btrfs_leaf_free_space(fs_info, right);
+	free_space = btrfs_leaf_free_space(right);
 	if (free_space < data_size) {
 		free_extent_buffer(right);
 		return 1;
@@ -1705,7 +1707,7 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root
 		free_extent_buffer(right);
 		return 1;
 	}
-	free_space = btrfs_leaf_free_space(fs_info, right);
+	free_space = btrfs_leaf_free_space(right);
 	if (free_space < data_size) {
 		free_extent_buffer(right);
 		return 1;
@@ -1844,7 +1846,7 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root
 	}
 
 	left = read_node_slot(fs_info, path->nodes[1], slot - 1);
-	free_space = btrfs_leaf_free_space(fs_info, left);
+	free_space = btrfs_leaf_free_space(left);
 	if (free_space < data_size) {
 		free_extent_buffer(left);
 		return 1;
@@ -1859,7 +1861,7 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root
 		return 1;
 	}
 
-	free_space = btrfs_leaf_free_space(fs_info, left);
+	free_space = btrfs_leaf_free_space(left);
 	if (free_space < data_size) {
 		free_extent_buffer(left);
 		return 1;
@@ -2083,7 +2085,7 @@ static noinline int split_leaf(struct btrfs_trans_handle *trans,
 		l = path->nodes[0];
 
 		/* did the pushes work? */
-		if (btrfs_leaf_free_space(root->fs_info, l) >= data_size)
+		if (btrfs_leaf_free_space(l) >= data_size)
 			return 0;
 	}
 
@@ -2240,7 +2242,7 @@ int btrfs_split_item(struct btrfs_trans_handle *trans,
 
 	leaf = path->nodes[0];
 	btrfs_item_key_to_cpu(leaf, &orig_key, path->slots[0]);
-	if (btrfs_leaf_free_space(root->fs_info, leaf) >=
+	if (btrfs_leaf_free_space(leaf) >=
 	    sizeof(struct btrfs_item))
 		goto split;
 
@@ -2261,8 +2263,7 @@ int btrfs_split_item(struct btrfs_trans_handle *trans,
 	ret = split_leaf(trans, root, &orig_key, path, 0, 0);
 	BUG_ON(ret);
 
-	BUG_ON(btrfs_leaf_free_space(root->fs_info, leaf) <
-	       sizeof(struct btrfs_item));
+	BUG_ON(btrfs_leaf_free_space(leaf) < sizeof(struct btrfs_item));
 	leaf = path->nodes[0];
 
 split:
@@ -2314,7 +2315,7 @@ split:
 	btrfs_mark_buffer_dirty(leaf);
 
 	ret = 0;
-	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
+	if (btrfs_leaf_free_space(leaf) < 0) {
 		btrfs_print_leaf(leaf);
 		BUG();
 	}
@@ -2410,7 +2411,7 @@ int btrfs_truncate_item(struct btrfs_root *root, struct btrfs_path *path,
 	btrfs_mark_buffer_dirty(leaf);
 
 	ret = 0;
-	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
+	if (btrfs_leaf_free_space(leaf) < 0) {
 		btrfs_print_leaf(leaf);
 		BUG();
 	}
@@ -2435,7 +2436,7 @@ int btrfs_extend_item(struct btrfs_root *root, struct btrfs_path *path,
 	nritems = btrfs_header_nritems(leaf);
 	data_end = leaf_data_end(root->fs_info, leaf);
 
-	if (btrfs_leaf_free_space(root->fs_info, leaf) < data_size) {
+	if (btrfs_leaf_free_space(leaf) < data_size) {
 		btrfs_print_leaf(leaf);
 		BUG();
 	}
@@ -2472,7 +2473,7 @@ int btrfs_extend_item(struct btrfs_root *root, struct btrfs_path *path,
 	btrfs_mark_buffer_dirty(leaf);
 
 	ret = 0;
-	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
+	if (btrfs_leaf_free_space(leaf) < 0) {
 		btrfs_print_leaf(leaf);
 		BUG();
 	}
@@ -2521,10 +2522,10 @@ int btrfs_insert_empty_items(struct btrfs_trans_handle *trans,
 	nritems = btrfs_header_nritems(leaf);
 	data_end = leaf_data_end(root->fs_info, leaf);
 
-	if (btrfs_leaf_free_space(root->fs_info, leaf) < total_size) {
+	if (btrfs_leaf_free_space(leaf) < total_size) {
 		btrfs_print_leaf(leaf);
 		printk("not enough freespace need %u have %d\n",
-		       total_size, btrfs_leaf_free_space(root->fs_info, leaf));
+		       total_size, btrfs_leaf_free_space(leaf));
 		BUG();
 	}
 
@@ -2582,7 +2583,7 @@ int btrfs_insert_empty_items(struct btrfs_trans_handle *trans,
 		btrfs_fixup_low_keys(root, path, &disk_key, 1);
 	}
 
-	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
+	if (btrfs_leaf_free_space(leaf) < 0) {
 		btrfs_print_leaf(leaf);
 		BUG();
 	}
diff --git a/ctree.h b/ctree.h
index 1fef37c93485..e291a7d48d14 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2664,8 +2664,7 @@ static inline int btrfs_next_item(struct btrfs_root *root,
 }
 
 int btrfs_prev_leaf(struct btrfs_root *root, struct btrfs_path *path);
-int btrfs_leaf_free_space(struct btrfs_fs_info *fs_info,
-			  struct extent_buffer *leaf);
+int btrfs_leaf_free_space(struct extent_buffer *leaf);
 void btrfs_fixup_low_keys(struct btrfs_root *root, struct btrfs_path *path,
 			  struct btrfs_disk_key *key, int level);
 int btrfs_set_item_key_safe(struct btrfs_root *root, struct btrfs_path *path,
diff --git a/print-tree.c b/print-tree.c
index a1a7954abdae..34a724d5f7ef 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1191,7 +1191,7 @@ void btrfs_print_leaf(struct extent_buffer *eb)
 
 	printf("leaf %llu items %d free space %d generation %llu owner ",
 		(unsigned long long)btrfs_header_bytenr(eb), nr,
-		btrfs_leaf_free_space(fs_info, eb),
+		btrfs_leaf_free_space(eb),
 		(unsigned long long)btrfs_header_generation(eb));
 	print_objectid(stdout, btrfs_header_owner(eb), 0);
 	printf("\n");
-- 
2.17.0

[PATCH 2/3] btrfs-progs: Allow tree to be printed without an fs_info

[Qu Wenruo]

For btrfs_print_tree() and btrfs_print_leaf(), the usage of fs_info is
mainly for nodesize and sectorsize.

However for nodesize, we can get it from @eb->len without the need for
fs_info at all.

For nodesize, introduce new helper BTRFS_NODEPTR_PER_EXTENT_BUFFER() to
get nodesize from @eb directly.
And with the help of previous modified btrfs_leaf_free_space(),
btrfs_print_tree() can live without fs_info at all.

For btrfs_print_leaf(), we modify print_extent_csum() to accept NULL
fs_info by skipping csum length calculation.

With all these modification, btrfs_print_tree/leaf() can be called
without accessing @fs_info at all, and make it more flex to handle
binary tree block dump, or inside gdb.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 ctree.h      |  7 +++++++
 print-tree.c | 15 +++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/ctree.h b/ctree.h
index e291a7d48d14..80fd19612434 100644
--- a/ctree.h
+++ b/ctree.h
@@ -1202,6 +1202,13 @@ static inline u32 BTRFS_NODEPTRS_PER_BLOCK(const struct btrfs_fs_info *info)
 	return BTRFS_LEAF_DATA_SIZE(info) / sizeof(struct btrfs_key_ptr);
 }
 
+static inline u32
+BTRFS_NODEPTRS_PER_EXTENT_BUFFER(const struct extent_buffer *eb)
+{
+	BUG_ON(eb->fs_info && eb->fs_info->nodesize != eb->len);
+	return __BTRFS_LEAF_DATA_SIZE(eb->len) / sizeof(struct btrfs_key_ptr);
+}
+
 #define BTRFS_FILE_EXTENT_INLINE_DATA_START		\
 	(offsetof(struct btrfs_file_extent_item, disk_bytenr))
 static inline u32 BTRFS_MAX_INLINE_DATA_SIZE(const struct btrfs_fs_info *info)
diff --git a/print-tree.c b/print-tree.c
index 34a724d5f7ef..31a851ef4413 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1150,6 +1150,14 @@ static void print_extent_csum(struct extent_buffer *eb,
 {
 	u32 size;
 
+	/*
+	 * If we don't have fs_info, only output its start position as we
+	 * don't have sectorsize for the calculation
+	 */
+	if (!fs_info) {
+		printf("\t\trange start %llu\n", (unsigned long long)start);
+		return;
+	}
 	size = (item_size / btrfs_super_csum_size(fs_info->super_copy)) *
 			fs_info->sectorsize;
 	printf("\t\trange start %llu end %llu length %u\n",
@@ -1371,7 +1379,7 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
 	printf("node %llu level %d items %d free %u generation %llu owner ",
 	       (unsigned long long)eb->start,
 	        btrfs_header_level(eb), nr,
-		(u32)BTRFS_NODEPTRS_PER_BLOCK(fs_info) - nr,
+		(u32)BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb) - nr,
 		(unsigned long long)btrfs_header_generation(eb));
 	print_objectid(stdout, btrfs_header_owner(eb), 0);
 	printf("\n");
@@ -1385,13 +1393,16 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
 		btrfs_print_key(&disk_key);
 		printf(" block %llu (%llu) gen %llu\n",
 		       (unsigned long long)blocknr,
-		       (unsigned long long)blocknr / fs_info->nodesize,
+		       (unsigned long long)blocknr / eb->len,
 		       (unsigned long long)btrfs_node_ptr_generation(eb, i));
 		fflush(stdout);
 	}
 	if (!follow)
 		return;
 
+	if (follow && !fs_info)
+		return;
+
 	for (i = 0; i < nr; i++) {
 		next = read_tree_block(fs_info,
 				btrfs_node_blockptr(eb, i),
-- 
2.17.0

[PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access

[Qu Wenruo]

For btrfs_print_tree(), if nr_items is corrupted, it can easily go
beyond extent buffer boundary.

Add extra nr_item check, and only print as many valid slots as possible.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 print-tree.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/print-tree.c b/print-tree.c
index 31a851ef4413..55db80bebb2a 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1376,6 +1376,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
 		btrfs_print_leaf(eb);
 		return;
 	}
+	/* We are crossing eb boundary, this node must be corrupted */
+	if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
+		warning(
+		"node nr_items corrupted, has %u limit %u, continue print anyway",
+			nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
 	printf("node %llu level %d items %d free %u generation %llu owner ",
 	       (unsigned long long)eb->start,
 	        btrfs_header_level(eb), nr,
@@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
 	print_uuids(eb);
 	fflush(stdout);
 	for (i = 0; i < nr; i++) {
-		u64 blocknr = btrfs_node_blockptr(eb, i);
+		u64 blocknr;
+
+		if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
+			break;
+		blocknr = btrfs_node_blockptr(eb, i);
 		btrfs_node_key(eb, &disk_key, i);
 		btrfs_disk_key_to_cpu(&key, &disk_key);
 		printf("\t");
-- 
2.17.0

Re: [PATCH 1/3] btrfs-progs: Remove fs_info parameter from btrfs_leaf_free_space()

[Su Yue]

On 04/30/2018 11:15 AM, Qu Wenruo wrote:
> For btrfs_leaf_free_space(), to get leaf data size, we have two way to
> get it:
> 
> 1) leaf->fs_info->nodesize
> 2) leaf->len
> 
> Anyway, we could get rid of @fs_info parameter for
> btrfs_leaf_free_space().
> And here we choose method 2), as it provides extra benefit to get leaf
> free space without initializing a real fs_info.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Su Yue <suy.fnst@cn.fujitsu.com>

> ---
>   btrfs-corrupt-block.c |  2 +-
>   check/main.c          |  2 +-
>   check/mode-lowmem.c   |  2 +-
>   ctree.c               | 49 ++++++++++++++++++++++---------------------
>   ctree.h               |  3 +--
>   print-tree.c          |  2 +-
>   6 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/btrfs-corrupt-block.c b/btrfs-corrupt-block.c
> index da0ec8c51e5a..fc06204b6481 100644
> --- a/btrfs-corrupt-block.c
> +++ b/btrfs-corrupt-block.c
> @@ -726,7 +726,7 @@ out:
>   static void shift_items(struct btrfs_root *root, struct extent_buffer *eb)
>   {
>   	int nritems = btrfs_header_nritems(eb);
> -	int shift_space = btrfs_leaf_free_space(root->fs_info, eb) / 2;
> +	int shift_space = btrfs_leaf_free_space(eb) / 2;
>   	int slot = nritems / 2;
>   	int i = 0;
>   	unsigned int data_end = btrfs_item_offset_nr(eb, nritems - 1);
> diff --git a/check/main.c b/check/main.c
> index c4a1801fb0ef..2030fe663e1b 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -5970,7 +5970,7 @@ static int run_next_block(struct btrfs_root *root,
>   		goto out;
>   
>   	if (btrfs_is_leaf(buf)) {
> -		btree_space_waste += btrfs_leaf_free_space(fs_info, buf);
> +		btree_space_waste += btrfs_leaf_free_space(buf);
>   		for (i = 0; i < nritems; i++) {
>   			struct btrfs_file_extent_item *fi;
>   
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index bfe45abab6cc..227ecf87cf97 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -390,7 +390,7 @@ static void account_bytes(struct btrfs_root *root, struct btrfs_path *path,
>   		total_extent_tree_bytes += eb->len;
>   
>   	if (level == 0) {
> -		btree_space_waste += btrfs_leaf_free_space(root->fs_info, eb);
> +		btree_space_waste += btrfs_leaf_free_space(eb);
>   	} else {
>   		free_nrs = (BTRFS_NODEPTRS_PER_BLOCK(root->fs_info) -
>   			    btrfs_header_nritems(eb));
> diff --git a/ctree.c b/ctree.c
> index d1c41925d71c..2c3ba70b000c 100644
> --- a/ctree.c
> +++ b/ctree.c
> @@ -480,11 +480,11 @@ btrfs_check_leaf(struct btrfs_root *root, struct btrfs_disk_key *parent_key,
>   		       (unsigned long long)btrfs_header_bytenr(buf));
>   		goto fail;
>   	}
> -	if (btrfs_leaf_free_space(root->fs_info, buf) < 0) {
> +	if (btrfs_leaf_free_space(buf) < 0) {
>   		ret = BTRFS_TREE_BLOCK_INVALID_FREE_SPACE;
>   		fprintf(stderr, "leaf free space incorrect %llu %d\n",
>   			(unsigned long long)btrfs_header_bytenr(buf),
> -			btrfs_leaf_free_space(root->fs_info, buf));
> +			btrfs_leaf_free_space(buf));
>   		goto fail;
>   	}
>   
> @@ -1193,7 +1193,7 @@ again:
>   		} else {
>   			p->slots[level] = slot;
>   			if (ins_len > 0 &&
> -			    ins_len > btrfs_leaf_free_space(root->fs_info, b)) {
> +			    ins_len > btrfs_leaf_free_space(b)) {
>   				int sret = split_leaf(trans, root, key,
>   						      p, ins_len, ret == 0);
>   				BUG_ON(sret > 0);
> @@ -1634,17 +1634,19 @@ static int leaf_space_used(struct extent_buffer *l, int start, int nr)
>    * the start of the leaf data.  IOW, how much room
>    * the leaf has left for both items and data
>    */
> -int btrfs_leaf_free_space(struct btrfs_fs_info *fs_info,
> -			  struct extent_buffer *leaf)
> +int btrfs_leaf_free_space(struct extent_buffer *leaf)
>   {
>   	int nritems = btrfs_header_nritems(leaf);
> +	u32 leaf_data_size;
>   	int ret;
>   
> -	ret = BTRFS_LEAF_DATA_SIZE(fs_info) - leaf_space_used(leaf, 0, nritems);
> +	BUG_ON(leaf->fs_info && leaf->fs_info->nodesize != leaf->len);
> +	leaf_data_size = __BTRFS_LEAF_DATA_SIZE(leaf->len);
> +	ret = leaf_data_size - leaf_space_used(leaf, 0 ,nritems);
>   	if (ret < 0) {
> -		printk("leaf free space ret %d, leaf data size %lu, used %d nritems %d\n",
> -		       ret, BTRFS_LEAF_DATA_SIZE(fs_info),
> -		       leaf_space_used(leaf, 0, nritems), nritems);
> +		printk("leaf free space ret %d, leaf data size %u, used %d nritems %d\n",
> +		       ret, leaf_data_size, leaf_space_used(leaf, 0, nritems),
> +		       nritems);
>   	}
>   	return ret;
>   }
> @@ -1692,7 +1694,7 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root
>   			return PTR_ERR(right);
>   		return -EIO;
>   	}
> -	free_space = btrfs_leaf_free_space(fs_info, right);
> +	free_space = btrfs_leaf_free_space(right);
>   	if (free_space < data_size) {
>   		free_extent_buffer(right);
>   		return 1;
> @@ -1705,7 +1707,7 @@ static int push_leaf_right(struct btrfs_trans_handle *trans, struct btrfs_root
>   		free_extent_buffer(right);
>   		return 1;
>   	}
> -	free_space = btrfs_leaf_free_space(fs_info, right);
> +	free_space = btrfs_leaf_free_space(right);
>   	if (free_space < data_size) {
>   		free_extent_buffer(right);
>   		return 1;
> @@ -1844,7 +1846,7 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root
>   	}
>   
>   	left = read_node_slot(fs_info, path->nodes[1], slot - 1);
> -	free_space = btrfs_leaf_free_space(fs_info, left);
> +	free_space = btrfs_leaf_free_space(left);
>   	if (free_space < data_size) {
>   		free_extent_buffer(left);
>   		return 1;
> @@ -1859,7 +1861,7 @@ static int push_leaf_left(struct btrfs_trans_handle *trans, struct btrfs_root
>   		return 1;
>   	}
>   
> -	free_space = btrfs_leaf_free_space(fs_info, left);
> +	free_space = btrfs_leaf_free_space(left);
>   	if (free_space < data_size) {
>   		free_extent_buffer(left);
>   		return 1;
> @@ -2083,7 +2085,7 @@ static noinline int split_leaf(struct btrfs_trans_handle *trans,
>   		l = path->nodes[0];
>   
>   		/* did the pushes work? */
> -		if (btrfs_leaf_free_space(root->fs_info, l) >= data_size)
> +		if (btrfs_leaf_free_space(l) >= data_size)
>   			return 0;
>   	}
>   
> @@ -2240,7 +2242,7 @@ int btrfs_split_item(struct btrfs_trans_handle *trans,
>   
>   	leaf = path->nodes[0];
>   	btrfs_item_key_to_cpu(leaf, &orig_key, path->slots[0]);
> -	if (btrfs_leaf_free_space(root->fs_info, leaf) >=
> +	if (btrfs_leaf_free_space(leaf) >=
>   	    sizeof(struct btrfs_item))
>   		goto split;
>   
> @@ -2261,8 +2263,7 @@ int btrfs_split_item(struct btrfs_trans_handle *trans,
>   	ret = split_leaf(trans, root, &orig_key, path, 0, 0);
>   	BUG_ON(ret);
>   
> -	BUG_ON(btrfs_leaf_free_space(root->fs_info, leaf) <
> -	       sizeof(struct btrfs_item));
> +	BUG_ON(btrfs_leaf_free_space(leaf) < sizeof(struct btrfs_item));
>   	leaf = path->nodes[0];
>   
>   split:
> @@ -2314,7 +2315,7 @@ split:
>   	btrfs_mark_buffer_dirty(leaf);
>   
>   	ret = 0;
> -	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
> +	if (btrfs_leaf_free_space(leaf) < 0) {
>   		btrfs_print_leaf(leaf);
>   		BUG();
>   	}
> @@ -2410,7 +2411,7 @@ int btrfs_truncate_item(struct btrfs_root *root, struct btrfs_path *path,
>   	btrfs_mark_buffer_dirty(leaf);
>   
>   	ret = 0;
> -	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
> +	if (btrfs_leaf_free_space(leaf) < 0) {
>   		btrfs_print_leaf(leaf);
>   		BUG();
>   	}
> @@ -2435,7 +2436,7 @@ int btrfs_extend_item(struct btrfs_root *root, struct btrfs_path *path,
>   	nritems = btrfs_header_nritems(leaf);
>   	data_end = leaf_data_end(root->fs_info, leaf);
>   
> -	if (btrfs_leaf_free_space(root->fs_info, leaf) < data_size) {
> +	if (btrfs_leaf_free_space(leaf) < data_size) {
>   		btrfs_print_leaf(leaf);
>   		BUG();
>   	}
> @@ -2472,7 +2473,7 @@ int btrfs_extend_item(struct btrfs_root *root, struct btrfs_path *path,
>   	btrfs_mark_buffer_dirty(leaf);
>   
>   	ret = 0;
> -	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
> +	if (btrfs_leaf_free_space(leaf) < 0) {
>   		btrfs_print_leaf(leaf);
>   		BUG();
>   	}
> @@ -2521,10 +2522,10 @@ int btrfs_insert_empty_items(struct btrfs_trans_handle *trans,
>   	nritems = btrfs_header_nritems(leaf);
>   	data_end = leaf_data_end(root->fs_info, leaf);
>   
> -	if (btrfs_leaf_free_space(root->fs_info, leaf) < total_size) {
> +	if (btrfs_leaf_free_space(leaf) < total_size) {
>   		btrfs_print_leaf(leaf);
>   		printk("not enough freespace need %u have %d\n",
> -		       total_size, btrfs_leaf_free_space(root->fs_info, leaf));
> +		       total_size, btrfs_leaf_free_space(leaf));
>   		BUG();
>   	}
>   
> @@ -2582,7 +2583,7 @@ int btrfs_insert_empty_items(struct btrfs_trans_handle *trans,
>   		btrfs_fixup_low_keys(root, path, &disk_key, 1);
>   	}
>   
> -	if (btrfs_leaf_free_space(root->fs_info, leaf) < 0) {
> +	if (btrfs_leaf_free_space(leaf) < 0) {
>   		btrfs_print_leaf(leaf);
>   		BUG();
>   	}
> diff --git a/ctree.h b/ctree.h
> index 1fef37c93485..e291a7d48d14 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -2664,8 +2664,7 @@ static inline int btrfs_next_item(struct btrfs_root *root,
>   }
>   
>   int btrfs_prev_leaf(struct btrfs_root *root, struct btrfs_path *path);
> -int btrfs_leaf_free_space(struct btrfs_fs_info *fs_info,
> -			  struct extent_buffer *leaf);
> +int btrfs_leaf_free_space(struct extent_buffer *leaf);
>   void btrfs_fixup_low_keys(struct btrfs_root *root, struct btrfs_path *path,
>   			  struct btrfs_disk_key *key, int level);
>   int btrfs_set_item_key_safe(struct btrfs_root *root, struct btrfs_path *path,
> diff --git a/print-tree.c b/print-tree.c
> index a1a7954abdae..34a724d5f7ef 100644
> --- a/print-tree.c
> +++ b/print-tree.c
> @@ -1191,7 +1191,7 @@ void btrfs_print_leaf(struct extent_buffer *eb)
>   
>   	printf("leaf %llu items %d free space %d generation %llu owner ",
>   		(unsigned long long)btrfs_header_bytenr(eb), nr,
> -		btrfs_leaf_free_space(fs_info, eb),
> +		btrfs_leaf_free_space(eb),
>   		(unsigned long long)btrfs_header_generation(eb));
>   	print_objectid(stdout, btrfs_header_owner(eb), 0);
>   	printf("\n");
> 

Re: [PATCH 2/3] btrfs-progs: Allow tree to be printed without an fs_info

[Su Yue]

On 04/30/2018 11:15 AM, Qu Wenruo wrote:
> For btrfs_print_tree() and btrfs_print_leaf(), the usage of fs_info is
> mainly for nodesize and sectorsize.
> 
> However for nodesize, we can get it from @eb->len without the need for
> fs_info at all.
> 
> For nodesize, introduce new helper BTRFS_NODEPTR_PER_EXTENT_BUFFER() to
> get nodesize from @eb directly.
> And with the help of previous modified btrfs_leaf_free_space(),
> btrfs_print_tree() can live without fs_info at all.
> 
> For btrfs_print_leaf(), we modify print_extent_csum() to accept NULL
> fs_info by skipping csum length calculation.
> 
> With all these modification, btrfs_print_tree/leaf() can be called
> without accessing @fs_info at all, and make it more flex to handle
> binary tree block dump, or inside gdb.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Su Yue <suy.fnst@cn.fujitsu.com>

> ---
>   ctree.h      |  7 +++++++
>   print-tree.c | 15 +++++++++++++--
>   2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/ctree.h b/ctree.h
> index e291a7d48d14..80fd19612434 100644
> --- a/ctree.h
> +++ b/ctree.h
> @@ -1202,6 +1202,13 @@ static inline u32 BTRFS_NODEPTRS_PER_BLOCK(const struct btrfs_fs_info *info)
>   	return BTRFS_LEAF_DATA_SIZE(info) / sizeof(struct btrfs_key_ptr);
>   }
>   
> +static inline u32
> +BTRFS_NODEPTRS_PER_EXTENT_BUFFER(const struct extent_buffer *eb)
> +{
> +	BUG_ON(eb->fs_info && eb->fs_info->nodesize != eb->len);
> +	return __BTRFS_LEAF_DATA_SIZE(eb->len) / sizeof(struct btrfs_key_ptr);
> +}
> +
>   #define BTRFS_FILE_EXTENT_INLINE_DATA_START		\
>   	(offsetof(struct btrfs_file_extent_item, disk_bytenr))
>   static inline u32 BTRFS_MAX_INLINE_DATA_SIZE(const struct btrfs_fs_info *info)
> diff --git a/print-tree.c b/print-tree.c
> index 34a724d5f7ef..31a851ef4413 100644
> --- a/print-tree.c
> +++ b/print-tree.c
> @@ -1150,6 +1150,14 @@ static void print_extent_csum(struct extent_buffer *eb,
>   {
>   	u32 size;
>   
> +	/*
> +	 * If we don't have fs_info, only output its start position as we
> +	 * don't have sectorsize for the calculation
> +	 */
> +	if (!fs_info) {
> +		printf("\t\trange start %llu\n", (unsigned long long)start);
> +		return;
> +	}
>   	size = (item_size / btrfs_super_csum_size(fs_info->super_copy)) *
>   			fs_info->sectorsize;
>   	printf("\t\trange start %llu end %llu length %u\n",
> @@ -1371,7 +1379,7 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
>   	printf("node %llu level %d items %d free %u generation %llu owner ",
>   	       (unsigned long long)eb->start,
>   	        btrfs_header_level(eb), nr,
> -		(u32)BTRFS_NODEPTRS_PER_BLOCK(fs_info) - nr,
> +		(u32)BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb) - nr,
>   		(unsigned long long)btrfs_header_generation(eb));
>   	print_objectid(stdout, btrfs_header_owner(eb), 0);
>   	printf("\n");
> @@ -1385,13 +1393,16 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
>   		btrfs_print_key(&disk_key);
>   		printf(" block %llu (%llu) gen %llu\n",
>   		       (unsigned long long)blocknr,
> -		       (unsigned long long)blocknr / fs_info->nodesize,
> +		       (unsigned long long)blocknr / eb->len,
>   		       (unsigned long long)btrfs_node_ptr_generation(eb, i));
>   		fflush(stdout);
>   	}
>   	if (!follow)
>   		return;
>   
> +	if (follow && !fs_info)
> +		return;
> +
>   	for (i = 0; i < nr; i++) {
>   		next = read_tree_block(fs_info,
>   				btrfs_node_blockptr(eb, i),
> 

Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access

[Su Yue]

On 04/30/2018 11:15 AM, Qu Wenruo wrote:
> For btrfs_print_tree(), if nr_items is corrupted, it can easily go
> beyond extent buffer boundary.
> 
> Add extra nr_item check, and only print as many valid slots as possible.
> 

Make sense.

> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   print-tree.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/print-tree.c b/print-tree.c
> index 31a851ef4413..55db80bebb2a 100644
> --- a/print-tree.c
> +++ b/print-tree.c
> @@ -1376,6 +1376,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
>   		btrfs_print_leaf(eb);
>   		return;
>   	}
> +	/* We are crossing eb boundary, this node must be corrupted */
> +	if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
> +		warning(
> +		"node nr_items corrupted, has %u limit %u, continue print anyway",
> +			nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
>   	printf("node %llu level %d items %d free %u generation %llu owner ",
>   	       (unsigned long long)eb->start,
>   	        btrfs_header_level(eb), nr,
> @@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
>   	print_uuids(eb);
>   	fflush(stdout);
>   		
> -		u64 blocknr = btrfs_node_blockptr(eb, i);
> +		u64 blocknr;
> +
> +		if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
> +			break;

Should it be i >= BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb)?

Here BTRFS_NODEPTRS_PER_EXTENT_BUFFER() is called during iterations.
The judement can be calculated in advance like:

	ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
	...
	for (i = 0; i < nr && i < ptr_num  ; i++) {

Thanks,
Su

> +		blocknr = btrfs_node_blockptr(eb, i);
>   		btrfs_node_key(eb, &disk_key, i);
>   		btrfs_disk_key_to_cpu(&key, &disk_key);
>   		printf("\t");
> 

Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2608 bytes --]



On 2018年04月30日 11:49, Su Yue wrote:
> 
> 
> On 04/30/2018 11:15 AM, Qu Wenruo wrote:
>> For btrfs_print_tree(), if nr_items is corrupted, it can easily go
>> beyond extent buffer boundary.
>>
>> Add extra nr_item check, and only print as many valid slots as possible.
>>
> 
> Make sense.
> 
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   print-tree.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/print-tree.c b/print-tree.c
>> index 31a851ef4413..55db80bebb2a 100644
>> --- a/print-tree.c
>> +++ b/print-tree.c
>> @@ -1376,6 +1376,11 @@ void btrfs_print_tree(struct extent_buffer *eb,
>> int follow)
>>           btrfs_print_leaf(eb);
>>           return;
>>       }
>> +    /* We are crossing eb boundary, this node must be corrupted */
>> +    if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
>> +        warning(
>> +        "node nr_items corrupted, has %u limit %u, continue print
>> anyway",
>> +            nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
>>       printf("node %llu level %d items %d free %u generation %llu
>> owner ",
>>              (unsigned long long)eb->start,
>>               btrfs_header_level(eb), nr,
>> @@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb,
>> int follow)
>>       print_uuids(eb);
>>       fflush(stdout);
>>          
>> -        u64 blocknr = btrfs_node_blockptr(eb, i);
>> +        u64 blocknr;
>> +
>> +        if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
>> +            break;
> 
> Should it be i >= BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb)?

BTRFS_NODEPTRS_PER_EXTENT_BUFFER() provides the maximum valid number.
So it 's >=.

> 
> Here BTRFS_NODEPTRS_PER_EXTENT_BUFFER() is called during iterations.
> The judement can be calculated in advance like:
> 
>     ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
>     ...
>     for (i = 0; i < nr && i < ptr_num  ; i++) {

Indeed looks better.

Thanks,
Qu

> 
> Thanks,
> Su
> 
>> +        blocknr = btrfs_node_blockptr(eb, i);
>>           btrfs_node_key(eb, &disk_key, i);
>>           btrfs_disk_key_to_cpu(&key, &disk_key);
>>           printf("\t");
>>
> 
> 
> -- 
> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access

[David Sterba]

On Mon, Apr 30, 2018 at 11:51:19AM +0800, Qu Wenruo wrote:
> >>           btrfs_print_leaf(eb);
> >>           return;
> >>       }
> >> +    /* We are crossing eb boundary, this node must be corrupted */
> >> +    if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
> >> +        warning(
> >> +        "node nr_items corrupted, has %u limit %u, continue print
> >> anyway",
> >> +            nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
> >>       printf("node %llu level %d items %d free %u generation %llu
> >> owner ",
> >>              (unsigned long long)eb->start,
> >>               btrfs_header_level(eb), nr,
> >> @@ -1386,7 +1391,11 @@ void btrfs_print_tree(struct extent_buffer *eb,
> >> int follow)
> >>       print_uuids(eb);
> >>       fflush(stdout);
> >>          
> >> -        u64 blocknr = btrfs_node_blockptr(eb, i);
> >> +        u64 blocknr;
> >> +
> >> +        if (i > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
> >> +            break;
> > 
> > Should it be i >= BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb)?
> 
> BTRFS_NODEPTRS_PER_EXTENT_BUFFER() provides the maximum valid number.
> So it 's >=.
> 
> > 
> > Here BTRFS_NODEPTRS_PER_EXTENT_BUFFER() is called during iterations.
> > The judement can be calculated in advance like:
> > 
> >     ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
> >     ...
> >     for (i = 0; i < nr && i < ptr_num  ; i++) {
> 
> Indeed looks better.

Please resend this patch with the suggested updates, thanks.

Re: [PATCH 1/3] btrfs-progs: Remove fs_info parameter from btrfs_leaf_free_space()

[David Sterba]

On Mon, Apr 30, 2018 at 11:15:43AM +0800, Qu Wenruo wrote:
> For btrfs_leaf_free_space(), to get leaf data size, we have two way to
> get it:
> 
> 1) leaf->fs_info->nodesize
> 2) leaf->len
> 
> Anyway, we could get rid of @fs_info parameter for
> btrfs_leaf_free_space().
> And here we choose method 2), as it provides extra benefit to get leaf
> free space without initializing a real fs_info.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

1 and 2 applied, 3 has some comments, please resend.

[PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access

[Qu Wenruo]

For btrfs_print_tree(), if nr_items is corrupted, it can easily go
beyond extent buffer boundary.

Add extra nr_item check, and only print as many valid slots as possible.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v2:
  Use better loop condition suggested by Su.
---
 print-tree.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/print-tree.c b/print-tree.c
index 31a851ef4413..1c2533a9678d 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -1364,6 +1364,7 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
 {
 	u32 i;
 	u32 nr;
+	u32 ptr_num;
 	struct btrfs_fs_info *fs_info = eb->fs_info;
 	struct btrfs_disk_key disk_key;
 	struct btrfs_key key;
@@ -1376,6 +1377,11 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
 		btrfs_print_leaf(eb);
 		return;
 	}
+	/* We are crossing eb boundary, this node must be corrupted */
+	if (nr > BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb))
+		warning(
+		"node nr_items corrupted, has %u limit %u, continue print anyway",
+			nr, BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb));
 	printf("node %llu level %d items %d free %u generation %llu owner ",
 	       (unsigned long long)eb->start,
 	        btrfs_header_level(eb), nr,
@@ -1385,8 +1391,10 @@ void btrfs_print_tree(struct extent_buffer *eb, int follow)
 	printf("\n");
 	print_uuids(eb);
 	fflush(stdout);
-	for (i = 0; i < nr; i++) {
+	ptr_num = BTRFS_NODEPTRS_PER_EXTENT_BUFFER(eb);
+	for (i = 0; i < nr && i < ptr_num; i++) {
 		u64 blocknr = btrfs_node_blockptr(eb, i);
+
 		btrfs_node_key(eb, &disk_key, i);
 		btrfs_disk_key_to_cpu(&key, &disk_key);
 		printf("\t");
-- 
2.17.0

Re: [PATCH 3/3] btrfs-progs: print-tree: Enhance btrfs_print_tree() check to avoid out-of-boundary memory access

[David Sterba]

On Thu, May 10, 2018 at 09:50:01AM +0800, Qu Wenruo wrote:
> For btrfs_print_tree(), if nr_items is corrupted, it can easily go
> beyond extent buffer boundary.
> 
> Add extra nr_item check, and only print as many valid slots as possible.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Applied, thanks.