[PATCH 0/2] btrfs-progs: enhance error handling of __free_extent()

[PATCH 0/2] btrfs-progs: enhance error handling of __free_extent()

[Qu Wenruo]

There is a github bug report about btrfs-convert crash, where
btrfs_print_leaf() is called on NULL path->nodes[0].

The first patch fix the bug by cross-port a fix from kernel part.

The second patch refactor the error handling of __free_extent(), mostly
follow the kernel patch "btrfs: refactor the error handling of __btrfs_free_extent()".

Qu Wenruo (2):
  btrfs-progs: do not dump leaf if the path is released inside
    __free_extent()
  btrfs-progs: btrfs: refactor the error handling of __free_extent()

 kernel-shared/extent-tree.c | 154 ++++++++++++++++++------------------
 1 file changed, 75 insertions(+), 79 deletions(-)

--
2.52.0

[PATCH 1/2] btrfs-progs: do not dump leaf if the path is released inside __free_extent()

[Qu Wenruo]

[BUG]
There is a bug report that btrfs-convert crashed during converting an
ext4 image which is almost full.

[CAUSE]
Just before the crash, btrfs-convert is committing the current
transaction but failed to locate the backref inside __free_extent().

Then it went through the error handling path, which prints an error
message and try to dump the leaf.

But in this particular case, the error code is not -ENOENT, thus the
path is already released, resuling path->nodes[0] to be NULL, and
btrfs_print_leaf() triggers a NULL pointer dereference.

[FIX]
The kernel version of btrfs_free_extent() is only dumping the tree for
-ENOENT error code. And patch "btrfs: refactor the error handling of
__btrfs_free_extent()" is submitted to make abort_and_dump() to only
dump the leaf if the path is not released.

So follow the same kernel patch, by only dumping the leaf if the path is
not released.

Issue: #1064
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/extent-tree.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel-shared/extent-tree.c b/kernel-shared/extent-tree.c
index 5800320f7bc5..9d0155502d5e 100644
--- a/kernel-shared/extent-tree.c
+++ b/kernel-shared/extent-tree.c
@@ -2058,8 +2058,10 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 		       (unsigned long long)root_objectid,
 		       (unsigned long long)owner_objectid,
 		       (unsigned long long)owner_offset);
-		printf("path->slots[0]: %d path->nodes[0]:\n", path->slots[0]);
-		btrfs_print_leaf(path->nodes[0]);
+		if (path->nodes[0]) {
+			printf("path->slots[0]: %d path->nodes[0]:\n", path->slots[0]);
+			btrfs_print_leaf(path->nodes[0]);
+		}
 		ret = -EIO;
 		goto fail;
 	}
-- 
2.52.0

[PATCH 2/2] btrfs-progs: btrfs: refactor the error handling of __free_extent()

[Qu Wenruo]

Just follow the kernel patch "btrfs: refactor the error handling of
__btrfs_free_extent()", to handle the error first for
lookup_extent_backref(), so we can reduce one indent level.

Furthermore remove the unnessary forced type casting of the error
message, and replace the old printk() with proper the error() helper.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 kernel-shared/extent-tree.c | 150 +++++++++++++++++-------------------
 1 file changed, 72 insertions(+), 78 deletions(-)

diff --git a/kernel-shared/extent-tree.c b/kernel-shared/extent-tree.c
index 9d0155502d5e..c32999055ecf 100644
--- a/kernel-shared/extent-tree.c
+++ b/kernel-shared/extent-tree.c
@@ -1980,84 +1980,10 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 				    bytenr, num_bytes, parent,
 				    root_objectid, owner_objectid,
 				    owner_offset);
-	if (ret == 0) {
-		extent_slot = path->slots[0];
-		while (extent_slot >= 0) {
-			btrfs_item_key_to_cpu(path->nodes[0], &key,
-					      extent_slot);
-			if (key.objectid != bytenr)
-				break;
-			if (key.type == BTRFS_EXTENT_ITEM_KEY &&
-			    key.offset == num_bytes) {
-				found_extent = 1;
-				break;
-			}
-			if (key.type == BTRFS_METADATA_ITEM_KEY &&
-			    key.offset == owner_objectid) {
-				found_extent = 1;
-				break;
-			}
-			if (path->slots[0] - extent_slot > 5)
-				break;
-			extent_slot--;
-		}
-		if (!found_extent) {
-			BUG_ON(iref);
-			ret = remove_extent_backref(trans, extent_root, path,
-						    NULL, refs_to_drop,
-						    is_data);
-			BUG_ON(ret);
-			btrfs_release_path(path);
-
-			key.objectid = bytenr;
-
-			if (skinny_metadata) {
-				key.type = BTRFS_METADATA_ITEM_KEY;
-				key.offset = owner_objectid;
-			} else {
-				key.type = BTRFS_EXTENT_ITEM_KEY;
-				key.offset = num_bytes;
-			}
-
-			ret = btrfs_search_slot(trans, extent_root,
-						&key, path, -1, 1);
-			if (ret > 0 && skinny_metadata && path->slots[0]) {
-				path->slots[0]--;
-				btrfs_item_key_to_cpu(path->nodes[0],
-						      &key,
-						      path->slots[0]);
-				if (key.objectid == bytenr &&
-				    key.type == BTRFS_EXTENT_ITEM_KEY &&
-				    key.offset == num_bytes)
-					ret = 0;
-			}
-
-			if (ret > 0 && skinny_metadata) {
-				skinny_metadata = 0;
-				btrfs_release_path(path);
-				key.type = BTRFS_EXTENT_ITEM_KEY;
-				key.offset = num_bytes;
-				ret = btrfs_search_slot(trans, extent_root,
-							&key, path, -1, 1);
-			}
-
-			if (ret) {
-				printk(KERN_ERR "umm, got %d back from search"
-				       ", was looking for %llu\n", ret,
-				       (unsigned long long)bytenr);
-				btrfs_print_leaf(path->nodes[0]);
-			}
-			BUG_ON(ret);
-			extent_slot = path->slots[0];
-		}
-	} else {
-		printk(KERN_ERR "btrfs unable to find ref byte nr %llu "
-		       "parent %llu root %llu  owner %llu offset %llu\n",
-		       (unsigned long long)bytenr,
-		       (unsigned long long)parent,
-		       (unsigned long long)root_objectid,
-		       (unsigned long long)owner_objectid,
-		       (unsigned long long)owner_offset);
+	if (ret) {
+		error("unable to find ref byte nr %llu parent %llu root %llu  owner %llu offset %llu ret %d\n",
+		       bytenr, parent, root_objectid, owner_objectid,
+		       owner_offset, ret);
 		if (path->nodes[0]) {
 			printf("path->slots[0]: %d path->nodes[0]:\n", path->slots[0]);
 			btrfs_print_leaf(path->nodes[0]);
@@ -2065,7 +1991,75 @@ static int __free_extent(struct btrfs_trans_handle *trans,
 		ret = -EIO;
 		goto fail;
 	}
+	extent_slot = path->slots[0];
+	while (extent_slot >= 0) {
+		btrfs_item_key_to_cpu(path->nodes[0], &key,
+				      extent_slot);
+		if (key.objectid != bytenr)
+			break;
+		if (key.type == BTRFS_EXTENT_ITEM_KEY &&
+		    key.offset == num_bytes) {
+			found_extent = 1;
+			break;
+		}
+		if (key.type == BTRFS_METADATA_ITEM_KEY &&
+		    key.offset == owner_objectid) {
+			found_extent = 1;
+			break;
+		}
+		if (path->slots[0] - extent_slot > 5)
+			break;
+		extent_slot--;
+	}
+	if (!found_extent) {
+		BUG_ON(iref);
+		ret = remove_extent_backref(trans, extent_root, path,
+					    NULL, refs_to_drop,
+					    is_data);
+		BUG_ON(ret);
+		btrfs_release_path(path);
 
+		key.objectid = bytenr;
+
+		if (skinny_metadata) {
+			key.type = BTRFS_METADATA_ITEM_KEY;
+			key.offset = owner_objectid;
+		} else {
+			key.type = BTRFS_EXTENT_ITEM_KEY;
+			key.offset = num_bytes;
+		}
+
+		ret = btrfs_search_slot(trans, extent_root,
+					&key, path, -1, 1);
+		if (ret > 0 && skinny_metadata && path->slots[0]) {
+			path->slots[0]--;
+			btrfs_item_key_to_cpu(path->nodes[0],
+					      &key,
+					      path->slots[0]);
+			if (key.objectid == bytenr &&
+			    key.type == BTRFS_EXTENT_ITEM_KEY &&
+			    key.offset == num_bytes)
+				ret = 0;
+		}
+
+		if (ret > 0 && skinny_metadata) {
+			skinny_metadata = 0;
+			btrfs_release_path(path);
+			key.type = BTRFS_EXTENT_ITEM_KEY;
+			key.offset = num_bytes;
+			ret = btrfs_search_slot(trans, extent_root,
+						&key, path, -1, 1);
+		}
+
+		if (ret) {
+			printk(KERN_ERR "umm, got %d back from search"
+			       ", was looking for %llu\n", ret,
+			       (unsigned long long)bytenr);
+			btrfs_print_leaf(path->nodes[0]);
+		}
+		BUG_ON(ret);
+		extent_slot = path->slots[0];
+	}
 	leaf = path->nodes[0];
 	item_size = btrfs_item_size(leaf, extent_slot);
 	if (item_size < sizeof(*ei)) {
-- 
2.52.0

Re: [PATCH 0/2] btrfs-progs: enhance error handling of __free_extent()

[Boris Burkov]

On Mon, Nov 24, 2025 at 02:45:25PM +1030, Qu Wenruo wrote:
> There is a github bug report about btrfs-convert crash, where
> btrfs_print_leaf() is called on NULL path->nodes[0].
> 
> The first patch fix the bug by cross-port a fix from kernel part.
> 
> The second patch refactor the error handling of __free_extent(), mostly
> follow the kernel patch "btrfs: refactor the error handling of __btrfs_free_extent()".

Reviewed-by: Boris Burkov <boris@bur.io>

> 
> Qu Wenruo (2):
>   btrfs-progs: do not dump leaf if the path is released inside
>     __free_extent()
>   btrfs-progs: btrfs: refactor the error handling of __free_extent()
> 
>  kernel-shared/extent-tree.c | 154 ++++++++++++++++++------------------
>  1 file changed, 75 insertions(+), 79 deletions(-)
> 
> --
> 2.52.0
>