btrfs: extent-tree: kill BUG_ON() in __btrfs_free_extent()
authorQu Wenruo <wqu@suse.com>
Wed, 19 Aug 2020 06:35:48 +0000 (14:35 +0800)
committerDavid Sterba <dsterba@suse.com>
Wed, 7 Oct 2020 10:12:14 +0000 (12:12 +0200)
__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 <nborisov@suse.com>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/extent-tree.c

index 79c4432e6a3faf7dea7859623c6b02f717b6eb8b..40e5b41e91cc0d180d4b6f03117db101774a46d8 100644 (file)
@@ -2934,6 +2934,65 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans)
        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,
                               struct btrfs_delayed_ref_node *node, u64 parent,
                               u64 root_objectid, u64 owner_objectid,
@@ -2966,7 +3025,15 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
        path->leave_spinning = 1;
 
        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)
                skinny_metadata = false;
@@ -2975,6 +3042,13 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
                                    parent, root_objectid, owner_objectid,
                                    owner_offset);
        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];
                while (extent_slot >= 0) {
                        btrfs_item_key_to_cpu(path->nodes[0], &key,
@@ -2991,13 +3065,21 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
                                found_extent = 1;
                                break;
                        }
+
+                       /* Quick path didn't find the EXTEMT/METADATA_ITEM */
                        if (path->slots[0] - extent_slot > 5)
                                break;
                        extent_slot--;
                }
 
                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,
                                                    refs_to_drop,
                                                    is_data, &last_ref);
@@ -3008,6 +3090,7 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
                        btrfs_release_path(path);
                        path->leave_spinning = 1;
 
+                       /* Slow path to locate EXTENT/METADATA_ITEM */
                        key.objectid = bytenr;
                        key.type = BTRFS_EXTENT_ITEM_KEY;
                        key.offset = num_bytes;
@@ -3082,19 +3165,26 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
        if (owner_objectid < BTRFS_FIRST_FREE_OBJECTID &&
            key.type == BTRFS_EXTENT_ITEM_KEY) {
                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);
                WARN_ON(owner_objectid != btrfs_tree_block_level(leaf, bi));
        }
 
        refs = btrfs_extent_refs(leaf, ei);
        if (refs < refs_to_drop) {
-               btrfs_err(info,
-                         "trying to drop %d refs but we only have %Lu for bytenr %Lu",
+               btrfs_crit(info,
+               "trying to drop %d refs but we only have %llu for bytenr %llu",
                          refs_to_drop, refs, bytenr);
-               ret = -EINVAL;
-               btrfs_abort_transaction(trans, ret);
-               goto out;
+               btrfs_abort_transaction(trans, -EUCLEAN);
+               goto err_dump;
        }
        refs -= refs_to_drop;
 
@@ -3106,7 +3196,12 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
                 * be updated by remove_extent_backref
                 */
                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 {
                        btrfs_set_extent_refs(leaf, ei, refs);
                        btrfs_mark_buffer_dirty(leaf);
@@ -3121,13 +3216,39 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
                        }
                }
        } else {
+               /* In this branch refs == 1 */
                if (found_extent) {
-                       BUG_ON(is_data && refs_to_drop !=
-                              extent_data_ref_count(path, iref));
+                       if (is_data && refs_to_drop !=
+                           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) {
-                               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 {
-                               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;
                                num_to_del = 2;
                        }
@@ -3168,6 +3289,19 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
 out:
        btrfs_free_path(path);
        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;
 }
 
 /*