[PATCH v2] btrfs: fix use-after-free in move_existing_remap()

[PATCH v2] btrfs: fix use-after-free in move_existing_remap()

[Mark Harmstone]

There is a potential use-after-free in move_existing_remap(): we're calling
btrfs_put_block_group() on dest_bg, then passing it to
btrfs_add_block_group_free_space() a few lines later.

Fix this by getting the BG at the start of the function and putting it
near the end. This also means we're not doing a lookup twice for the
same thing.

Link: https://lore.kernel.org/linux-btrfs/20260125123908.2096548-1-clm@meta.com/
Reported-by: Chris Mason <clm@fb.com>
Fixes: bbea42dfb91f ("btrfs: move existing remaps before relocating block group")
Signed-off-by: Mark Harmstone <mark@harmstone.com>
---
 fs/btrfs/relocation.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 88b1ec416fe272..1c42c5180bddd5 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4177,6 +4177,8 @@ static int move_existing_remap(struct btrfs_fs_info *fs_info,
 	dest_addr = ins.objectid;
 	dest_length = ins.offset;
 
+	dest_bg = btrfs_lookup_block_group(fs_info, dest_addr);
+
 	if (!is_data && !IS_ALIGNED(dest_length, fs_info->nodesize)) {
 		u64 new_length = ALIGN_DOWN(dest_length, fs_info->nodesize);
 
@@ -4287,15 +4289,12 @@ static int move_existing_remap(struct btrfs_fs_info *fs_info,
 	if (unlikely(ret))
 		goto end;
 
-	dest_bg = btrfs_lookup_block_group(fs_info, dest_addr);
-
 	adjust_block_group_remap_bytes(trans, dest_bg, dest_length);
 
 	mutex_lock(&dest_bg->free_space_lock);
 	bg_needs_free_space = test_bit(BLOCK_GROUP_FLAG_NEEDS_FREE_SPACE,
 				       &dest_bg->runtime_flags);
 	mutex_unlock(&dest_bg->free_space_lock);
-	btrfs_put_block_group(dest_bg);
 
 	if (bg_needs_free_space) {
 		ret = btrfs_add_block_group_free_space(trans, dest_bg);
@@ -4325,13 +4324,13 @@ static int move_existing_remap(struct btrfs_fs_info *fs_info,
 			btrfs_end_transaction(trans);
 		}
 	} else {
-		dest_bg = btrfs_lookup_block_group(fs_info, dest_addr);
 		btrfs_free_reserved_bytes(dest_bg, dest_length, 0);
-		btrfs_put_block_group(dest_bg);
 
 		ret = btrfs_commit_transaction(trans);
 	}
 
+	btrfs_put_block_group(dest_bg);
+
 	return ret;
 }
 
-- 
2.52.0

Re: [PATCH v2] btrfs: fix use-after-free in move_existing_remap()

[Johannes Thumshirn]

Looks good,

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH v2] btrfs: fix use-after-free in move_existing_remap()

[David Sterba]

Subject changed to btrfs: hold block group reference during entire
move_existing_remap()"

On Fri, Mar 06, 2026 at 02:18:31PM +0000, Mark Harmstone wrote:
> There is a potential use-after-free in move_existing_remap(): we're calling
> btrfs_put_block_group() on dest_bg, then passing it to
> btrfs_add_block_group_free_space() a few lines later.
> 
> Fix this by getting the BG at the start of the function and putting it
> near the end. This also means we're not doing a lookup twice for the
> same thing.
> 
> Link: https://lore.kernel.org/linux-btrfs/20260125123908.2096548-1-clm@meta.com/
> Reported-by: Chris Mason <clm@fb.com>
> Fixes: bbea42dfb91f ("btrfs: move existing remaps before relocating block group")
> Signed-off-by: Mark Harmstone <mark@harmstone.com>
> ---
>  fs/btrfs/relocation.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 88b1ec416fe272..1c42c5180bddd5 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4177,6 +4177,8 @@ static int move_existing_remap(struct btrfs_fs_info *fs_info,
>  	dest_addr = ins.objectid;
>  	dest_length = ins.offset;
>  
> +	dest_bg = btrfs_lookup_block_group(fs_info, dest_addr);
> +
>  	if (!is_data && !IS_ALIGNED(dest_length, fs_info->nodesize)) {
>  		u64 new_length = ALIGN_DOWN(dest_length, fs_info->nodesize);
>  
> @@ -4287,15 +4289,12 @@ static int move_existing_remap(struct btrfs_fs_info *fs_info,
>  	if (unlikely(ret))
>  		goto end;
>  
> -	dest_bg = btrfs_lookup_block_group(fs_info, dest_addr);
> -
>  	adjust_block_group_remap_bytes(trans, dest_bg, dest_length);
>  
>  	mutex_lock(&dest_bg->free_space_lock);
>  	bg_needs_free_space = test_bit(BLOCK_GROUP_FLAG_NEEDS_FREE_SPACE,
>  				       &dest_bg->runtime_flags);
>  	mutex_unlock(&dest_bg->free_space_lock);
> -	btrfs_put_block_group(dest_bg);
>  
>  	if (bg_needs_free_space) {
>  		ret = btrfs_add_block_group_free_space(trans, dest_bg);
> @@ -4325,13 +4324,13 @@ static int move_existing_remap(struct btrfs_fs_info *fs_info,
>  			btrfs_end_transaction(trans);
>  		}
>  	} else {
> -		dest_bg = btrfs_lookup_block_group(fs_info, dest_addr);
>  		btrfs_free_reserved_bytes(dest_bg, dest_length, 0);
> -		btrfs_put_block_group(dest_bg);
>  
>  		ret = btrfs_commit_transaction(trans);
>  	}
>  
> +	btrfs_put_block_group(dest_bg);
> +
>  	return ret;
>  }
>  
> -- 
> 2.52.0
>