Commit bb386803 authored by Filipe Manana's avatar Filipe Manana Committed by David Sterba
Browse files

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: default avatar <syzbot+a306f914b4d01b3958fe@syzkaller.appspotmail.com>
Link: https://lore.kernel.org/linux-btrfs/000000000000fcba1e05e998263c@google.com/


Reviewed-by: default avatarQu Wenruo <wqu@suse.com>
Signed-off-by: default avatarFilipe Manana <fdmanana@suse.com>
Reviewed-by: default avatarDavid Sterba <dsterba@suse.com>
Signed-off-by: default avatarDavid Sterba <dsterba@suse.com>
parent b7519157

fs/btrfs/ctree.c

+42 −11
Original line number Diff line number Diff line
@@ -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,

		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,

		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

fs/btrfs/extent-tree.c

+14 −10
Original line number Diff line number Diff line
@@ -3419,7 +3419,7 @@ static noinline int check_ref_cleanup(struct btrfs_trans_handle *trans, 
	return 0;

}



void btrfs_free_tree_block(struct btrfs_trans_handle *trans,

int btrfs_free_tree_block(struct btrfs_trans_handle *trans,

			  u64 root_id,

			  struct extent_buffer *buf,

			  u64 parent, int last_ref)
@@ -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 @@ void btrfs_free_tree_block(struct btrfs_trans_handle *trans, 
	 * 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,

	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",

fs/btrfs/extent-tree.h

+4 −4
Original line number Diff line number Diff line
@@ -127,7 +127,7 @@ 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,

int btrfs_free_tree_block(struct btrfs_trans_handle *trans,

			  u64 root_id,

			  struct extent_buffer *buf,

			  u64 parent, int last_ref);

fs/btrfs/free-space-tree.c

+7 −3
Original line number Diff line number Diff line
@@ -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),

	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);

}

fs/btrfs/ioctl.c

+5 −1
Original line number Diff line number Diff line
@@ -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;

	}