[PATCH v2 0/4] btrfs: zoned: LLM inspired fixes

[PATCH v2 0/4] btrfs: zoned: LLM inspired fixes

[Johannes Thumshirn]

I got inspired by Darrick's "LLM inspired fixes for XFS" serieses and
told $LLM to have a look at zoned.c and find some obvious problems.
After sorting out what I think were false positives, here is what I came
up with.

Changes to v1:
- Add comment about the possible UAF to wait_eb_writebacks()

Johannes Thumshirn (4):
  btrfs: zoned: only change active zone counter on successful
    (de)activation
  btrfs: zoned: fix possible UAF in wait_eb_writebacks
  btrfs: zoned: requeue block group if zone reset bails out
  btrfs: zoned: avoid underflow of bytes_zone_unusable

 fs/btrfs/block-group.c |  2 +-
 fs/btrfs/zoned.c       | 67 ++++++++++++++++++++++++++++++++----------
 2 files changed, 53 insertions(+), 16 deletions(-)

-- 
2.55.0

[PATCH v2 1/4] btrfs: zoned: only change active zone counter on successful (de)activation

[Johannes Thumshirn]

btrfs_zone_activate() unconditionally decrements the reserved_active_zones
counter regardless if btrfs_dev_set_active_zone() fails to set the zone
active, i.e. because it raced with another call that already set the bit
in the bitmask, or not.

This can lead to a double decrement of the counter in case the bit has
already been set.

Only decrement the counter *iff* btrfs_dev_set_active_zone() successfully
marked the zone in the bitmap.

Mirror this behaviour when clearing the bit again.

Fixes: a7e1ac7bdc5a ("btrfs: zoned: reserve zones for an active metadata/system block group")
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/zoned.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 9f562cc34e38..d68b91008e76 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1126,11 +1126,13 @@ u64 btrfs_find_allocatable_zones(struct btrfs_device *device, u64 hole_start,
 	return pos;
 }
 
-static bool btrfs_dev_set_active_zone(struct btrfs_device *device, u64 pos)
+static bool btrfs_dev_set_active_zone(struct btrfs_device *device, u64 pos, bool *new)
 {
 	struct btrfs_zoned_device_info *zone_info = device->zone_info;
 	unsigned int zno = (pos >> zone_info->zone_size_shift);
 
+	*new = false;
+
 	/* We can use any number of zones */
 	if (zone_info->max_active_zones == 0)
 		return true;
@@ -1142,23 +1144,30 @@ static bool btrfs_dev_set_active_zone(struct btrfs_device *device, u64 pos)
 		if (test_and_set_bit(zno, zone_info->active_zones)) {
 			/* Someone already set the bit */
 			atomic_inc(&zone_info->active_zones_left);
+		} else {
+			*new = true;
 		}
 	}
 
 	return true;
 }
 
-static void btrfs_dev_clear_active_zone(struct btrfs_device *device, u64 pos)
+static bool btrfs_dev_clear_active_zone(struct btrfs_device *device, u64 pos)
 {
 	struct btrfs_zoned_device_info *zone_info = device->zone_info;
 	unsigned int zno = (pos >> zone_info->zone_size_shift);
 
+
 	/* We can use any number of zones */
 	if (zone_info->max_active_zones == 0)
-		return;
+		return false;
 
-	if (test_and_clear_bit(zno, zone_info->active_zones))
+	if (test_and_clear_bit(zno, zone_info->active_zones)) {
 		atomic_inc(&zone_info->active_zones_left);
+		return true;
+	}
+
+	return false;
 }
 
 int btrfs_reset_device_zone(struct btrfs_device *device, u64 physical,
@@ -2411,6 +2420,7 @@ bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 	u64 physical;
 	const bool is_data = (block_group->flags & BTRFS_BLOCK_GROUP_DATA);
 	bool ret;
+	bool new;
 	int i;
 
 	if (!btrfs_is_zoned(block_group->fs_info))
@@ -2464,12 +2474,12 @@ bool btrfs_zone_activate(struct btrfs_block_group *block_group)
 			goto out_unlock;
 		}
 
-		if (!btrfs_dev_set_active_zone(device, physical)) {
+		if (!btrfs_dev_set_active_zone(device, physical, &new)) {
 			/* Cannot activate the zone */
 			ret = false;
 			goto out_unlock;
 		}
-		if (!is_data)
+		if (!is_data && new)
 			zinfo->reserved_active_zones--;
 	}
 
@@ -2516,6 +2526,7 @@ static int call_zone_finish(struct btrfs_block_group *block_group,
 	struct btrfs_device *device = stripe->dev;
 	const u64 physical = stripe->physical;
 	struct btrfs_zoned_device_info *zinfo = device->zone_info;
+	bool cleared;
 	int ret;
 
 	if (!device->bdev)
@@ -2537,9 +2548,9 @@ static int call_zone_finish(struct btrfs_block_group *block_group,
 			return ret;
 	}
 
-	if (!(block_group->flags & BTRFS_BLOCK_GROUP_DATA))
+	cleared = btrfs_dev_clear_active_zone(device, physical);
+	if (!(block_group->flags & BTRFS_BLOCK_GROUP_DATA) && cleared)
 		zinfo->reserved_active_zones++;
-	btrfs_dev_clear_active_zone(device, physical);
 
 	return 0;
 }
-- 
2.55.0

[PATCH v2 2/4] btrfs: zoned: fix possible UAF in wait_eb_writebacks

[Johannes Thumshirn]

wait_eb_writebacks() iterates the fs_info->buffer_tree xarray and waits
for the writeback of these extent-buffers. Waiting for writeback needs
to be done without the rcu_read_lock() held (because it can sleep) and
thus the loop drops the rcu_read_lock() before calling into
wait_on_extent_buffer_writeback(). But extent_buffers are freed through
RCU, so if btrfs_release_extent_buffer_rcu() is scheduled while we're
still waiting on the writeback, the extent_buffer will be freed causing
a use-after-free.

Similar to what is done in find_extent_buffer_nolock(), get a reference
to the extent_buffer before calling into
wait_on_extent_buffer_writeback() so a sucessfull writeback does not
free the extent_buffer while we still have a reference to it.

Fixes: 2dd7e7bc0282 ("btrfs: zoned: wait for extent buffer IOs before finishing a zone")
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/zoned.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index d68b91008e76..261e7991c1d1 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -2513,8 +2513,16 @@ static void wait_eb_writebacks(struct btrfs_block_group *block_group)
 			continue;
 		if (eb->start >= end)
 			break;
+		/*
+		 * Skip if we can't get the refcount for the extent buffer,
+		 * i.e. because it was already written back. Otherwise an eb
+		 * disappering underneath us can cause a use-after-free.
+		 */
+		if (!refcount_inc_not_zero(&eb->refs))
+			continue;
 		rcu_read_unlock();
 		wait_on_extent_buffer_writeback(eb);
+		free_extent_buffer(eb);
 		rcu_read_lock();
 	}
 	rcu_read_unlock();
-- 
2.55.0

[PATCH v2 3/4] btrfs: zoned: requeue block group if zone reset bails out

[Johannes Thumshirn]

btrfs_reset_unused_block_groups() drops the block-group from
fs_info->unused_bgs before performing the work that can actually fail,
the actual submission of a REQ_OP_ZONE_RESET.

Every failed operation after this leaves the block-group dangling,
physically reset, but the in-memory structures aren't and it is not
requeued to be reset on fs_info->unused_bgs.

Instead of bailing out, re-add this block-group to fs_info->unused_bgs
list.

Fixes: 453a73c3069a ("btrfs: zoned: reclaim unused zone by zone resetting")
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/zoned.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 261e7991c1d1..96d3957db87f 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -3137,6 +3137,8 @@ int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num
 {
 	struct btrfs_fs_info *fs_info = space_info->fs_info;
 	const sector_t zone_size_sectors = fs_info->zone_size >> SECTOR_SHIFT;
+	LIST_HEAD(retry_list);
+	int ret = 0;
 
 	if (!btrfs_is_zoned(fs_info))
 		return 0;
@@ -3178,11 +3180,10 @@ int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num
 		}
 		if (!found) {
 			spin_unlock(&fs_info->unused_bgs_lock);
-			return 0;
+			goto out;
 		}
 
 		list_del_init(&bg->bg_list);
-		btrfs_put_block_group(bg);
 		spin_unlock(&fs_info->unused_bgs_lock);
 
 		/*
@@ -3196,7 +3197,6 @@ int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num
 		for (int i = 0; i < map->num_stripes; i++) {
 			struct btrfs_io_stripe *stripe = &map->stripes[i];
 			unsigned int nofs_flags;
-			int ret;
 
 			nofs_flags = memalloc_nofs_save();
 			ret = blkdev_zone_mgmt(stripe->dev->bdev, REQ_OP_ZONE_RESET,
@@ -3206,7 +3206,7 @@ int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num
 
 			if (ret) {
 				up_read(&fs_info->dev_replace.rwsem);
-				return ret;
+				goto requeue;
 			}
 		}
 		up_read(&fs_info->dev_replace.rwsem);
@@ -3217,7 +3217,7 @@ int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num
 		if (bg->ro) {
 			spin_unlock(&bg->lock);
 			spin_unlock(&space_info->lock);
-			continue;
+			goto requeue;
 		}
 
 		reclaimed = bg->alloc_offset;
@@ -3246,12 +3246,30 @@ int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num
 		btrfs_return_free_space(space_info, reclaimed);
 		spin_unlock(&space_info->lock);
 
+		btrfs_put_block_group(bg);
+
 		if (num_bytes <= reclaimed)
 			break;
 		num_bytes -= reclaimed;
+		continue;
+
+requeue:
+		spin_lock(&fs_info->unused_bgs_lock);
+		list_add_tail(&bg->bg_list, &retry_list);
+		spin_unlock(&fs_info->unused_bgs_lock);
+
+		if (ret)
+			goto out;
 	}
 
-	return 0;
+out:
+	if (!list_empty(&retry_list)) {
+		spin_lock(&fs_info->unused_bgs_lock);
+		list_splice_tail(&retry_list, &fs_info->unused_bgs);
+		spin_unlock(&fs_info->unused_bgs_lock);
+	}
+
+	return ret;
 }
 
 void btrfs_show_zoned_stats(struct btrfs_fs_info *fs_info, struct seq_file *seq)
-- 
2.55.0

[PATCH v2 4/4] btrfs: zoned: avoid underflow of bytes_zone_unusable

[Johannes Thumshirn]

The two functions btrfs_free_reserved_bytes() and
btrfs_reset_unused_block_groups() both manipulate
space_info->bytes_zone_unusable without using the accessor function
btrfs_space_info_update_bytes_zone_unusable() potentially risking an
underflow (in case of btrfs_reset_unused_block_groups()).

Use btrfs_space_info_update_bytes_zone_unusable() in both function, in
the case of btrfs_free_reserved_bytes() it is mostly for documenting
purposes.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/block-group.c | 2 +-
 fs/btrfs/zoned.c       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index c148b9d6e597..74a61db93250 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -4114,7 +4114,7 @@ void btrfs_free_reserved_bytes(struct btrfs_block_group *cache, u64 num_bytes,
 	if (bg_ro)
 		space_info->bytes_readonly += num_bytes;
 	else if (btrfs_is_zoned(cache->fs_info))
-		space_info->bytes_zone_unusable += num_bytes;
+		btrfs_space_info_update_bytes_zone_unusable(space_info, num_bytes);
 
 	space_info->bytes_reserved -= num_bytes;
 	space_info->max_extent_size = 0;
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 96d3957db87f..89286a92a13f 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -3241,7 +3241,7 @@ int btrfs_reset_unused_block_groups(struct btrfs_space_info *space_info, u64 num
 		ASSERT(reclaimed == bg->zone_capacity,
 		       "reclaimed=%llu bg->zone_capacity=%llu", reclaimed, bg->zone_capacity);
 		bg->free_space_ctl->free_space += reclaimed;
-		space_info->bytes_zone_unusable -= reclaimed;
+		btrfs_space_info_update_bytes_zone_unusable(space_info, -reclaimed);
 		spin_unlock(&bg->lock);
 		btrfs_return_free_space(space_info, reclaimed);
 		spin_unlock(&space_info->lock);
-- 
2.55.0

Re: [PATCH v2 1/4] btrfs: zoned: only change active zone counter on successful (de)activation

[Wang Yugui]

Hi,

> btrfs_zone_activate() unconditionally decrements the reserved_active_zones
> counter regardless if btrfs_dev_set_active_zone() fails to set the zone
> active, i.e. because it raced with another call that already set the bit
> in the bitmask, or not.
> 
> This can lead to a double decrement of the counter in case the bit has
> already been set.
> 
> Only decrement the counter *iff* btrfs_dev_set_active_zone() successfully
> marked the zone in the bitmap.
> 
> Mirror this behaviour when clearing the bit again.
> 
> Fixes: a7e1ac7bdc5a ("btrfs: zoned: reserve zones for an active metadata/system block group")
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/btrfs/zoned.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index 9f562cc34e38..d68b91008e76 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1126,11 +1126,13 @@ u64 btrfs_find_allocatable_zones(struct btrfs_device *device, u64 hole_start,
>  	return pos;
>  }
>  
> -static bool btrfs_dev_set_active_zone(struct btrfs_device *device, u64 pos)
> +static bool btrfs_dev_set_active_zone(struct btrfs_device *device, u64 pos, bool *new)
>  {
>  	struct btrfs_zoned_device_info *zone_info = device->zone_info;
>  	unsigned int zno = (pos >> zone_info->zone_size_shift);
>  
> +	*new = false;
> +
>  	/* We can use any number of zones */
>  	if (zone_info->max_active_zones == 0)
>  		return true;
> @@ -1142,23 +1144,30 @@ static bool btrfs_dev_set_active_zone(struct btrfs_device *device, u64 pos)
>  		if (test_and_set_bit(zno, zone_info->active_zones)) {
>  			/* Someone already set the bit */
>  			atomic_inc(&zone_info->active_zones_left);
> +		} else {
> +			*new = true;
>  		}
>  	}
>  
>  	return true;
>  }
>  
> -static void btrfs_dev_clear_active_zone(struct btrfs_device *device, u64 pos)
> +static bool btrfs_dev_clear_active_zone(struct btrfs_device *device, u64 pos)
>  {
>  	struct btrfs_zoned_device_info *zone_info = device->zone_info;
>  	unsigned int zno = (pos >> zone_info->zone_size_shift);
>  
> +
>  	/* We can use any number of zones */
>  	if (zone_info->max_active_zones == 0)
> -		return;
> +		return false;
>  
> -	if (test_and_clear_bit(zno, zone_info->active_zones))
> +	if (test_and_clear_bit(zno, zone_info->active_zones)) {
>  		atomic_inc(&zone_info->active_zones_left);
> +		return true;
> +	}
> +
> +	return false;
>  }
>  
>  int btrfs_reset_device_zone(struct btrfs_device *device, u64 physical,
> @@ -2411,6 +2420,7 @@ bool btrfs_zone_activate(struct btrfs_block_group *block_group)
>  	u64 physical;
>  	const bool is_data = (block_group->flags & BTRFS_BLOCK_GROUP_DATA);
>  	bool ret;
> +	bool new;

'new' is not a good var name because it is a keyword in C++/Java.

'is_new' just like above 'is_data'  maybe a better var name.

Best  Regards
Wang Yugui

Re: [PATCH v2 1/4] btrfs: zoned: only change active zone counter on successful (de)activation

[Johannes Thumshirn]

On Fri, Oct 02, 2026 at 11:38:57PM +0800, Wang Yugui wrote:
> > @@ -2411,6 +2420,7 @@ bool btrfs_zone_activate(struct btrfs_block_group *block_group)
> >  	u64 physical;
> >  	const bool is_data = (block_group->flags & BTRFS_BLOCK_GROUP_DATA);
> >  	bool ret;
> > +	bool new;
> 
> 'new' is not a good var name because it is a keyword in C++/Java.
> 
> 'is_new' just like above 'is_data'  maybe a better var name.

But this is C not C++ or Java.