[PATCH v2] btrfs: fix potential segfault in balance_remap_chunks()

[PATCH v2] btrfs: fix potential segfault in balance_remap_chunks()

[Mark Harmstone]

Fix a potential segfault in balance_remap_chunks(): if we quit early
because btrfs_inc_block_group_ro() fails, all the remaining items in the
chunks list will still have their bg value set to NULL. It's thus not
safe to dereference this pointer without checking first.

Link: https://lore.kernel.org/linux-btrfs/20260125120717.1578828-1-clm@meta.com/
Reported-by: Chris Mason <clm@fb.com>
Fixes: 81e5a4551c32 ("btrfs: allow balancing remap tree")
Signed-off-by: Mark Harmstone <mark@harmstone.com>
---
 fs/btrfs/volumes.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 95accc9361bd26..aff286d9df4aa0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4285,20 +4285,24 @@ static int balance_remap_chunks(struct btrfs_fs_info *fs_info, struct btrfs_path
 end:
 	while (!list_empty(chunks)) {
 		bool is_unused;
+		struct btrfs_block_group *bg;
 
 		rci = list_first_entry(chunks, struct remap_chunk_info, list);
 
-		spin_lock(&rci->bg->lock);
-		is_unused = !btrfs_is_block_group_used(rci->bg);
-		spin_unlock(&rci->bg->lock);
+		bg = rci->bg;
+		if (bg) {
+			spin_lock(&bg->lock);
+			is_unused = !btrfs_is_block_group_used(bg);
+			spin_unlock(&bg->lock);
 
-		if (is_unused)
-			btrfs_mark_bg_unused(rci->bg);
+			if (is_unused)
+				btrfs_mark_bg_unused(bg);
 
-		if (rci->made_ro)
-			btrfs_dec_block_group_ro(rci->bg);
+			if (rci->made_ro)
+				btrfs_dec_block_group_ro(bg);
 
-		btrfs_put_block_group(rci->bg);
+			btrfs_put_block_group(bg);
+		}
 
 		list_del(&rci->list);
 		kfree(rci);
-- 
2.52.0

Re: [PATCH v2] btrfs: fix potential segfault in balance_remap_chunks()

[Johannes Thumshirn]

Looks good,

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

Re: [PATCH v2] btrfs: fix potential segfault in balance_remap_chunks()

[David Sterba]

Subject changed to "btrfs: check block group before marking it unused in
balance_remap_chunks()"

On Mon, Mar 09, 2026 at 06:50:37PM +0000, Mark Harmstone wrote:
> Fix a potential segfault in balance_remap_chunks(): if we quit early
> because btrfs_inc_block_group_ro() fails, all the remaining items in the
> chunks list will still have their bg value set to NULL. It's thus not
> safe to dereference this pointer without checking first.
> 
> Link: https://lore.kernel.org/linux-btrfs/20260125120717.1578828-1-clm@meta.com/
> Reported-by: Chris Mason <clm@fb.com>
> Fixes: 81e5a4551c32 ("btrfs: allow balancing remap tree")
> Signed-off-by: Mark Harmstone <mark@harmstone.com>
> ---
>  fs/btrfs/volumes.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 95accc9361bd26..aff286d9df4aa0 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -4285,20 +4285,24 @@ static int balance_remap_chunks(struct btrfs_fs_info *fs_info, struct btrfs_path
>  end:
>  	while (!list_empty(chunks)) {
>  		bool is_unused;
> +		struct btrfs_block_group *bg;
>  
>  		rci = list_first_entry(chunks, struct remap_chunk_info, list);
>  
> -		spin_lock(&rci->bg->lock);
> -		is_unused = !btrfs_is_block_group_used(rci->bg);
> -		spin_unlock(&rci->bg->lock);
> +		bg = rci->bg;
> +		if (bg) {
> +			spin_lock(&bg->lock);
> +			is_unused = !btrfs_is_block_group_used(bg);
> +			spin_unlock(&bg->lock);
>  
> -		if (is_unused)
> -			btrfs_mark_bg_unused(rci->bg);
> +			if (is_unused)
> +				btrfs_mark_bg_unused(bg);
>  
> -		if (rci->made_ro)
> -			btrfs_dec_block_group_ro(rci->bg);
> +			if (rci->made_ro)
> +				btrfs_dec_block_group_ro(bg);
>  
> -		btrfs_put_block_group(rci->bg);
> +			btrfs_put_block_group(bg);
> +		}
>  
>  		list_del(&rci->list);
>  		kfree(rci);
> -- 
> 2.52.0
>