btrfs: do not BUG_ON() when freeing tree block after error

When freeing a tree block, at btrfs_free_tree_block(), if we fail to
create a delayed reference we don't deal with the error and just do a
BUG_ON(). The error most likely to happen is -ENOMEM, and we have a
comment mentioning that only -ENOMEM can happen, but that is not true,
because in case qgroups are enabled any error returned from
btrfs_qgroup_trace_extent_post() (can be -EUCLEAN or anything returned
from btrfs_search_slot() for example) can be propagated back to
btrfs_free_tree_block().

So stop doing a BUG_ON() and return the error to the callers and make
them abort the transaction to prevent leaking space. Syzbot was
triggering this, likely due to memory allocation failure injection.

Reported-by: syzbot+a306f914b4d01b3958fe@syzkaller.appspotmail.com
Link: https://lore.kernel.org/linux-btrfs/000000000000fcba1e05e998263c@google.com/
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 48aa14046343a42c0def39ea0e5f973808c981e9..a155dbc0bffa94e59ea5f91ee220fb9b9839820c 100644 (file)
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -620,10 +620,16 @@ int btrfs_force_cow_block(struct btrfs_trans_handle *trans,
                atomic_inc(&cow->refs);
                rcu_assign_pointer(root->node, cow);
 
-               btrfs_free_tree_block(trans, btrfs_root_id(root), buf,
-                                     parent_start, last_ref);
+               ret = btrfs_free_tree_block(trans, btrfs_root_id(root), buf,
+                                           parent_start, last_ref);
                free_extent_buffer(buf);
                add_root_to_dirty_list(root);
+               if (ret < 0) {
+                       btrfs_tree_unlock(cow);
+                       free_extent_buffer(cow);
+                       btrfs_abort_transaction(trans, ret);
+                       return ret;
+               }
        } else {
                WARN_ON(trans->transid != btrfs_header_generation(parent));
                ret = btrfs_tree_mod_log_insert_key(parent, parent_slot,
@@ -648,8 +654,14 @@ int btrfs_force_cow_block(struct btrfs_trans_handle *trans,
                                return ret;
                        }
                }
-               btrfs_free_tree_block(trans, btrfs_root_id(root), buf,
-                                     parent_start, last_ref);
+               ret = btrfs_free_tree_block(trans, btrfs_root_id(root), buf,
+                                           parent_start, last_ref);
+               if (ret < 0) {
+                       btrfs_tree_unlock(cow);
+                       free_extent_buffer(cow);
+                       btrfs_abort_transaction(trans, ret);
+                       return ret;
+               }
        }
        if (unlock_orig)
                btrfs_tree_unlock(buf);
@@ -983,9 +995,13 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
                free_extent_buffer(mid);
 
                root_sub_used_bytes(root);
-               btrfs_free_tree_block(trans, btrfs_root_id(root), mid, 0, 1);
+               ret = btrfs_free_tree_block(trans, btrfs_root_id(root), mid, 0, 1);
                /* once for the root ptr */
                free_extent_buffer_stale(mid);
+               if (ret < 0) {
+                       btrfs_abort_transaction(trans, ret);
+                       goto out;
+               }
                return 0;
        }
        if (btrfs_header_nritems(mid) >
@@ -1053,10 +1069,14 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
                                goto out;
                        }
                        root_sub_used_bytes(root);
-                       btrfs_free_tree_block(trans, btrfs_root_id(root), right,
-                                             0, 1);
+                       ret = btrfs_free_tree_block(trans, btrfs_root_id(root),
+                                                   right, 0, 1);
                        free_extent_buffer_stale(right);
                        right = NULL;
+                       if (ret < 0) {
+                               btrfs_abort_transaction(trans, ret);
+                               goto out;
+                       }
                } else {
                        struct btrfs_disk_key right_key;
                        btrfs_node_key(right, &right_key, 0);
@@ -1111,9 +1131,13 @@ static noinline int balance_level(struct btrfs_trans_handle *trans,
                        goto out;
                }
                root_sub_used_bytes(root);
-               btrfs_free_tree_block(trans, btrfs_root_id(root), mid, 0, 1);
+               ret = btrfs_free_tree_block(trans, btrfs_root_id(root), mid, 0, 1);
                free_extent_buffer_stale(mid);
                mid = NULL;
+               if (ret < 0) {
+                       btrfs_abort_transaction(trans, ret);
+                       goto out;
+               }
        } else {
                /* update the parent key to reflect our changes */
                struct btrfs_disk_key mid_key;
@@ -2878,7 +2902,11 @@ static noinline int insert_new_root(struct btrfs_trans_handle *trans,
        old = root->node;
        ret = btrfs_tree_mod_log_insert_root(root->node, c, false);
        if (ret < 0) {
-               btrfs_free_tree_block(trans, btrfs_root_id(root), c, 0, 1);
+               int ret2;
+
+               ret2 = btrfs_free_tree_block(trans, btrfs_root_id(root), c, 0, 1);
+               if (ret2 < 0)
+                       btrfs_abort_transaction(trans, ret2);
                btrfs_tree_unlock(c);
                free_extent_buffer(c);
                return ret;
@@ -4447,9 +4475,12 @@ static noinline int btrfs_del_leaf(struct btrfs_trans_handle *trans,
        root_sub_used_bytes(root);
 
        atomic_inc(&leaf->refs);
-       btrfs_free_tree_block(trans, btrfs_root_id(root), leaf, 0, 1);
+       ret = btrfs_free_tree_block(trans, btrfs_root_id(root), leaf, 0, 1);
        free_extent_buffer_stale(leaf);
-       return 0;
+       if (ret < 0)
+               btrfs_abort_transaction(trans, ret);
+
+       return ret;
 }
 /*
  * delete the item at the leaf level in path.  If that empties

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index eda4241921645a9a2430e54ade1b27f202008c0e..58a72a57414a85f3d46b3bb3cdde8f9ecb74cc45 100644 (file)
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3419,10 +3419,10 @@ out_delayed_unlock:
        return 0;
 }
 
-void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
-                          u64 root_id,
-                          struct extent_buffer *buf,
-                          u64 parent, int last_ref)
+int btrfs_free_tree_block(struct btrfs_trans_handle *trans,
+                         u64 root_id,
+                         struct extent_buffer *buf,
+                         u64 parent, int last_ref)
 {
        struct btrfs_fs_info *fs_info = trans->fs_info;
        struct btrfs_block_group *bg;
@@ -3449,11 +3449,12 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
                btrfs_init_tree_ref(&generic_ref, btrfs_header_level(buf), 0, false);
                btrfs_ref_tree_mod(fs_info, &generic_ref);
                ret = btrfs_add_delayed_tree_ref(trans, &generic_ref, NULL);
-               BUG_ON(ret); /* -ENOMEM */
+               if (ret < 0)
+                       return ret;
        }
 
        if (!last_ref)
-               return;
+               return 0;
 
        if (btrfs_header_generation(buf) != trans->transid)
                goto out;
@@ -3510,6 +3511,7 @@ out:
         * matter anymore.
         */
        clear_bit(EXTENT_BUFFER_CORRUPT, &buf->bflags);
+       return 0;
 }
 
 /* Can return -ENOMEM */
@@ -5754,7 +5756,7 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans,
                                 struct walk_control *wc)
 {
        struct btrfs_fs_info *fs_info = root->fs_info;
-       int ret;
+       int ret = 0;
        int level = wc->level;
        struct extent_buffer *eb = path->nodes[level];
        u64 parent = 0;
@@ -5849,12 +5851,14 @@ static noinline int walk_up_proc(struct btrfs_trans_handle *trans,
                        goto owner_mismatch;
        }
 
-       btrfs_free_tree_block(trans, btrfs_root_id(root), eb, parent,
-                             wc->refs[level] == 1);
+       ret = btrfs_free_tree_block(trans, btrfs_root_id(root), eb, parent,
+                                   wc->refs[level] == 1);
+       if (ret < 0)
+               btrfs_abort_transaction(trans, ret);
 out:
        wc->refs[level] = 0;
        wc->flags[level] = 0;
-       return 0;
+       return ret;
 
 owner_mismatch:
        btrfs_err_rl(fs_info, "unexpected tree owner, have %llu expect %llu",

diff --git a/fs/btrfs/extent-tree.h b/fs/btrfs/extent-tree.h
index af9f8800d5aca5d8c5c000868f4640476573a25a..2ad51130c037e5835705741e8bd49b5e99bce1f0 100644 (file)
--- a/fs/btrfs/extent-tree.h
+++ b/fs/btrfs/extent-tree.h
@@ -127,10 +127,10 @@ struct extent_buffer *btrfs_alloc_tree_block(struct btrfs_trans_handle *trans,
                                             u64 empty_size,
                                             u64 reloc_src_root,
                                             enum btrfs_lock_nesting nest);
-void btrfs_free_tree_block(struct btrfs_trans_handle *trans,
-                          u64 root_id,
-                          struct extent_buffer *buf,
-                          u64 parent, int last_ref);
+int btrfs_free_tree_block(struct btrfs_trans_handle *trans,
+                         u64 root_id,
+                         struct extent_buffer *buf,
+                         u64 parent, int last_ref);
 int btrfs_alloc_reserved_file_extent(struct btrfs_trans_handle *trans,
                                     struct btrfs_root *root, u64 owner,
                                     u64 offset, u64 ram_bytes,

diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c
index 90f2938bd743d308db6b325258fa08d9abe85ada..7ba50e133921a40cf3100da891a6a4850de90338 100644 (file)
--- a/fs/btrfs/free-space-tree.c
+++ b/fs/btrfs/free-space-tree.c
@@ -1300,10 +1300,14 @@ int btrfs_delete_free_space_tree(struct btrfs_fs_info *fs_info)
        btrfs_tree_lock(free_space_root->node);
        btrfs_clear_buffer_dirty(trans, free_space_root->node);
        btrfs_tree_unlock(free_space_root->node);
-       btrfs_free_tree_block(trans, btrfs_root_id(free_space_root),
-                             free_space_root->node, 0, 1);
-
+       ret = btrfs_free_tree_block(trans, btrfs_root_id(free_space_root),
+                                   free_space_root->node, 0, 1);
        btrfs_put_root(free_space_root);
+       if (ret < 0) {
+               btrfs_abort_transaction(trans, ret);
+               btrfs_end_transaction(trans);
+               return ret;
+       }
 
        return btrfs_commit_transaction(trans);
 }

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 06447ee6d7ce022f71cd88495635416ac0973101..d28ebabe37209cbdd8a00d1a13b206708ebcb136 100644 (file)
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -714,6 +714,8 @@ static noinline int create_subvol(struct mnt_idmap *idmap,
        ret = btrfs_insert_root(trans, fs_info->tree_root, &key,
                                root_item);
        if (ret) {
+               int ret2;
+
                /*
                 * Since we don't abort the transaction in this case, free the
                 * tree block so that we don't leak space and leave the
@@ -724,7 +726,9 @@ static noinline int create_subvol(struct mnt_idmap *idmap,
                btrfs_tree_lock(leaf);
                btrfs_clear_buffer_dirty(trans, leaf);
                btrfs_tree_unlock(leaf);
-               btrfs_free_tree_block(trans, objectid, leaf, 0, 1);
+               ret2 = btrfs_free_tree_block(trans, objectid, leaf, 0, 1);
+               if (ret2 < 0)
+                       btrfs_abort_transaction(trans, ret2);
                free_extent_buffer(leaf);
                goto out;
        }

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 31249b5200b77c3a215fb63a9f52622aafedce51..8b01bfdee820db5f8223144c903a4e345ddc4b37 100644 (file)
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1441,9 +1441,11 @@ int btrfs_quota_disable(struct btrfs_fs_info *fs_info)
        btrfs_tree_lock(quota_root->node);
        btrfs_clear_buffer_dirty(trans, quota_root->node);
        btrfs_tree_unlock(quota_root->node);
-       btrfs_free_tree_block(trans, btrfs_root_id(quota_root),
-                             quota_root->node, 0, 1);
+       ret = btrfs_free_tree_block(trans, btrfs_root_id(quota_root),
+                                   quota_root->node, 0, 1);
 
+       if (ret < 0)
+               btrfs_abort_transaction(trans, ret);
 
 out:
        btrfs_put_root(quota_root);