[PATCH -next] block: fix memory leak for elevator on add_disk failure

[PATCH -next] block: fix memory leak for elevator on add_disk failure

[Yu Kuai]

From: Yu Kuai <yukuai3@huawei.com>

The default elevator is allocated in the begining of device_add_disk(),
however, it's not freed in the following error path.

Fixes: 737eb78e82d5 ("block: Delay default elevator initialization")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 block/genhd.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index b3d04e79e854..fea319c8f99e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -437,9 +437,10 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
 	 * Otherwise just allocate the device numbers for both the whole device
 	 * and all partitions from the extended dev_t space.
 	 */
+	ret = -EINVAL;
 	if (disk->major) {
 		if (WARN_ON(!disk->minors))
-			return -EINVAL;
+			goto out_exit_elevator;
 
 		if (disk->minors > DISK_MAX_PARTS) {
 			pr_err("block: can't allocate more than %d partitions\n",
@@ -447,14 +448,14 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
 			disk->minors = DISK_MAX_PARTS;
 		}
 		if (disk->first_minor + disk->minors > MINORMASK + 1)
-			return -EINVAL;
+			goto out_exit_elevator;
 	} else {
 		if (WARN_ON(disk->minors))
-			return -EINVAL;
+			goto out_exit_elevator;
 
 		ret = blk_alloc_ext_minor();
 		if (ret < 0)
-			return ret;
+			goto out_exit_elevator;
 		disk->major = BLOCK_EXT_MAJOR;
 		disk->first_minor = ret;
 	}
@@ -571,6 +572,9 @@ int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
 out_free_ext_minor:
 	if (disk->major == BLOCK_EXT_MAJOR)
 		blk_free_ext_minor(disk->first_minor);
+out_exit_elevator:
+	if (disk->queue->elevator)
+		elevator_exit(disk->queue);
 	return ret;
 }
 EXPORT_SYMBOL(device_add_disk);
-- 
2.31.1

Re: [PATCH -next] block: fix memory leak for elevator on add_disk failure

[Ming Lei]

On Fri, Oct 21, 2022 at 11:39:50AM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> The default elevator is allocated in the begining of device_add_disk(),
> however, it's not freed in the following error path.
> 
> Fixes: 737eb78e82d5 ("block: Delay default elevator initialization")

Originally elevator can be teardown in disk release, or queue release
earlier time, so the fixes tag should be:

Fixes: 50e34d78815e ("block: disable the elevator int del_gendisk")


Thanks, 
Ming

Re: [PATCH -next] block: fix memory leak for elevator on add_disk failure

[Christoph Hellwig]

Looks good (with the updated Fixes tag as pointed out by Ming).

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH -next] block: fix memory leak for elevator on add_disk failure

[Yu Kuai]

在 2022/10/21 11:47, Ming Lei 写道:
> On Fri, Oct 21, 2022 at 11:39:50AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> The default elevator is allocated in the begining of device_add_disk(),
>> however, it's not freed in the following error path.
>>
>> Fixes: 737eb78e82d5 ("block: Delay default elevator initialization")
> 
> Originally elevator can be teardown in disk release, or queue release
> earlier time, so the fixes tag should be:
> 
> Fixes: 50e34d78815e ("block: disable the elevator int del_gendisk")

You're right! Thanks for the notice, I'll send a new version.

Kuai
> 
> 
> Thanks,
> Ming
> 
> .
>