Commit 9fecd132 authored by Filipe Manana's avatar Filipe Manana Committed by David Sterba

btrfs: fix a block group ref counter leak after failure to remove block group

When removing a block group, if we fail to delete the block group's item
from the extent tree, we jump to the 'out' label and end up decrementing
the block group's reference count once only (by 1), resulting in a counter
leak because the block group at that point was already removed from the
block group cache rbtree - so we have to decrement the reference count
twice, once for the rbtree and once for our lookup at the start of the
function.

There is a second bug where if removing the free space tree entries (the
call to remove_block_group_free_space()) fails we end up jumping to the
'out_put_group' label but end up decrementing the reference count only
once, when we should have done it twice, since we have already removed
the block group from the block group cache rbtree. This happens because
the reference count decrement for the rbtree reference happens after
attempting to remove the free space tree entries, which is far away from
the place where we remove the block group from the rbtree.

To make things less error prone, decrement the reference count for the
rbtree immediately after removing the block group from it. This also
eleminates the need for two different exit labels on error, renaming
'out_put_label' to just 'out' and removing the old 'out'.

Fixes: f6033c5e ("btrfs: fix block group leak when removing fails")
CC: stable@vger.kernel.org # 4.4+
Reviewed-by: default avatarNikolay Borisov <nborisov@suse.com>
Reviewed-by: default avatarAnand Jain <anand.jain@oracle.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 55e20bd1
...@@ -940,7 +940,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans, ...@@ -940,7 +940,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
path = btrfs_alloc_path(); path = btrfs_alloc_path();
if (!path) { if (!path) {
ret = -ENOMEM; ret = -ENOMEM;
goto out_put_group; goto out;
} }
/* /*
...@@ -978,7 +978,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans, ...@@ -978,7 +978,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
ret = btrfs_orphan_add(trans, BTRFS_I(inode)); ret = btrfs_orphan_add(trans, BTRFS_I(inode));
if (ret) { if (ret) {
btrfs_add_delayed_iput(inode); btrfs_add_delayed_iput(inode);
goto out_put_group; goto out;
} }
clear_nlink(inode); clear_nlink(inode);
/* One for the block groups ref */ /* One for the block groups ref */
...@@ -1001,13 +1001,13 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans, ...@@ -1001,13 +1001,13 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
ret = btrfs_search_slot(trans, tree_root, &key, path, -1, 1); ret = btrfs_search_slot(trans, tree_root, &key, path, -1, 1);
if (ret < 0) if (ret < 0)
goto out_put_group; goto out;
if (ret > 0) if (ret > 0)
btrfs_release_path(path); btrfs_release_path(path);
if (ret == 0) { if (ret == 0) {
ret = btrfs_del_item(trans, tree_root, path); ret = btrfs_del_item(trans, tree_root, path);
if (ret) if (ret)
goto out_put_group; goto out;
btrfs_release_path(path); btrfs_release_path(path);
} }
...@@ -1016,6 +1016,9 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans, ...@@ -1016,6 +1016,9 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
&fs_info->block_group_cache_tree); &fs_info->block_group_cache_tree);
RB_CLEAR_NODE(&block_group->cache_node); RB_CLEAR_NODE(&block_group->cache_node);
/* Once for the block groups rbtree */
btrfs_put_block_group(block_group);
if (fs_info->first_logical_byte == block_group->start) if (fs_info->first_logical_byte == block_group->start)
fs_info->first_logical_byte = (u64)-1; fs_info->first_logical_byte = (u64)-1;
spin_unlock(&fs_info->block_group_cache_lock); spin_unlock(&fs_info->block_group_cache_lock);
...@@ -1125,10 +1128,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans, ...@@ -1125,10 +1128,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
ret = remove_block_group_free_space(trans, block_group); ret = remove_block_group_free_space(trans, block_group);
if (ret) if (ret)
goto out_put_group; goto out;
/* Once for the block groups rbtree */
btrfs_put_block_group(block_group);
ret = remove_block_group_item(trans, path, block_group); ret = remove_block_group_item(trans, path, block_group);
if (ret < 0) if (ret < 0)
...@@ -1145,10 +1145,9 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans, ...@@ -1145,10 +1145,9 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans,
free_extent_map(em); free_extent_map(em);
} }
out_put_group: out:
/* Once for the lookup reference */ /* Once for the lookup reference */
btrfs_put_block_group(block_group); btrfs_put_block_group(block_group);
out:
if (remove_rsv) if (remove_rsv)
btrfs_delayed_refs_rsv_release(fs_info, 1); btrfs_delayed_refs_rsv_release(fs_info, 1);
btrfs_free_path(path); btrfs_free_path(path);
......
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