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

[Damien Le Moal <dlemoal@kernel.org>]

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