Commit 1c2a07f5 authored by Qu Wenruo's avatar Qu Wenruo Committed by David Sterba

btrfs: extent-tree: kill BUG_ON() in __btrfs_free_extent()

__btrfs_free_extent() is doing two things:

1. Reduce the refs number of an extent backref
   Either it's an inline extent backref (inside EXTENT/METADATA item) or
   a keyed extent backref (SHARED_* item).
   We only need to locate that backref line, either reduce the number or
   remove the backref line completely.

2. Update the refs count in EXTENT/METADATA_ITEM

During step 1), we will try to locate the EXTENT/METADATA_ITEM without
triggering another btrfs_search_slot() as fast path.

Only when we fail to locate that item, we will trigger another
btrfs_search_slot() to get that EXTENT/METADATA_ITEM after we
updated/deleted the backref line.

And we have a lot of strict checks on things like refs_to_drop against
extent refs and special case checks for single ref extents.

There are 7 BUG_ON()s, although they're doing correct checks, they can
be triggered by crafted images.

This patch improves the function:

- Introduce two examples to show what __btrfs_free_extent() is doing
  One inline backref case and one keyed case.  Should cover most cases.

- Kill all BUG_ON()s with proper error message and optional leaf dump

- Add comment to show the overall flow

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202819
[ The report triggers one BUG_ON() in __btrfs_free_extent() ]
Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
Reviewed-by: default avatarJosef Bacik <josef@toxicpanda.com>
Signed-off-by: default avatarQu Wenruo <wqu@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent f98b6215
...@@ -2934,6 +2934,65 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans) ...@@ -2934,6 +2934,65 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
return 0; return 0;
} }
/*
* Drop one or more refs of @node.
*
* 1. Locate the extent refs.
* It's either inline in EXTENT/METADATA_ITEM or in keyed SHARED_* item.
* Locate it, then reduce the refs number or remove the ref line completely.
*
* 2. Update the refs count in EXTENT/METADATA_ITEM
*
* Inline backref case:
*
* in extent tree we have:
*
* item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 16201 itemsize 82
* refs 2 gen 6 flags DATA
* extent data backref root FS_TREE objectid 258 offset 0 count 1
* extent data backref root FS_TREE objectid 257 offset 0 count 1
*
* This function gets called with:
*
* node->bytenr = 13631488
* node->num_bytes = 1048576
* root_objectid = FS_TREE
* owner_objectid = 257
* owner_offset = 0
* refs_to_drop = 1
*
* Then we should get some like:
*
* item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 16201 itemsize 82
* refs 1 gen 6 flags DATA
* extent data backref root FS_TREE objectid 258 offset 0 count 1
*
* Keyed backref case:
*
* in extent tree we have:
*
* item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 3971 itemsize 24
* refs 754 gen 6 flags DATA
* [...]
* item 2 key (13631488 EXTENT_DATA_REF <HASH>) itemoff 3915 itemsize 28
* extent data backref root FS_TREE objectid 866 offset 0 count 1
*
* This function get called with:
*
* node->bytenr = 13631488
* node->num_bytes = 1048576
* root_objectid = FS_TREE
* owner_objectid = 866
* owner_offset = 0
* refs_to_drop = 1
*
* Then we should get some like:
*
* item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 3971 itemsize 24
* refs 753 gen 6 flags DATA
*
* And that (13631488 EXTENT_DATA_REF <HASH>) gets removed.
*/
static int __btrfs_free_extent(struct btrfs_trans_handle *trans, static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
struct btrfs_delayed_ref_node *node, u64 parent, struct btrfs_delayed_ref_node *node, u64 parent,
u64 root_objectid, u64 owner_objectid, u64 root_objectid, u64 owner_objectid,
...@@ -2966,7 +3025,15 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, ...@@ -2966,7 +3025,15 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
path->leave_spinning = 1; path->leave_spinning = 1;
is_data = owner_objectid >= BTRFS_FIRST_FREE_OBJECTID; is_data = owner_objectid >= BTRFS_FIRST_FREE_OBJECTID;
BUG_ON(!is_data && refs_to_drop != 1);
if (!is_data && refs_to_drop != 1) {
btrfs_crit(info,
"invalid refs_to_drop, dropping more than 1 refs for tree block %llu refs_to_drop %u",
node->bytenr, refs_to_drop);
ret = -EINVAL;
btrfs_abort_transaction(trans, ret);
goto out;
}
if (is_data) if (is_data)
skinny_metadata = false; skinny_metadata = false;
...@@ -2975,6 +3042,13 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, ...@@ -2975,6 +3042,13 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
parent, root_objectid, owner_objectid, parent, root_objectid, owner_objectid,
owner_offset); owner_offset);
if (ret == 0) { if (ret == 0) {
/*
* Either the inline backref or the SHARED_DATA_REF/
* SHARED_BLOCK_REF is found
*
* Here is a quick path to locate EXTENT/METADATA_ITEM.
* It's possible the EXTENT/METADATA_ITEM is near current slot.
*/
extent_slot = path->slots[0]; extent_slot = path->slots[0];
while (extent_slot >= 0) { while (extent_slot >= 0) {
btrfs_item_key_to_cpu(path->nodes[0], &key, btrfs_item_key_to_cpu(path->nodes[0], &key,
...@@ -2991,13 +3065,21 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, ...@@ -2991,13 +3065,21 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
found_extent = 1; found_extent = 1;
break; break;
} }
/* Quick path didn't find the EXTEMT/METADATA_ITEM */
if (path->slots[0] - extent_slot > 5) if (path->slots[0] - extent_slot > 5)
break; break;
extent_slot--; extent_slot--;
} }
if (!found_extent) { if (!found_extent) {
BUG_ON(iref); if (iref) {
btrfs_crit(info,
"invalid iref, no EXTENT/METADATA_ITEM found but has inline extent ref");
btrfs_abort_transaction(trans, -EUCLEAN);
goto err_dump;
}
/* Must be SHARED_* item, remove the backref first */
ret = remove_extent_backref(trans, path, NULL, ret = remove_extent_backref(trans, path, NULL,
refs_to_drop, refs_to_drop,
is_data, &last_ref); is_data, &last_ref);
...@@ -3008,6 +3090,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, ...@@ -3008,6 +3090,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
btrfs_release_path(path); btrfs_release_path(path);
path->leave_spinning = 1; path->leave_spinning = 1;
/* Slow path to locate EXTENT/METADATA_ITEM */
key.objectid = bytenr; key.objectid = bytenr;
key.type = BTRFS_EXTENT_ITEM_KEY; key.type = BTRFS_EXTENT_ITEM_KEY;
key.offset = num_bytes; key.offset = num_bytes;
...@@ -3082,19 +3165,26 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, ...@@ -3082,19 +3165,26 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID && if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID &&
key.type == BTRFS_EXTENT_ITEM_KEY) { key.type == BTRFS_EXTENT_ITEM_KEY) {
struct btrfs_tree_block_info *bi; struct btrfs_tree_block_info *bi;
BUG_ON(item_size < sizeof(*ei) + sizeof(*bi)); if (item_size < sizeof(*ei) + sizeof(*bi)) {
btrfs_crit(info,
"invalid extent item size for key (%llu, %u, %llu) owner %llu, has %u expect >= %lu",
key.objectid, key.type, key.offset,
owner_objectid, item_size,
sizeof(*ei) + sizeof(*bi));
btrfs_abort_transaction(trans, -EUCLEAN);
goto err_dump;
}
bi = (struct btrfs_tree_block_info *)(ei + 1); bi = (struct btrfs_tree_block_info *)(ei + 1);
WARN_ON(owner_objectid != btrfs_tree_block_level(leaf, bi)); WARN_ON(owner_objectid != btrfs_tree_block_level(leaf, bi));
} }
refs = btrfs_extent_refs(leaf, ei); refs = btrfs_extent_refs(leaf, ei);
if (refs < refs_to_drop) { if (refs < refs_to_drop) {
btrfs_err(info, btrfs_crit(info,
"trying to drop %d refs but we only have %Lu for bytenr %Lu", "trying to drop %d refs but we only have %llu for bytenr %llu",
refs_to_drop, refs, bytenr); refs_to_drop, refs, bytenr);
ret = -EINVAL; btrfs_abort_transaction(trans, -EUCLEAN);
btrfs_abort_transaction(trans, ret); goto err_dump;
goto out;
} }
refs -= refs_to_drop; refs -= refs_to_drop;
...@@ -3106,7 +3196,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, ...@@ -3106,7 +3196,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
* be updated by remove_extent_backref * be updated by remove_extent_backref
*/ */
if (iref) { if (iref) {
BUG_ON(!found_extent); if (!found_extent) {
btrfs_crit(info,
"invalid iref, got inlined extent ref but no EXTENT/METADATA_ITEM found");
btrfs_abort_transaction(trans, -EUCLEAN);
goto err_dump;
}
} else { } else {
btrfs_set_extent_refs(leaf, ei, refs); btrfs_set_extent_refs(leaf, ei, refs);
btrfs_mark_buffer_dirty(leaf); btrfs_mark_buffer_dirty(leaf);
...@@ -3121,13 +3216,39 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, ...@@ -3121,13 +3216,39 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
} }
} }
} else { } else {
/* In this branch refs == 1 */
if (found_extent) { if (found_extent) {
BUG_ON(is_data && refs_to_drop != if (is_data && refs_to_drop !=
extent_data_ref_count(path, iref)); extent_data_ref_count(path, iref)) {
btrfs_crit(info,
"invalid refs_to_drop, current refs %u refs_to_drop %u",
extent_data_ref_count(path, iref),
refs_to_drop);
btrfs_abort_transaction(trans, -EUCLEAN);
goto err_dump;
}
if (iref) { if (iref) {
BUG_ON(path->slots[0] != extent_slot); if (path->slots[0] != extent_slot) {
btrfs_crit(info,
"invalid iref, extent item key (%llu %u %llu) doesn't have wanted iref",
key.objectid, key.type,
key.offset);
btrfs_abort_transaction(trans, -EUCLEAN);
goto err_dump;
}
} else { } else {
BUG_ON(path->slots[0] != extent_slot + 1); /*
* No inline ref, we must be at SHARED_* item,
* And it's single ref, it must be:
* | extent_slot ||extent_slot + 1|
* [ EXTENT/METADATA_ITEM ][ SHARED_* ITEM ]
*/
if (path->slots[0] != extent_slot + 1) {
btrfs_crit(info,
"invalid SHARED_* item, previous item is not EXTENT/METADATA_ITEM");
btrfs_abort_transaction(trans, -EUCLEAN);
goto err_dump;
}
path->slots[0] = extent_slot; path->slots[0] = extent_slot;
num_to_del = 2; num_to_del = 2;
} }
...@@ -3168,6 +3289,19 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans, ...@@ -3168,6 +3289,19 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
out: out:
btrfs_free_path(path); btrfs_free_path(path);
return ret; return ret;
err_dump:
/*
* Leaf dump can take up a lot of log buffer, so we only do full leaf
* dump for debug build.
*/
if (IS_ENABLED(CONFIG_BTRFS_DEBUG)) {
btrfs_crit(info, "path->slots[0]=%d extent_slot=%d",
path->slots[0], extent_slot);
btrfs_print_leaf(path->nodes[0]);
}
btrfs_free_path(path);
return -EUCLEAN;
} }
/* /*
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment