Re: [PATCH v2 1/3] Btrfs: fix a block group ref counter leak after failure to remove block group

From: David Sterba <dsterba@suse.cz>
To: fdmanana@kernel.org
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2 1/3] Btrfs: fix a block group ref counter leak after failure to remove block group
Date: Thu, 4 Jun 2020 19:11:11 +0200	[thread overview]
Message-ID: <20200604171111.GF27034@twin.jikos.cz> (raw)
In-Reply-To: <20200603101112.23369-1-fdmanana@kernel.org>

On Wed, Jun 03, 2020 at 11:11:12AM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> 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: f6033c5e333238 ("btrfs: fix block group leak when removing fails")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> 
> V2: Updated changelog to describe a second bug the patch fixes, pointed
>     out by Nikolay.

V2 updated in misc-next, thanks.