diff mbox series

ACPI / APEI: release resources if gen_pool_add fails

Message ID 1560478845-19818-1-git-send-email-zhangliguang@linux.alibaba.com
State Changes Requested, archived
Headers show
Series ACPI / APEI: release resources if gen_pool_add fails | expand

Commit Message

Liguang Zhang June 14, 2019, 2:20 a.m. UTC
If gen_pool_add fails, then ensure that ghes_estatus_pool and vmalloc
resources are released to avoid memory leak.

Signed-off-by: liguang.zlg <zhangliguang@linux.alibaba.com>
---
 drivers/acpi/apei/ghes.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Rafael J. Wysocki June 14, 2019, 9:27 a.m. UTC | #1
On Fri, Jun 14, 2019 at 4:20 AM luanshi <zhangliguang@linux.alibaba.com> wrote:
>
> If gen_pool_add fails, then ensure that ghes_estatus_pool and vmalloc
> resources are released to avoid memory leak.

The patch does more than this, it also destroys ghes_estatus_pool if
vmalloc() fails.

What about using this changelog:

"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.zlg <zhangliguang@linux.alibaba.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..8472c96 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) {
> +               vfree(addr);
> +               gen_pool_destroy(ghes_estatus_pool);
> +       }
> +       return rc;
>  }
>
>  static int map_gen_v2(struct ghes *ghes)
> --
> 1.8.3.1
>
Liguang Zhang June 14, 2019, 9:42 a.m. UTC | #2
在 2019/6/14 17:27, Rafael J. Wysocki 写道:
> On Fri, Jun 14, 2019 at 4:20 AM luanshi <zhangliguang@linux.alibaba.com> wrote:
>> If gen_pool_add fails, then ensure that ghes_estatus_pool and vmalloc
>> resources are released to avoid memory leak.
> The patch does more than this, it also destroys ghes_estatus_pool if
> vmalloc() fails.
>
> What about using this changelog:
>
> "To avoid memory leaks, destroy ghes_estatus_pool and release memory
> allocated via vmalloc() on errors in ghes_estatus_pool_init()."

Thanks,

I will modify the changelog by your suggestion, and resend this patch.


Thanks,

Liguang

>
>> Signed-off-by: liguang.zlg <zhangliguang@linux.alibaba.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..8472c96 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) {
>> +               vfree(addr);
>> +               gen_pool_destroy(ghes_estatus_pool);
>> +       }
>> +       return rc;
>>   }
>>
>>   static int map_gen_v2(struct ghes *ghes)
>> --
>> 1.8.3.1
>>
diff mbox series

Patch

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 993940d..8472c96 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) {
+		vfree(addr);
+		gen_pool_destroy(ghes_estatus_pool);
+	}
+	return rc;
 }
 
 static int map_gen_v2(struct ghes *ghes)