[PATCH] btrfs-progs: check for invalid free space tree entries

[PATCH] btrfs-progs: check for invalid free space tree entries

[Josef Bacik]

While testing some changes to how we reclaim block groups I started
hitting failures with my TEST_DEV.  This occurred because I had a bug
and failed to properly remove a block groups free space tree entries.
However this wasn't caught in testing when it happened because
btrfs check only checks that the free space cache for the existing block
groups is valid, it doesn't check for free space entries that don't have
a corresponding block group.

Fix this by checking for free space entries that don't have a
corresponding block group.  Additionally add a test image to validate
this fix.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 check/main.c                                  |  79 ++++++++++++++++++
 .../corrupt-free-space-tree.img.xz            | Bin 0 -> 1368 bytes
 2 files changed, 79 insertions(+)
 create mode 100644 tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz

diff --git a/check/main.c b/check/main.c
index 4f7ab8b2..fb119bfe 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5729,6 +5729,83 @@ static int verify_space_cache(struct btrfs_root *root,
 	return ret;
 }
 
+static int check_free_space_tree(struct btrfs_root *root)
+{
+	struct btrfs_key key = {};
+	struct btrfs_path *path;
+	int ret = 0;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	while (1) {
+		struct btrfs_block_group *bg;
+		u64 cur_start = key.objectid;
+
+		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+		if (ret < 0)
+			goto out;
+
+		/*
+		 * We should be landing on an item, so if we're above the
+		 * nritems we know we hit the end of the tree.
+		 */
+		if (path->slots[0] >= btrfs_header_nritems(path->nodes[0]))
+			break;
+
+		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+
+		if (key.type != BTRFS_FREE_SPACE_INFO_KEY) {
+			fprintf(stderr, "Failed to find a space info key at %llu [%llu %u %llu]\n",
+				cur_start, key.objectid, key.type, key.offset);
+			ret = -EINVAL;
+			goto out;
+		}
+
+		bg = btrfs_lookup_first_block_group(gfs_info, key.objectid);
+		if (!bg) {
+			fprintf(stderr, "We have a space info key for a block group that doesn't exist\n");
+			ret = -EINVAL;
+			goto out;
+		}
+
+		btrfs_release_path(path);
+		key.objectid += key.offset;
+		key.offset = 0;
+	}
+	ret = 0;
+out:
+	btrfs_free_path(path);
+	return ret;
+}
+
+static int check_free_space_trees(struct btrfs_root *root)
+{
+	struct btrfs_root *free_space_root;
+	struct rb_node *n;
+	struct btrfs_key key = {
+		.objectid = BTRFS_FREE_SPACE_TREE_OBJECTID,
+		.type = BTRFS_ROOT_ITEM_KEY,
+		.offset = 0,
+	};
+	int ret = 0;
+
+	free_space_root = btrfs_global_root(gfs_info, &key);
+	while (1) {
+		ret = check_free_space_tree(free_space_root);
+		if (ret)
+			break;
+		n = rb_next(&root->rb_node);
+		if (!n)
+			break;
+		free_space_root = rb_entry(n, struct btrfs_root, rb_node);
+		if (root->root_key.objectid != BTRFS_FREE_SPACE_TREE_OBJECTID)
+			break;
+	}
+	return ret;
+}
+
 static int check_space_cache(struct btrfs_root *root)
 {
 	struct extent_io_tree used;
@@ -10121,6 +10198,8 @@ static int validate_free_space_cache(struct btrfs_root *root)
 	}
 
 	ret = check_space_cache(root);
+	if (!ret && btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE))
+		ret = check_free_space_trees(root);
 	if (ret && btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE) &&
 	    repair) {
 		ret = do_clear_free_space_cache(2);
diff --git a/tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz b/tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz
new file mode 100644
index 0000000000000000000000000000000000000000..93280b82749c87183c8222740f15f6abca3d9bbf
GIT binary patch
literal 1368
zcmV-e1*iJ`H+ooF000E$*0e?f03iVu0001VFXf}+Q~w1OT>wRyj;C3^v%$$4d1ocf
zjjaF1$3^@a-*Xj3r-#5<9#Wqg%5NYWtuFyaX3{;HIsTkK=A=aNB3|l5$%+=o8g1{`
z9>n_MeGBmNWMc2O8%=OwNuQkT%DC=u6%>3ias*)m2w2Vj{<X`j2OnKGMEZ%xR1cy`
zv?-{L(P2Vkw0p<|MXz$0(HMoM8$Cs<!9}Ebp|h0L_u(K6y=q5|H5~9~lfr>K%U40+
z_*HaH3>s3Be3<P8!YiHA`cw6c01&vER1Oyfl&3F9`o@wse1epc9c+bHS%moHfGbi0
za>G|qceZM^9=!MAA{x7B*DdZw`=MZbP>g&&UJ78w*_xFJ*ymY);<Mbk211PVqr9pY
z2$a*{$ddCoWl0v${~c<P0kv>=&F%~Em~&)LIgI!eG5|{o17mJxTHbL0LzMS}X7Rn(
z8aG+sA}om=lMvs%yH^30B*fHO+M3JW%myi2Q@oISpjzVQL->PHN#F;kkTDP9m3d&G
zATJRzI9%32{Q#bQRBPs+qO=%P(RJuP|KXmbBKz&TwxJq`c>bGV$H_w?rL#W~CBA&Y
zTVrF&)Lgdkg=t8HuJHV%5gY<|+kT6=fq0y7i5Dq~A`8gp{)hlOigUZY*~kzbW*nyy
zPy`p*ZYH)a7M@cM%W+<aS#c!NBv6M#k&B?|Gp>@{OMl<t$~kLNW5L+}or+3uMfZCa
zPdfQpi-M`W(}|V|bGaH}H$OWtV5FtcB!t8htp{EN<x2hz!>(-bc0W+2Ou$DqMhHg0
zAXXfJ)lD^OQ+ZsozgJ|ED^p@%Xk(nRRA=1*@@0`G!v_f=eu`$4g=+OcYHd?o<s{k<
zxnc&Y__V)N3e2C_ckEzS<C+G7Nmfx90n%C0*tw3Kx?CrF*A4nznbUo#(uhw;yi{b&
zUFA$8;MagqpNk9<F7lTE&f5YUW5zkYknyP}6_&g7maa>n#o^)Go`Vsk4wWALj%6xo
zaptcSD`V9YyHG^8)iRj!Joyn!>AZaZYq|%`TqWRNiYO4%%UH-fOt<QXMD?{|hGUbp
zrYY)<e8>}&<1eT_XhMooY9PF)hWGE81Aws&1!uWAsqM;Ila>-d{V}&$g0ds)zg&5W
zy@oE>Vku@9rAuL@*wgdz7LuW?v&?fv^VEo(ELFU8)sSplTO|-_Q>g)KAl2D8@Wv4Y
zy}hHu5?eK*DQHnJhNk1<@fb?xFkdSAlPxV;TKl{n;f9EkWjJt73KuO}-qG&G+*R)4
zhwOeTHj!?vZ|(K2a(js@blT`!xJhTI{G8PmIgngOpf5HKr4%f_C@7a5O$XsR>`ph3
zJz=xXUxXe6<DAbu*W&a8RFF;`p>>7gvsh$Ew3X_(ssfEfMDkER1cYqYxN3}-w)g0`
z`SCbd(vQevQ;+U<8TtQb{OE;fs!)YC&?A1H9|)Br;s1wxo&K&9?UjEYW|mKVQq6tH
zm(NruvFH7=%Q?*I)g6tb0-kV?<>qj-__Hc`D^QsGKI9`d+nL^1g*N?EU%c~9&I=R?
zBW6edh^W^U!u{1k%mq`Fcp#QU4}(<wxlpXBWvSX#&huX4Ijqe+-iDRro04Xy+{kR)
z(^+iDSztUr)`280d<1=uO{~1UM1%4>BZvQQ4zC#B;#@cKzqPk)OV4xGUgcG@RzDoN
zvL!%GwW?EsbyiH<;7;8+b{X<zmZ8HfcL|V@C=a>Xdph+>?Zp590000pn{%NX14{`2
a0kH~zs0jcD4TH0>#Ao{g000001X)^gHlHK_

literal 0
HcmV?d00001

-- 
2.26.3

Re: [PATCH] btrfs-progs: check for invalid free space tree entries

[Qu Wenruo]

On 2022/7/30 05:08, Josef Bacik wrote:
> While testing some changes to how we reclaim block groups I started
> hitting failures with my TEST_DEV.  This occurred because I had a bug
> and failed to properly remove a block groups free space tree entries.
> However this wasn't caught in testing when it happened because
> btrfs check only checks that the free space cache for the existing block
> groups is valid, it doesn't check for free space entries that don't have
> a corresponding block group.
>
> Fix this by checking for free space entries that don't have a
> corresponding block group.  Additionally add a test image to validate
> this fix.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   check/main.c                                  |  79 ++++++++++++++++++
>   .../corrupt-free-space-tree.img.xz            | Bin 0 -> 1368 bytes
>   2 files changed, 79 insertions(+)
>   create mode 100644 tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz
>
> diff --git a/check/main.c b/check/main.c
> index 4f7ab8b2..fb119bfe 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -5729,6 +5729,83 @@ static int verify_space_cache(struct btrfs_root *root,
>   	return ret;
>   }
>
> +static int check_free_space_tree(struct btrfs_root *root)
> +{
> +	struct btrfs_key key = {};
> +	struct btrfs_path *path;
> +	int ret = 0;
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +
> +	while (1) {
> +		struct btrfs_block_group *bg;
> +		u64 cur_start = key.objectid;
> +
> +		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +		if (ret < 0)
> +			goto out;
> +
> +		/*
> +		 * We should be landing on an item, so if we're above the
> +		 * nritems we know we hit the end of the tree.
> +		 */
> +		if (path->slots[0] >= btrfs_header_nritems(path->nodes[0]))
> +			break;
> +
> +		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +
> +		if (key.type != BTRFS_FREE_SPACE_INFO_KEY) {
> +			fprintf(stderr, "Failed to find a space info key at %llu [%llu %u %llu]\n",
> +				cur_start, key.objectid, key.type, key.offset);
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		bg = btrfs_lookup_first_block_group(gfs_info, key.objectid);
> +		if (!bg) {
> +			fprintf(stderr, "We have a space info key for a block group that doesn't exist\n");
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		btrfs_release_path(path);
> +		key.objectid += key.offset;
> +		key.offset = 0;
> +	}
> +	ret = 0;
> +out:
> +	btrfs_free_path(path);
> +	return ret;
> +}
> +
> +static int check_free_space_trees(struct btrfs_root *root)
> +{
> +	struct btrfs_root *free_space_root;
> +	struct rb_node *n;
> +	struct btrfs_key key = {
> +		.objectid = BTRFS_FREE_SPACE_TREE_OBJECTID,
> +		.type = BTRFS_ROOT_ITEM_KEY,
> +		.offset = 0,
> +	};
> +	int ret = 0;
> +
> +	free_space_root = btrfs_global_root(gfs_info, &key);
> +	while (1) {
> +		ret = check_free_space_tree(free_space_root);
> +		if (ret)
> +			break;
> +		n = rb_next(&root->rb_node);
> +		if (!n)
> +			break;
> +		free_space_root = rb_entry(n, struct btrfs_root, rb_node);
> +		if (root->root_key.objectid != BTRFS_FREE_SPACE_TREE_OBJECTID)
> +			break;
> +	}
> +	return ret;
> +}
> +
>   static int check_space_cache(struct btrfs_root *root)
>   {
>   	struct extent_io_tree used;
> @@ -10121,6 +10198,8 @@ static int validate_free_space_cache(struct btrfs_root *root)
>   	}
>
>   	ret = check_space_cache(root);
> +	if (!ret && btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE))
> +		ret = check_free_space_trees(root);
>   	if (ret && btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE) &&
>   	    repair) {
>   		ret = do_clear_free_space_cache(2);
> diff --git a/tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz b/tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz
> new file mode 100644
> index 0000000000000000000000000000000000000000..93280b82749c87183c8222740f15f6abca3d9bbf
> GIT binary patch
> literal 1368
> zcmV-e1*iJ`H+ooF000E$*0e?f03iVu0001VFXf}+Q~w1OT>wRyj;C3^v%$$4d1ocf
> zjjaF1$3^@a-*Xj3r-#5<9#Wqg%5NYWtuFyaX3{;HIsTkK=A=aNB3|l5$%+=o8g1{`
> z9>n_MeGBmNWMc2O8%=OwNuQkT%DC=u6%>3ias*)m2w2Vj{<X`j2OnKGMEZ%xR1cy`
> zv?-{L(P2Vkw0p<|MXz$0(HMoM8$Cs<!9}Ebp|h0L_u(K6y=q5|H5~9~lfr>K%U40+
> z_*HaH3>s3Be3<P8!YiHA`cw6c01&vER1Oyfl&3F9`o@wse1epc9c+bHS%moHfGbi0
> za>G|qceZM^9=!MAA{x7B*DdZw`=MZbP>g&&UJ78w*_xFJ*ymY);<Mbk211PVqr9pY
> z2$a*{$ddCoWl0v${~c<P0kv>=&F%~Em~&)LIgI!eG5|{o17mJxTHbL0LzMS}X7Rn(
> z8aG+sA}om=lMvs%yH^30B*fHO+M3JW%myi2Q@oISpjzVQL->PHN#F;kkTDP9m3d&G
> zATJRzI9%32{Q#bQRBPs+qO=%P(RJuP|KXmbBKz&TwxJq`c>bGV$H_w?rL#W~CBA&Y
> zTVrF&)Lgdkg=t8HuJHV%5gY<|+kT6=fq0y7i5Dq~A`8gp{)hlOigUZY*~kzbW*nyy
> zPy`p*ZYH)a7M@cM%W+<aS#c!NBv6M#k&B?|Gp>@{OMl<t$~kLNW5L+}or+3uMfZCa
> zPdfQpi-M`W(}|V|bGaH}H$OWtV5FtcB!t8htp{EN<x2hz!>(-bc0W+2Ou$DqMhHg0
> zAXXfJ)lD^OQ+ZsozgJ|ED^p@%Xk(nRRA=1*@@0`G!v_f=eu`$4g=+OcYHd?o<s{k<
> zxnc&Y__V)N3e2C_ckEzS<C+G7Nmfx90n%C0*tw3Kx?CrF*A4nznbUo#(uhw;yi{b&
> zUFA$8;MagqpNk9<F7lTE&f5YUW5zkYknyP}6_&g7maa>n#o^)Go`Vsk4wWALj%6xo
> zaptcSD`V9YyHG^8)iRj!Joyn!>AZaZYq|%`TqWRNiYO4%%UH-fOt<QXMD?{|hGUbp
> zrYY)<e8>}&<1eT_XhMooY9PF)hWGE81Aws&1!uWAsqM;Ila>-d{V}&$g0ds)zg&5W
> zy@oE>Vku@9rAuL@*wgdz7LuW?v&?fv^VEo(ELFU8)sSplTO|-_Q>g)KAl2D8@Wv4Y
> zy}hHu5?eK*DQHnJhNk1<@fb?xFkdSAlPxV;TKl{n;f9EkWjJt73KuO}-qG&G+*R)4
> zhwOeTHj!?vZ|(K2a(js@blT`!xJhTI{G8PmIgngOpf5HKr4%f_C@7a5O$XsR>`ph3
> zJz=xXUxXe6<DAbu*W&a8RFF;`p>>7gvsh$Ew3X_(ssfEfMDkER1cYqYxN3}-w)g0`
> z`SCbd(vQevQ;+U<8TtQb{OE;fs!)YC&?A1H9|)Br;s1wxo&K&9?UjEYW|mKVQq6tH
> zm(NruvFH7=%Q?*I)g6tb0-kV?<>qj-__Hc`D^QsGKI9`d+nL^1g*N?EU%c~9&I=R?
> zBW6edh^W^U!u{1k%mq`Fcp#QU4}(<wxlpXBWvSX#&huX4Ijqe+-iDRro04Xy+{kR)
> z(^+iDSztUr)`280d<1=uO{~1UM1%4>BZvQQ4zC#B;#@cKzqPk)OV4xGUgcG@RzDoN
> zvL!%GwW?EsbyiH<;7;8+b{X<zmZ8HfcL|V@C=a>Xdph+>?Zp590000pn{%NX14{`2
> a0kH~zs0jcD4TH0>#Ao{g000001X)^gHlHK_
>
> literal 0
> HcmV?d00001
>

Re: [PATCH] btrfs-progs: check for invalid free space tree entries

[David Sterba]

On Fri, Jul 29, 2022 at 05:08:53PM -0400, Josef Bacik wrote:
> While testing some changes to how we reclaim block groups I started
> hitting failures with my TEST_DEV.  This occurred because I had a bug
> and failed to properly remove a block groups free space tree entries.
> However this wasn't caught in testing when it happened because
> btrfs check only checks that the free space cache for the existing block
> groups is valid, it doesn't check for free space entries that don't have
> a corresponding block group.
> 
> Fix this by checking for free space entries that don't have a
> corresponding block group.  Additionally add a test image to validate
> this fix.
> 
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Added to devel, thanks.

Re: [PATCH] btrfs-progs: check for invalid free space tree entries

[Qu Wenruo]

On 2022/7/30 05:08, Josef Bacik wrote:
> While testing some changes to how we reclaim block groups I started
> hitting failures with my TEST_DEV.  This occurred because I had a bug
> and failed to properly remove a block groups free space tree entries.
> However this wasn't caught in testing when it happened because
> btrfs check only checks that the free space cache for the existing block
> groups is valid, it doesn't check for free space entries that don't have
> a corresponding block group.
>
> Fix this by checking for free space entries that don't have a
> corresponding block group.  Additionally add a test image to validate
> this fix.

David, something is wrong with the patch in upstream, but not in the
patch in the mailing list.

>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>   check/main.c                                  |  79 ++++++++++++++++++
>   .../corrupt-free-space-tree.img.xz            | Bin 0 -> 1368 bytes
>   2 files changed, 79 insertions(+)
>   create mode 100644 tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz
>
> diff --git a/check/main.c b/check/main.c
> index 4f7ab8b2..fb119bfe 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -5729,6 +5729,83 @@ static int verify_space_cache(struct btrfs_root *root,
>   	return ret;
>   }
>
> +static int check_free_space_tree(struct btrfs_root *root)
> +{
> +	struct btrfs_key key = {};
> +	struct btrfs_path *path;
> +	int ret = 0;
> +
> +	path = btrfs_alloc_path();
> +	if (!path)
> +		return -ENOMEM;
> +
> +	while (1) {
> +		struct btrfs_block_group *bg;
> +		u64 cur_start = key.objectid;
> +
> +		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> +		if (ret < 0)
> +			goto out;

Here, the original code just goto out tag, and there we call
btrfs_free_path() and free everything.

But in devel branch, we're using on-stack path, and out tag no longer
has the btrfs_release_path(), but just return.

This leaks the path, causing every valid btrfs to cause eb leakage:

[adam@btrfs-vm btrfs-progs]$ ./btrfs check ~/test.img
Opening filesystem to check...
Checking filesystem on /home/adam/test.img
UUID: 956ba692-d951-4c63-8c50-73541d01a6f3
[1/7] checking root items
[2/7] checking extents
[3/7] checking free space tree
[4/7] checking fs roots
[5/7] checking only csums items (without verifying data)
[6/7] checking root refs
[7/7] checking quota groups skipped (not enabled on this FS)
found 16461824 bytes used, no error found
total csum bytes: 13836
total tree bytes: 2293760
total fs tree bytes: 2129920
total extent tree bytes: 65536
btree space waste bytes: 441740
file data blocks allocated: 14168064
  referenced 14168064
extent buffer leak: start 33062912 len 16384

In my case, above eb is the fst root node.

I'll send out a fix for it since it's already in the v5.19 release.

Thanks,
Qu
> +
> +		/*
> +		 * We should be landing on an item, so if we're above the
> +		 * nritems we know we hit the end of the tree.
> +		 */
> +		if (path->slots[0] >= btrfs_header_nritems(path->nodes[0]))
> +			break;
> +
> +		btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
> +
> +		if (key.type != BTRFS_FREE_SPACE_INFO_KEY) {
> +			fprintf(stderr, "Failed to find a space info key at %llu [%llu %u %llu]\n",
> +				cur_start, key.objectid, key.type, key.offset);
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		bg = btrfs_lookup_first_block_group(gfs_info, key.objectid);
> +		if (!bg) {
> +			fprintf(stderr, "We have a space info key for a block group that doesn't exist\n");
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
> +		btrfs_release_path(path);
> +		key.objectid += key.offset;
> +		key.offset = 0;
> +	}
> +	ret = 0;
> +out:
> +	btrfs_free_path(path);
> +	return ret;
> +}
> +
> +static int check_free_space_trees(struct btrfs_root *root)
> +{
> +	struct btrfs_root *free_space_root;
> +	struct rb_node *n;
> +	struct btrfs_key key = {
> +		.objectid = BTRFS_FREE_SPACE_TREE_OBJECTID,
> +		.type = BTRFS_ROOT_ITEM_KEY,
> +		.offset = 0,
> +	};
> +	int ret = 0;
> +
> +	free_space_root = btrfs_global_root(gfs_info, &key);
> +	while (1) {
> +		ret = check_free_space_tree(free_space_root);
> +		if (ret)
> +			break;
> +		n = rb_next(&root->rb_node);
> +		if (!n)
> +			break;
> +		free_space_root = rb_entry(n, struct btrfs_root, rb_node);
> +		if (root->root_key.objectid != BTRFS_FREE_SPACE_TREE_OBJECTID)
> +			break;
> +	}
> +	return ret;
> +}
> +
>   static int check_space_cache(struct btrfs_root *root)
>   {
>   	struct extent_io_tree used;
> @@ -10121,6 +10198,8 @@ static int validate_free_space_cache(struct btrfs_root *root)
>   	}
>
>   	ret = check_space_cache(root);
> +	if (!ret && btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE))
> +		ret = check_free_space_trees(root);
>   	if (ret && btrfs_fs_compat_ro(gfs_info, FREE_SPACE_TREE) &&
>   	    repair) {
>   		ret = do_clear_free_space_cache(2);
> diff --git a/tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz b/tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz
> new file mode 100644
> index 0000000000000000000000000000000000000000..93280b82749c87183c8222740f15f6abca3d9bbf
> GIT binary patch
> literal 1368
> zcmV-e1*iJ`H+ooF000E$*0e?f03iVu0001VFXf}+Q~w1OT>wRyj;C3^v%$$4d1ocf
> zjjaF1$3^@a-*Xj3r-#5<9#Wqg%5NYWtuFyaX3{;HIsTkK=A=aNB3|l5$%+=o8g1{`
> z9>n_MeGBmNWMc2O8%=OwNuQkT%DC=u6%>3ias*)m2w2Vj{<X`j2OnKGMEZ%xR1cy`
> zv?-{L(P2Vkw0p<|MXz$0(HMoM8$Cs<!9}Ebp|h0L_u(K6y=q5|H5~9~lfr>K%U40+
> z_*HaH3>s3Be3<P8!YiHA`cw6c01&vER1Oyfl&3F9`o@wse1epc9c+bHS%moHfGbi0
> za>G|qceZM^9=!MAA{x7B*DdZw`=MZbP>g&&UJ78w*_xFJ*ymY);<Mbk211PVqr9pY
> z2$a*{$ddCoWl0v${~c<P0kv>=&F%~Em~&)LIgI!eG5|{o17mJxTHbL0LzMS}X7Rn(
> z8aG+sA}om=lMvs%yH^30B*fHO+M3JW%myi2Q@oISpjzVQL->PHN#F;kkTDP9m3d&G
> zATJRzI9%32{Q#bQRBPs+qO=%P(RJuP|KXmbBKz&TwxJq`c>bGV$H_w?rL#W~CBA&Y
> zTVrF&)Lgdkg=t8HuJHV%5gY<|+kT6=fq0y7i5Dq~A`8gp{)hlOigUZY*~kzbW*nyy
> zPy`p*ZYH)a7M@cM%W+<aS#c!NBv6M#k&B?|Gp>@{OMl<t$~kLNW5L+}or+3uMfZCa
> zPdfQpi-M`W(}|V|bGaH}H$OWtV5FtcB!t8htp{EN<x2hz!>(-bc0W+2Ou$DqMhHg0
> zAXXfJ)lD^OQ+ZsozgJ|ED^p@%Xk(nRRA=1*@@0`G!v_f=eu`$4g=+OcYHd?o<s{k<
> zxnc&Y__V)N3e2C_ckEzS<C+G7Nmfx90n%C0*tw3Kx?CrF*A4nznbUo#(uhw;yi{b&
> zUFA$8;MagqpNk9<F7lTE&f5YUW5zkYknyP}6_&g7maa>n#o^)Go`Vsk4wWALj%6xo
> zaptcSD`V9YyHG^8)iRj!Joyn!>AZaZYq|%`TqWRNiYO4%%UH-fOt<QXMD?{|hGUbp
> zrYY)<e8>}&<1eT_XhMooY9PF)hWGE81Aws&1!uWAsqM;Ila>-d{V}&$g0ds)zg&5W
> zy@oE>Vku@9rAuL@*wgdz7LuW?v&?fv^VEo(ELFU8)sSplTO|-_Q>g)KAl2D8@Wv4Y
> zy}hHu5?eK*DQHnJhNk1<@fb?xFkdSAlPxV;TKl{n;f9EkWjJt73KuO}-qG&G+*R)4
> zhwOeTHj!?vZ|(K2a(js@blT`!xJhTI{G8PmIgngOpf5HKr4%f_C@7a5O$XsR>`ph3
> zJz=xXUxXe6<DAbu*W&a8RFF;`p>>7gvsh$Ew3X_(ssfEfMDkER1cYqYxN3}-w)g0`
> z`SCbd(vQevQ;+U<8TtQb{OE;fs!)YC&?A1H9|)Br;s1wxo&K&9?UjEYW|mKVQq6tH
> zm(NruvFH7=%Q?*I)g6tb0-kV?<>qj-__Hc`D^QsGKI9`d+nL^1g*N?EU%c~9&I=R?
> zBW6edh^W^U!u{1k%mq`Fcp#QU4}(<wxlpXBWvSX#&huX4Ijqe+-iDRro04Xy+{kR)
> z(^+iDSztUr)`280d<1=uO{~1UM1%4>BZvQQ4zC#B;#@cKzqPk)OV4xGUgcG@RzDoN
> zvL!%GwW?EsbyiH<;7;8+b{X<zmZ8HfcL|V@C=a>Xdph+>?Zp590000pn{%NX14{`2
> a0kH~zs0jcD4TH0>#Ao{g000001X)^gHlHK_
>
> literal 0
> HcmV?d00001
>

Re: [PATCH] btrfs-progs: check for invalid free space tree entries

[David Sterba]

On Tue, Aug 30, 2022 at 12:46:53PM +0800, Qu Wenruo wrote:
> 
> 
> On 2022/7/30 05:08, Josef Bacik wrote:
> > While testing some changes to how we reclaim block groups I started
> > hitting failures with my TEST_DEV.  This occurred because I had a bug
> > and failed to properly remove a block groups free space tree entries.
> > However this wasn't caught in testing when it happened because
> > btrfs check only checks that the free space cache for the existing block
> > groups is valid, it doesn't check for free space entries that don't have
> > a corresponding block group.
> >
> > Fix this by checking for free space entries that don't have a
> > corresponding block group.  Additionally add a test image to validate
> > this fix.
> 
> David, something is wrong with the patch in upstream, but not in the
> patch in the mailing list.
> 
> >
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> > ---
> >   check/main.c                                  |  79 ++++++++++++++++++
> >   .../corrupt-free-space-tree.img.xz            | Bin 0 -> 1368 bytes
> >   2 files changed, 79 insertions(+)
> >   create mode 100644 tests/fsck-tests/058-bad-free-space-tree-entry/corrupt-free-space-tree.img.xz
> >
> > diff --git a/check/main.c b/check/main.c
> > index 4f7ab8b2..fb119bfe 100644
> > --- a/check/main.c
> > +++ b/check/main.c
> > @@ -5729,6 +5729,83 @@ static int verify_space_cache(struct btrfs_root *root,
> >   	return ret;
> >   }
> >
> > +static int check_free_space_tree(struct btrfs_root *root)
> > +{
> > +	struct btrfs_key key = {};
> > +	struct btrfs_path *path;
> > +	int ret = 0;
> > +
> > +	path = btrfs_alloc_path();
> > +	if (!path)
> > +		return -ENOMEM;
> > +
> > +	while (1) {
> > +		struct btrfs_block_group *bg;
> > +		u64 cur_start = key.objectid;
> > +
> > +		ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
> > +		if (ret < 0)
> > +			goto out;
> 
> Here, the original code just goto out tag, and there we call
> btrfs_free_path() and free everything.
> 
> But in devel branch, we're using on-stack path, and out tag no longer
> has the btrfs_release_path(), but just return.

I did the conversion from allocated to on-stack variable and caused the
bug, tanks for catching it.