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@xxxxxxxxxxxxxxxxx>
> Reviewed-by: James Morse <james.morse@xxxxxxx>
> Tested-by: James Morse <james.morse@xxxxxxx>
> ---
> 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.
