[PATCH v2] block: fix zones_cond memory leak in blk_revalidate_disk_zones()

[PATCH v2] 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 this by freeing args->zones_cond at each error site: in
blk_revalidate_disk_zones() when disk_revalidate_zone_resources() or zone
reporting fails, and in disk_update_zone_resources() before jumping to
the unfreeze label.

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

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 9d1dd6ccfad7..be99ab785dcd 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -1960,6 +1960,7 @@ static int disk_update_zone_resources(struct gendisk *disk,
 		queue_limits_cancel_update(q);
 		pr_warn("%s: Invalid number of conventional zones %u / %u\n",
 			disk->disk_name, args->nr_conv_zones, disk->nr_zones);
+		kfree(args->zones_cond);
 		ret = -ENODEV;
 		goto unfreeze;
 	}
@@ -2239,6 +2240,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
 	ret = disk_revalidate_zone_resources(disk, &args);
 	if (ret) {
 		memalloc_noio_restore(noio_flag);
+		kfree(args.zones_cond);
 		return ret;
 	}
 
@@ -2264,6 +2266,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
 
 	pr_warn("%s: failed to revalidate zones\n", disk->disk_name);
 
+	kfree(args.zones_cond);
 	memflags = blk_mq_freeze_queue(q);
 	disk_free_zone_resources(disk);
 	blk_mq_unfreeze_queue(q, memflags);
-- 
2.51.1

Re: [PATCH v2] block: fix zones_cond memory leak in blk_revalidate_disk_zones()

[Damien Le Moal]

On 3/31/26 19:01, 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 this by freeing args->zones_cond at each error site: in
> blk_revalidate_disk_zones() when disk_revalidate_zone_resources() or zone
> reporting fails, and in disk_update_zone_resources() before jumping to
> the unfreeze label.
> 
> Fixes: 6e945ffb6555 ("block: use zone condition to determine conventional zones")
> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> ---
>  block/blk-zoned.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 9d1dd6ccfad7..be99ab785dcd 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -1960,6 +1960,7 @@ static int disk_update_zone_resources(struct gendisk *disk,
>  		queue_limits_cancel_update(q);
>  		pr_warn("%s: Invalid number of conventional zones %u / %u\n",
>  			disk->disk_name, args->nr_conv_zones, disk->nr_zones);
> +		kfree(args->zones_cond);

Let's keep the cleanups together. So move this before pr_warn().
Also, the call to queue_limits_commit_update() may fail, so you need a free
after the unfreeze label in that function too.

>  		ret = -ENODEV;
>  		goto unfreeze;
>  	}
> @@ -2239,6 +2240,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
>  	ret = disk_revalidate_zone_resources(disk, &args);
>  	if (ret) {
>  		memalloc_noio_restore(noio_flag);
> +		kfree(args.zones_cond);

This should be in disk_revalidate_zone_resources().

>  		return ret;
>  	}
>  
> @@ -2264,6 +2266,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
>  
>  	pr_warn("%s: failed to revalidate zones\n", disk->disk_name);
>  
> +	kfree(args.zones_cond);
>  	memflags = blk_mq_freeze_queue(q);
>  	disk_free_zone_resources(disk);
>  	blk_mq_unfreeze_queue(q, memflags);

I thinks something like this may be cleaner as it avoids having that kfree() all
over the place:

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 10655213e8e1..18cd9a1c6c53 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -2028,6 +2028,7 @@ static int disk_revalidate_zone_resources(struct gendisk
*disk,
 {
        struct queue_limits *lim = &disk->queue->limits;
        unsigned int pool_size;
+       int ret = 0;

        args->disk = disk;
        args->nr_zones =
@@ -2050,10 +2051,13 @@ static int disk_revalidate_zone_resources(struct gendisk
*disk,
                pool_size =
                        min(BLK_ZONE_WPLUG_DEFAULT_POOL_SIZE, args->nr_zones);

-       if (!disk->zone_wplugs_hash)
-               return disk_alloc_zone_resources(disk, pool_size);
+       if (!disk->zone_wplugs_hash) {
+               ret = disk_alloc_zone_resources(disk, pool_size);
+               if (ret)
+                       kfree(args->zones_cond);
+       }

-       return 0;
+       return ret;
 }

 /*
@@ -2085,6 +2089,7 @@ 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);
+       args->zones_cond = NULL;

        /*
         * Some devices can advertise zone resource limits that are larger than
@@ -2365,21 +2370,30 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
        }
        memalloc_noio_restore(noio_flag);

+       if (ret <= 0)
+               goto free_resources;
+
        /*
         * If zones where reported, make sure that the entire disk capacity
         * has been checked.
         */
-       if (ret > 0 && args.sector != capacity) {
+       if (args.sector != capacity) {
                pr_warn("%s: Missing zones from sector %llu\n",
                        disk->disk_name, args.sector);
                ret = -ENODEV;
+               goto free_resources;
        }

-       if (ret > 0)
-               return disk_update_zone_resources(disk, &args);
+       ret = disk_update_zone_resources(disk, &args);
+       if (ret)
+               goto free_resources;
+
+       return 0;

+free_resources:
        pr_warn("%s: failed to revalidate zones\n", disk->disk_name);

+       kfree(args.zones_cond);
        memflags = blk_mq_freeze_queue(q);
        disk_free_zone_resources(disk);
        blk_mq_unfreeze_queue(q, memflags);

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] block: fix zones_cond memory leak in blk_revalidate_disk_zones()

[Jackie Liu]

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


> 
> On 3/31/26 19:01, 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 this by freeing args->zones_cond at each error site: in
> >  blk_revalidate_disk_zones() when disk_revalidate_zone_resources() or zone
> >  reporting fails, and in disk_update_zone_resources() before jumping to
> >  the unfreeze label.
> >  
> >  Fixes: 6e945ffb6555 ("block: use zone condition to determine conventional zones")
> >  Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
> >  ---
> >  block/blk-zoned.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >  
> >  diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> >  index 9d1dd6ccfad7..be99ab785dcd 100644
> >  --- a/block/blk-zoned.c
> >  +++ b/block/blk-zoned.c
> >  @@ -1960,6 +1960,7 @@ static int disk_update_zone_resources(struct gendisk *disk,
> >  queue_limits_cancel_update(q);
> >  pr_warn("%s: Invalid number of conventional zones %u / %u\n",
> >  disk->disk_name, args->nr_conv_zones, disk->nr_zones);
> >  + kfree(args->zones_cond);
> > 
> Let's keep the cleanups together. So move this before pr_warn().
> Also, the call to queue_limits_commit_update() may fail, so you need a free
> after the unfreeze label in that function too.
> 
> > 
> > ret = -ENODEV;
> >  goto unfreeze;
> >  }
> >  @@ -2239,6 +2240,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
> >  ret = disk_revalidate_zone_resources(disk, &args);
> >  if (ret) {
> >  memalloc_noio_restore(noio_flag);
> >  + kfree(args.zones_cond);
> > 
> This should be in disk_revalidate_zone_resources().
> 
> > 
> > return ret;
> >  }
> >  
> >  @@ -2264,6 +2266,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
> >  
> >  pr_warn("%s: failed to revalidate zones\n", disk->disk_name);
> >  
> >  + kfree(args.zones_cond);
> >  memflags = blk_mq_freeze_queue(q);
> >  disk_free_zone_resources(disk);
> >  blk_mq_unfreeze_queue(q, memflags);
> > 
> I thinks something like this may be cleaner as it avoids having that kfree() all
> over the place:
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 10655213e8e1..18cd9a1c6c53 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -2028,6 +2028,7 @@ static int disk_revalidate_zone_resources(struct gendisk
> *disk,
>  {
>  struct queue_limits *lim = &disk->queue->limits;
>  unsigned int pool_size;
> + int ret = 0;
> 
>  args->disk = disk;
>  args->nr_zones =
> @@ -2050,10 +2051,13 @@ static int disk_revalidate_zone_resources(struct gendisk
> *disk,
>  pool_size =
>  min(BLK_ZONE_WPLUG_DEFAULT_POOL_SIZE, args->nr_zones);
> 
> - if (!disk->zone_wplugs_hash)
> - return disk_alloc_zone_resources(disk, pool_size);
> + if (!disk->zone_wplugs_hash) {
> + ret = disk_alloc_zone_resources(disk, pool_size);
> + if (ret)
> + kfree(args->zones_cond);
> + }
> 
> - return 0;
> + return ret;
>  }
> 
>  /*
> @@ -2085,6 +2089,7 @@ 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);
> + args->zones_cond = NULL;
> 
>  /*
>  * Some devices can advertise zone resource limits that are larger than
> @@ -2365,21 +2370,30 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
>  }
>  memalloc_noio_restore(noio_flag);
> 
> + if (ret <= 0)
> + goto free_resources;
> +
>  /*
>  * If zones where reported, make sure that the entire disk capacity
>  * has been checked.
>  */
> - if (ret > 0 && args.sector != capacity) {
> + if (args.sector != capacity) {
>  pr_warn("%s: Missing zones from sector %llu\n",
>  disk->disk_name, args.sector);
>  ret = -ENODEV;
> + goto free_resources;
>  }
> 
> - if (ret > 0)
> - return disk_update_zone_resources(disk, &args);
> + ret = disk_update_zone_resources(disk, &args);
> + if (ret)
> + goto free_resources;
> +
> + return 0;
> 
> +free_resources:
>  pr_warn("%s: failed to revalidate zones\n", disk->disk_name);
> 
> + kfree(args.zones_cond);
>  memflags = blk_mq_freeze_queue(q);
>  disk_free_zone_resources(disk);
>  blk_mq_unfreeze_queue(q, memflags);
> 

Haha, the patch I wrote at the beginning had similar advantages to yours. 
I also wanted to refactor it so that the correct path wouldn't be hidden, 
but that would involve more code than it does now, so I didn't do it. Since
you mentioned it, I'll use your method, which is actually quite good.

Thanks.

-- 
Jackie Liu

> -- 
> Damien Le Moal
> Western Digital Research
>