[PATCH 1/2] block: fix zones_cond memory leak in blk_revalidate_disk_zones()

[PATCH 1/2] block: fix zones_cond memory leak in blk_revalidate_disk_zones()

[Jackie Liu]

From: Jackie Liu <liuyun01@kylinos.cn>

Fix memory leaks of args.zones_cond allocated in
disk_revalidate_zone_resources() on multiple error paths:

1) When disk_revalidate_zone_resources() itself fails (e.g.
   disk_alloc_zone_resources() returns an error), blk_revalidate_disk_zones()
   returns directly without freeing args.zones_cond.

2) When report_zones() fails or the capacity check fails,
   disk_free_zone_resources() only frees the old disk->zones_cond, not
   the newly allocated args.zones_cond.

3) When the nr_conv_zones validation fails in disk_update_zone_resources(),
   the code jumps to unfreeze before disk_set_zones_cond_array() transfers
   ownership of args->zones_cond to disk->zones_cond.

Fix cases 1 and 2 by adding a free_zones_cond label at the end of
blk_revalidate_disk_zones() to centralize the cleanup. Fix case 3 by
moving disk_set_zones_cond_array() before the nr_conv_zones check in
disk_update_zone_resources() so that ownership is transferred early and
disk_free_zone_resources() at the unfreeze label properly frees it.

Fixes: 6e945ffb6555 ("block: use zone condition to determine conventional zones")
Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
---
 block/blk-zoned.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 9d1dd6ccfad7..2ea790e4f320 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -1956,6 +1956,8 @@ static int disk_update_zone_resources(struct gendisk *disk,
 	memflags = blk_mq_freeze_queue(q);
 
 	disk->nr_zones = args->nr_zones;
+	disk_set_zones_cond_array(disk, args->zones_cond);
+
 	if (args->nr_conv_zones >= disk->nr_zones) {
 		queue_limits_cancel_update(q);
 		pr_warn("%s: Invalid number of conventional zones %u / %u\n",
@@ -1966,7 +1968,6 @@ static int disk_update_zone_resources(struct gendisk *disk,
 
 	disk->zone_capacity = args->zone_capacity;
 	disk->last_zone_capacity = args->last_zone_capacity;
-	disk_set_zones_cond_array(disk, args->zones_cond);
 
 	/*
 	 * Some devices can advertise zone resource limits that are larger than
@@ -2239,7 +2240,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
 	ret = disk_revalidate_zone_resources(disk, &args);
 	if (ret) {
 		memalloc_noio_restore(noio_flag);
-		return ret;
+		goto free_zones_cond;
 	}
 
 	ret = disk->fops->report_zones(disk, 0, UINT_MAX, &rep_args);
@@ -2268,6 +2269,8 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
 	disk_free_zone_resources(disk);
 	blk_mq_unfreeze_queue(q, memflags);
 
+free_zones_cond:
+	kfree(args.zones_cond);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones);
-- 
2.51.1

Re: [PATCH 1/2] block: fix zones_cond memory leak in blk_revalidate_disk_zones()

[Damien Le Moal]

On 3/31/26 17:47, Jackie Liu wrote:
> From: Jackie Liu <liuyun01@kylinos.cn>
> 
> Fix memory leaks of args.zones_cond allocated in
> disk_revalidate_zone_resources() on multiple error paths:
> 
> 1) When disk_revalidate_zone_resources() itself fails (e.g.
>    disk_alloc_zone_resources() returns an error), blk_revalidate_disk_zones()
>    returns directly without freeing args.zones_cond.
> 
> 2) When report_zones() fails or the capacity check fails,
>    disk_free_zone_resources() only frees the old disk->zones_cond, not
>    the newly allocated args.zones_cond.
> 
> 3) When the nr_conv_zones validation fails in disk_update_zone_resources(),
>    the code jumps to unfreeze before disk_set_zones_cond_array() transfers
>    ownership of args->zones_cond to disk->zones_cond.
> 
> Fix cases 1 and 2 by adding a free_zones_cond label at the end of
> blk_revalidate_disk_zones() to centralize the cleanup. Fix case 3 by
> moving disk_set_zones_cond_array() before the nr_conv_zones check in
> disk_update_zone_resources() so that ownership is transferred early and
> disk_free_zone_resources() at the unfreeze label properly frees it.
> 
> Fixes: 6e945ffb6555 ("block: use zone condition to determine conventional zones")
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>  block/blk-zoned.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 9d1dd6ccfad7..2ea790e4f320 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -1956,6 +1956,8 @@ static int disk_update_zone_resources(struct gendisk *disk,
>  	memflags = blk_mq_freeze_queue(q);
>  
>  	disk->nr_zones = args->nr_zones;
> +	disk_set_zones_cond_array(disk, args->zones_cond);
> +
>  	if (args->nr_conv_zones >= disk->nr_zones) {
>  		queue_limits_cancel_update(q);
>  		pr_warn("%s: Invalid number of conventional zones %u / %u\n",
> @@ -1966,7 +1968,6 @@ static int disk_update_zone_resources(struct gendisk *disk,
>  
>  	disk->zone_capacity = args->zone_capacity;
>  	disk->last_zone_capacity = args->last_zone_capacity;
> -	disk_set_zones_cond_array(disk, args->zones_cond);
>  
>  	/*
>  	 * Some devices can advertise zone resource limits that are larger than
> @@ -2239,7 +2240,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
>  	ret = disk_revalidate_zone_resources(disk, &args);
>  	if (ret) {
>  		memalloc_noio_restore(noio_flag);
> -		return ret;
> +		goto free_zones_cond;
>  	}
>  
>  	ret = disk->fops->report_zones(disk, 0, UINT_MAX, &rep_args);
> @@ -2268,6 +2269,8 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
>  	disk_free_zone_resources(disk);
>  	blk_mq_unfreeze_queue(q, memflags);
>  
> +free_zones_cond:
> +	kfree(args.zones_cond);

This does not look correct: on success case, this will free the array despite
that array being set already. So rather than this, I think it is better to
change disk_revalidate_zone_resources() to free the array it allocated in the
case of an error. That will be a lot cleaner than this.

>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones);


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/2] block: fix zones_cond memory leak in blk_revalidate_disk_zones()

[Jackie Liu]

2026年3月31日 17:18, "Damien Le Moal" <dlemoal@kernel.org mailto:dlemoal@kernel.org?to=%22Damien%20Le%20Moal%22%20%3Cdlemoal%40kernel.org%3E > 写到:


> 
> On 3/31/26 17:47, Jackie Liu wrote:
> 
> > 
> > From: Jackie Liu <liuyun01@kylinos.cn>
> >  
> >  Fix memory leaks of args.zones_cond allocated in
> >  disk_revalidate_zone_resources() on multiple error paths:
> >  
> >  1) When disk_revalidate_zone_resources() itself fails (e.g.
> >  disk_alloc_zone_resources() returns an error), blk_revalidate_disk_zones()
> >  returns directly without freeing args.zones_cond.
> >  
> >  2) When report_zones() fails or the capacity check fails,
> >  disk_free_zone_resources() only frees the old disk->zones_cond, not
> >  the newly allocated args.zones_cond.
> >  
> >  3) When the nr_conv_zones validation fails in disk_update_zone_resources(),
> >  the code jumps to unfreeze before disk_set_zones_cond_array() transfers
> >  ownership of args->zones_cond to disk->zones_cond.
> >  
> >  Fix cases 1 and 2 by adding a free_zones_cond label at the end of
> >  blk_revalidate_disk_zones() to centralize the cleanup. Fix case 3 by
> >  moving disk_set_zones_cond_array() before the nr_conv_zones check in
> >  disk_update_zone_resources() so that ownership is transferred early and
> >  disk_free_zone_resources() at the unfreeze label properly frees it.
> >  
> >  Fixes: 6e945ffb6555 ("block: use zone condition to determine conventional zones")
> >  Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >  ---
> >  block/blk-zoned.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >  
> >  diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> >  index 9d1dd6ccfad7..2ea790e4f320 100644
> >  --- a/block/blk-zoned.c
> >  +++ b/block/blk-zoned.c
> >  @@ -1956,6 +1956,8 @@ static int disk_update_zone_resources(struct gendisk *disk,
> >  memflags = blk_mq_freeze_queue(q);
> >  
> >  disk->nr_zones = args->nr_zones;
> >  + disk_set_zones_cond_array(disk, args->zones_cond);
> >  +
> >  if (args->nr_conv_zones >= disk->nr_zones) {
> >  queue_limits_cancel_update(q);
> >  pr_warn("%s: Invalid number of conventional zones %u / %u\n",
> >  @@ -1966,7 +1968,6 @@ static int disk_update_zone_resources(struct gendisk *disk,
> >  
> >  disk->zone_capacity = args->zone_capacity;
> >  disk->last_zone_capacity = args->last_zone_capacity;
> >  - disk_set_zones_cond_array(disk, args->zones_cond);
> >  
> >  /*
> >  * Some devices can advertise zone resource limits that are larger than
> >  @@ -2239,7 +2240,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
> >  ret = disk_revalidate_zone_resources(disk, &args);
> >  if (ret) {
> >  memalloc_noio_restore(noio_flag);
> >  - return ret;
> >  + goto free_zones_cond;
> >  }
> >  
> >  ret = disk->fops->report_zones(disk, 0, UINT_MAX, &rep_args);
> >  @@ -2268,6 +2269,8 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
> >  disk_free_zone_resources(disk);
> >  blk_mq_unfreeze_queue(q, memflags);
> >  
> >  +free_zones_cond:
> >  + kfree(args.zones_cond);
> > 
> This does not look correct: on success case, this will free the array despite
> that array being set already. So rather than this, I think it is better to
> change disk_revalidate_zone_resources() to free the array it allocated in the
> case of an error. That will be a lot cleaner than this.

Thanks for the review.

Actually, the free_zones_cond label is only reachable on error paths
(ret <= 0). On the success path (ret > 0), the function returns
directly via "return disk_update_zone_resources(disk, &args)" and never
reaches this label. So the logic should be correct.

That said, I agree that having disk_revalidate_zone_resources() free
the array itself on error is cleaner and easier to follow. I'll send
a v2 with that approach.

-- 
Jackie

> 
> > 
> > return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones);
> > 
> -- 
> Damien Le Moal
> Western Digital Research
>

Re: [PATCH 1/2] block: fix zones_cond memory leak in blk_revalidate_disk_zones()

[Damien Le Moal]

On 3/31/26 18:47, Jackie Liu wrote:
>> This does not look correct: on success case, this will free the array despite
>> that array being set already. So rather than this, I think it is better to
>> change disk_revalidate_zone_resources() to free the array it allocated in the
>> case of an error. That will be a lot cleaner than this.
> 
> Thanks for the review.
> 
> Actually, the free_zones_cond label is only reachable on error paths
> (ret <= 0). On the success path (ret > 0), the function returns
> directly via "return disk_update_zone_resources(disk, &args)" and never
> reaches this label. So the logic should be correct.

Ah, yes, indeed.

> 
> That said, I agree that having disk_revalidate_zone_resources() free
> the array itself on error is cleaner and easier to follow. I'll send
> a v2 with that approach.

Sounds good. Thanks.

> 


-- 
Damien Le Moal
Western Digital Research