Skip to content

Commit 1c2a07f

Browse files
adam900710kdave
authored andcommitted
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: Nikolay Borisov <[email protected]> Reviewed-by: Josef Bacik <[email protected]> Signed-off-by: Qu Wenruo <[email protected]> Signed-off-by: David Sterba <[email protected]>
1 parent f98b621 commit 1c2a07f

File tree

1 file changed

+147
-13
lines changed

1 file changed

+147
-13
lines changed

fs/btrfs/extent-tree.c

Lines changed: 147 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2934,6 +2934,65 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
29342934
return 0;
29352935
}
29362936

2937+
/*
2938+
* Drop one or more refs of @node.
2939+
*
2940+
* 1. Locate the extent refs.
2941+
* It's either inline in EXTENT/METADATA_ITEM or in keyed SHARED_* item.
2942+
* Locate it, then reduce the refs number or remove the ref line completely.
2943+
*
2944+
* 2. Update the refs count in EXTENT/METADATA_ITEM
2945+
*
2946+
* Inline backref case:
2947+
*
2948+
* in extent tree we have:
2949+
*
2950+
* item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 16201 itemsize 82
2951+
* refs 2 gen 6 flags DATA
2952+
* extent data backref root FS_TREE objectid 258 offset 0 count 1
2953+
* extent data backref root FS_TREE objectid 257 offset 0 count 1
2954+
*
2955+
* This function gets called with:
2956+
*
2957+
* node->bytenr = 13631488
2958+
* node->num_bytes = 1048576
2959+
* root_objectid = FS_TREE
2960+
* owner_objectid = 257
2961+
* owner_offset = 0
2962+
* refs_to_drop = 1
2963+
*
2964+
* Then we should get some like:
2965+
*
2966+
* item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 16201 itemsize 82
2967+
* refs 1 gen 6 flags DATA
2968+
* extent data backref root FS_TREE objectid 258 offset 0 count 1
2969+
*
2970+
* Keyed backref case:
2971+
*
2972+
* in extent tree we have:
2973+
*
2974+
* item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 3971 itemsize 24
2975+
* refs 754 gen 6 flags DATA
2976+
* [...]
2977+
* item 2 key (13631488 EXTENT_DATA_REF <HASH>) itemoff 3915 itemsize 28
2978+
* extent data backref root FS_TREE objectid 866 offset 0 count 1
2979+
*
2980+
* This function get called with:
2981+
*
2982+
* node->bytenr = 13631488
2983+
* node->num_bytes = 1048576
2984+
* root_objectid = FS_TREE
2985+
* owner_objectid = 866
2986+
* owner_offset = 0
2987+
* refs_to_drop = 1
2988+
*
2989+
* Then we should get some like:
2990+
*
2991+
* item 0 key (13631488 EXTENT_ITEM 1048576) itemoff 3971 itemsize 24
2992+
* refs 753 gen 6 flags DATA
2993+
*
2994+
* And that (13631488 EXTENT_DATA_REF <HASH>) gets removed.
2995+
*/
29372996
static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
29382997
struct btrfs_delayed_ref_node *node, u64 parent,
29392998
u64 root_objectid, u64 owner_objectid,
@@ -2966,7 +3025,15 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
29663025
path->leave_spinning = 1;
29673026

29683027
is_data = owner_objectid >= BTRFS_FIRST_FREE_OBJECTID;
2969-
BUG_ON(!is_data && refs_to_drop != 1);
3028+
3029+
if (!is_data && refs_to_drop != 1) {
3030+
btrfs_crit(info,
3031+
"invalid refs_to_drop, dropping more than 1 refs for tree block %llu refs_to_drop %u",
3032+
node->bytenr, refs_to_drop);
3033+
ret = -EINVAL;
3034+
btrfs_abort_transaction(trans, ret);
3035+
goto out;
3036+
}
29703037

29713038
if (is_data)
29723039
skinny_metadata = false;
@@ -2975,6 +3042,13 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
29753042
parent, root_objectid, owner_objectid,
29763043
owner_offset);
29773044
if (ret == 0) {
3045+
/*
3046+
* Either the inline backref or the SHARED_DATA_REF/
3047+
* SHARED_BLOCK_REF is found
3048+
*
3049+
* Here is a quick path to locate EXTENT/METADATA_ITEM.
3050+
* It's possible the EXTENT/METADATA_ITEM is near current slot.
3051+
*/
29783052
extent_slot = path->slots[0];
29793053
while (extent_slot >= 0) {
29803054
btrfs_item_key_to_cpu(path->nodes[0], &key,
@@ -2991,13 +3065,21 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
29913065
found_extent = 1;
29923066
break;
29933067
}
3068+
3069+
/* Quick path didn't find the EXTEMT/METADATA_ITEM */
29943070
if (path->slots[0] - extent_slot > 5)
29953071
break;
29963072
extent_slot--;
29973073
}
29983074

29993075
if (!found_extent) {
3000-
BUG_ON(iref);
3076+
if (iref) {
3077+
btrfs_crit(info,
3078+
"invalid iref, no EXTENT/METADATA_ITEM found but has inline extent ref");
3079+
btrfs_abort_transaction(trans, -EUCLEAN);
3080+
goto err_dump;
3081+
}
3082+
/* Must be SHARED_* item, remove the backref first */
30013083
ret = remove_extent_backref(trans, path, NULL,
30023084
refs_to_drop,
30033085
is_data, &last_ref);
@@ -3008,6 +3090,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
30083090
btrfs_release_path(path);
30093091
path->leave_spinning = 1;
30103092

3093+
/* Slow path to locate EXTENT/METADATA_ITEM */
30113094
key.objectid = bytenr;
30123095
key.type = BTRFS_EXTENT_ITEM_KEY;
30133096
key.offset = num_bytes;
@@ -3082,19 +3165,26 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
30823165
if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID &&
30833166
key.type == BTRFS_EXTENT_ITEM_KEY) {
30843167
struct btrfs_tree_block_info *bi;
3085-
BUG_ON(item_size < sizeof(*ei) + sizeof(*bi));
3168+
if (item_size < sizeof(*ei) + sizeof(*bi)) {
3169+
btrfs_crit(info,
3170+
"invalid extent item size for key (%llu, %u, %llu) owner %llu, has %u expect >= %lu",
3171+
key.objectid, key.type, key.offset,
3172+
owner_objectid, item_size,
3173+
sizeof(*ei) + sizeof(*bi));
3174+
btrfs_abort_transaction(trans, -EUCLEAN);
3175+
goto err_dump;
3176+
}
30863177
bi = (struct btrfs_tree_block_info *)(ei + 1);
30873178
WARN_ON(owner_objectid != btrfs_tree_block_level(leaf, bi));
30883179
}
30893180

30903181
refs = btrfs_extent_refs(leaf, ei);
30913182
if (refs < refs_to_drop) {
3092-
btrfs_err(info,
3093-
"trying to drop %d refs but we only have %Lu for bytenr %Lu",
3183+
btrfs_crit(info,
3184+
"trying to drop %d refs but we only have %llu for bytenr %llu",
30943185
refs_to_drop, refs, bytenr);
3095-
ret = -EINVAL;
3096-
btrfs_abort_transaction(trans, ret);
3097-
goto out;
3186+
btrfs_abort_transaction(trans, -EUCLEAN);
3187+
goto err_dump;
30983188
}
30993189
refs -= refs_to_drop;
31003190

@@ -3106,7 +3196,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
31063196
* be updated by remove_extent_backref
31073197
*/
31083198
if (iref) {
3109-
BUG_ON(!found_extent);
3199+
if (!found_extent) {
3200+
btrfs_crit(info,
3201+
"invalid iref, got inlined extent ref but no EXTENT/METADATA_ITEM found");
3202+
btrfs_abort_transaction(trans, -EUCLEAN);
3203+
goto err_dump;
3204+
}
31103205
} else {
31113206
btrfs_set_extent_refs(leaf, ei, refs);
31123207
btrfs_mark_buffer_dirty(leaf);
@@ -3121,13 +3216,39 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
31213216
}
31223217
}
31233218
} else {
3219+
/* In this branch refs == 1 */
31243220
if (found_extent) {
3125-
BUG_ON(is_data && refs_to_drop !=
3126-
extent_data_ref_count(path, iref));
3221+
if (is_data && refs_to_drop !=
3222+
extent_data_ref_count(path, iref)) {
3223+
btrfs_crit(info,
3224+
"invalid refs_to_drop, current refs %u refs_to_drop %u",
3225+
extent_data_ref_count(path, iref),
3226+
refs_to_drop);
3227+
btrfs_abort_transaction(trans, -EUCLEAN);
3228+
goto err_dump;
3229+
}
31273230
if (iref) {
3128-
BUG_ON(path->slots[0] != extent_slot);
3231+
if (path->slots[0] != extent_slot) {
3232+
btrfs_crit(info,
3233+
"invalid iref, extent item key (%llu %u %llu) doesn't have wanted iref",
3234+
key.objectid, key.type,
3235+
key.offset);
3236+
btrfs_abort_transaction(trans, -EUCLEAN);
3237+
goto err_dump;
3238+
}
31293239
} else {
3130-
BUG_ON(path->slots[0] != extent_slot + 1);
3240+
/*
3241+
* No inline ref, we must be at SHARED_* item,
3242+
* And it's single ref, it must be:
3243+
* | extent_slot ||extent_slot + 1|
3244+
* [ EXTENT/METADATA_ITEM ][ SHARED_* ITEM ]
3245+
*/
3246+
if (path->slots[0] != extent_slot + 1) {
3247+
btrfs_crit(info,
3248+
"invalid SHARED_* item, previous item is not EXTENT/METADATA_ITEM");
3249+
btrfs_abort_transaction(trans, -EUCLEAN);
3250+
goto err_dump;
3251+
}
31313252
path->slots[0] = extent_slot;
31323253
num_to_del = 2;
31333254
}
@@ -3168,6 +3289,19 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
31683289
out:
31693290
btrfs_free_path(path);
31703291
return ret;
3292+
err_dump:
3293+
/*
3294+
* Leaf dump can take up a lot of log buffer, so we only do full leaf
3295+
* dump for debug build.
3296+
*/
3297+
if (IS_ENABLED(CONFIG_BTRFS_DEBUG)) {
3298+
btrfs_crit(info, "path->slots[0]=%d extent_slot=%d",
3299+
path->slots[0], extent_slot);
3300+
btrfs_print_leaf(path->nodes[0]);
3301+
}
3302+
3303+
btrfs_free_path(path);
3304+
return -EUCLEAN;
31713305
}
31723306

31733307
/*

0 commit comments

Comments
 (0)