* [PATCH 1/2] btrfs-progs: do not dump leaf if the path is released inside __free_extent()
2025-11-24 4:15 [PATCH 0/2] btrfs-progs: enhance error handling of __free_extent() Qu Wenruo
@ 2025-11-24 4:15 ` Qu Wenruo
2025-11-24 4:15 ` [PATCH 2/2] btrfs-progs: btrfs: refactor the error handling of __free_extent() Qu Wenruo
2025-11-25 17:05 ` [PATCH 0/2] btrfs-progs: enhance " Boris Burkov
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2025-11-24 4:15 UTC (permalink / raw)
To: linux-btrfs
[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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] btrfs-progs: btrfs: refactor the error handling of __free_extent()
2025-11-24 4:15 [PATCH 0/2] btrfs-progs: enhance error handling of __free_extent() Qu Wenruo
2025-11-24 4:15 ` [PATCH 1/2] btrfs-progs: do not dump leaf if the path is released inside __free_extent() Qu Wenruo
@ 2025-11-24 4:15 ` Qu Wenruo
2025-11-25 17:05 ` [PATCH 0/2] btrfs-progs: enhance " Boris Burkov
2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2025-11-24 4:15 UTC (permalink / raw)
To: linux-btrfs
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
^ permalink raw reply related [flat|nested] 4+ messages in thread