Linux-ACPI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] ACPI / APEI: release resources if gen_pool_add fails
@ 2019-06-17  1:27 luanshi
  2019-06-21 15:29 ` Borislav Petkov
  0 siblings, 1 reply; 3+ messages in thread
From: luanshi @ 2019-06-17  1:27 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: linux-acpi, James Morse, Tony Luck, Borislav Petkov

To avoid memory leaks, destroy ghes_estatus_pool and release memory
allocated via vmalloc() on errors in ghes_estatus_pool_init().

Signed-off-by: Liguang Zhang <zhangliguang@linux.alibaba.com>
Reviewed-by: James Morse <james.morse@arm.com>
Tested-by: James Morse <james.morse@arm.com>
---
 drivers/acpi/apei/ghes.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 993940d..4e5de30 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -153,6 +153,7 @@ static void ghes_unmap(void __iomem *vaddr, enum fixed_addresses fixmap_idx)
 int ghes_estatus_pool_init(int num_ghes)
 {
 	unsigned long addr, len;
+	int rc = 0;
 
 	ghes_estatus_pool = gen_pool_create(GHES_ESTATUS_POOL_MIN_ALLOC_ORDER, -1);
 	if (!ghes_estatus_pool)
@@ -163,8 +164,10 @@ int ghes_estatus_pool_init(int num_ghes)
 
 	ghes_estatus_pool_size_request = PAGE_ALIGN(len);
 	addr = (unsigned long)vmalloc(PAGE_ALIGN(len));
-	if (!addr)
+	if (!addr) {
+		gen_pool_destroy(ghes_estatus_pool);
 		return -ENOMEM;
+	}
 
 	/*
 	 * New allocation must be visible in all pgd before it can be found by
@@ -172,7 +175,12 @@ int ghes_estatus_pool_init(int num_ghes)
 	 */
 	vmalloc_sync_all();
 
-	return gen_pool_add(ghes_estatus_pool, addr, PAGE_ALIGN(len), -1);
+	rc = gen_pool_add(ghes_estatus_pool, addr, PAGE_ALIGN(len), -1);
+	if (rc) {
+		gen_pool_destroy(ghes_estatus_pool);
+		vfree((void *)addr);
+	}
+	return rc;
 }
 
 static int map_gen_v2(struct ghes *ghes)
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ACPI / APEI: release resources if gen_pool_add fails
  2019-06-17  1:27 [PATCH v2] ACPI / APEI: release resources if gen_pool_add fails luanshi
@ 2019-06-21 15:29 ` Borislav Petkov
  2019-06-23  2:48   ` 乱石
  0 siblings, 1 reply; 3+ messages in thread
From: Borislav Petkov @ 2019-06-21 15:29 UTC (permalink / raw)
  To: luanshi; +Cc: Rafael J. Wysocki, Len Brown, linux-acpi, James Morse, Tony Luck

On Mon, Jun 17, 2019 at 09:27:50AM +0800, luanshi wrote:
> To avoid memory leaks, destroy ghes_estatus_pool and release memory
> allocated via vmalloc() on errors in ghes_estatus_pool_init().
> 
> Signed-off-by: Liguang Zhang <zhangliguang@linux.alibaba.com>
> Reviewed-by: James Morse <james.morse@arm.com>
> Tested-by: James Morse <james.morse@arm.com>
> ---
>  drivers/acpi/apei/ghes.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 993940d..4e5de30 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -153,6 +153,7 @@ static void ghes_unmap(void __iomem *vaddr, enum fixed_addresses fixmap_idx)
>  int ghes_estatus_pool_init(int num_ghes)
>  {
>  	unsigned long addr, len;
> +	int rc = 0;
>  
>  	ghes_estatus_pool = gen_pool_create(GHES_ESTATUS_POOL_MIN_ALLOC_ORDER, -1);
>  	if (!ghes_estatus_pool)
> @@ -163,8 +164,10 @@ int ghes_estatus_pool_init(int num_ghes)
>  
>  	ghes_estatus_pool_size_request = PAGE_ALIGN(len);
>  	addr = (unsigned long)vmalloc(PAGE_ALIGN(len));
> -	if (!addr)
> +	if (!addr) {
> +		gen_pool_destroy(ghes_estatus_pool);
>  		return -ENOMEM;
> +	}
>  
>  	/*
>  	 * New allocation must be visible in all pgd before it can be found by
> @@ -172,7 +175,12 @@ int ghes_estatus_pool_init(int num_ghes)
>  	 */
>  	vmalloc_sync_all();
>  
> -	return gen_pool_add(ghes_estatus_pool, addr, PAGE_ALIGN(len), -1);
> +	rc = gen_pool_add(ghes_estatus_pool, addr, PAGE_ALIGN(len), -1);
> +	if (rc) {
> +		gen_pool_destroy(ghes_estatus_pool);
> +		vfree((void *)addr);
> +	}
> +	return rc;

Please put the error path in labels at the end of the function to which
you goto from each error case, like it is usually done in kernel code,
instead of repeating the free calls in each error handling path.

Grep the tree for examples, if you need some.

Thx.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] ACPI / APEI: release resources if gen_pool_add fails
  2019-06-21 15:29 ` Borislav Petkov
@ 2019-06-23  2:48   ` 乱石
  0 siblings, 0 replies; 3+ messages in thread
From: 乱石 @ 2019-06-23  2:48 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Rafael J. Wysocki, Len Brown, linux-acpi, James Morse, Tony Luck

Hi Borislav,

在 2019/6/21 23:29, Borislav Petkov 写道:
> On Mon, Jun 17, 2019 at 09:27:50AM +0800, luanshi wrote:
>> To avoid memory leaks, destroy ghes_estatus_pool and release memory
>> allocated via vmalloc() on errors in ghes_estatus_pool_init().
>>
>> Signed-off-by: Liguang Zhang <zhangliguang@linux.alibaba.com>
>> Reviewed-by: James Morse <james.morse@arm.com>
>> Tested-by: James Morse <james.morse@arm.com>
>> ---
>>   drivers/acpi/apei/ghes.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 993940d..4e5de30 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -153,6 +153,7 @@ static void ghes_unmap(void __iomem *vaddr, enum fixed_addresses fixmap_idx)
>>   int ghes_estatus_pool_init(int num_ghes)
>>   {
>>   	unsigned long addr, len;
>> +	int rc = 0;
>>   
>>   	ghes_estatus_pool = gen_pool_create(GHES_ESTATUS_POOL_MIN_ALLOC_ORDER, -1);
>>   	if (!ghes_estatus_pool)
>> @@ -163,8 +164,10 @@ int ghes_estatus_pool_init(int num_ghes)
>>   
>>   	ghes_estatus_pool_size_request = PAGE_ALIGN(len);
>>   	addr = (unsigned long)vmalloc(PAGE_ALIGN(len));
>> -	if (!addr)
>> +	if (!addr) {
>> +		gen_pool_destroy(ghes_estatus_pool);
>>   		return -ENOMEM;
>> +	}
>>   
>>   	/*
>>   	 * New allocation must be visible in all pgd before it can be found by
>> @@ -172,7 +175,12 @@ int ghes_estatus_pool_init(int num_ghes)
>>   	 */
>>   	vmalloc_sync_all();
>>   
>> -	return gen_pool_add(ghes_estatus_pool, addr, PAGE_ALIGN(len), -1);
>> +	rc = gen_pool_add(ghes_estatus_pool, addr, PAGE_ALIGN(len), -1);
>> +	if (rc) {
>> +		gen_pool_destroy(ghes_estatus_pool);
>> +		vfree((void *)addr);
>> +	}
>> +	return rc;
> Please put the error path in labels at the end of the function to which
> you goto from each error case, like it is usually done in kernel code,
> instead of repeating the free calls in each error handling path.

Thanks, I will modify this patch by your suggestion and send the patch 
v3 for review.

Thanks,

Liguang

>
> Grep the tree for examples, if you need some.
>
> Thx.
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-17  1:27 [PATCH v2] ACPI / APEI: release resources if gen_pool_add fails luanshi
2019-06-21 15:29 ` Borislav Petkov
2019-06-23  2:48   ` 乱石

Linux-ACPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \
		linux-acpi@vger.kernel.org linux-acpi@archiver.kernel.org
	public-inbox-index linux-acpi


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi


AGPL code for this site: git clone https://public-inbox.org/ public-inbox