On 1.06.20 г. 21:12 ч., fdmanana@xxxxxxxxxx wrote:
> From: Filipe Manana <fdmanana@xxxxxxxx>
>
> 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.
However I'm having hard time reconciling this. The block group is
removed from the block_group_cache_tree after we've called
btrfs_del_item. So if btrfs_del_item or btrfs_search_slot fail the code
jumps at out_put_group and puts the reference acquired at the beginning
of the function via btrfs_lookup_block_group.
I think what you meant is if we fail to delete the block group's item
from the freespace tree, that is if we fail
remove_block_group_free_space, then we'd have a ref leak. With this
modification to the changelog:
Reviewed-by: Nikolay Borisov <nborisov@xxxxxxxx>
>
> 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'.
I agree with this.
>
> Fixes: f6033c5e333238 ("btrfs: fix block group leak when removing fails")
> Signed-off-by: Filipe Manana <fdmanana@xxxxxxxx>
<snip>
