[PATCH 2.6.17-rc6] Fix the memory leak in acpi_evaluate_integer()

[PATCH 2.6.17-rc6] Fix the memory leak in acpi_evaluate_integer()

[Catalin Marinas]

From: Catalin Marinas <catalin.marinas@arm.com>

A leak can happen because of the early returns from this function
(discovered by kmemleak).

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---

 drivers/acpi/utils.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 6458c47..71afcd3 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -273,20 +273,22 @@ acpi_evaluate_integer(acpi_handle handle
 	status = acpi_evaluate_object(handle, pathname, arguments, &buffer);
 	if (ACPI_FAILURE(status)) {
 		acpi_util_eval_error(handle, pathname, status);
-		return_ACPI_STATUS(status);
+		goto out;
 	}
 
 	if (element->type != ACPI_TYPE_INTEGER) {
 		acpi_util_eval_error(handle, pathname, AE_BAD_DATA);
-		return_ACPI_STATUS(AE_BAD_DATA);
+		status = AE_BAD_DATA;
+		goto out;
 	}
 
 	*data = element->integer.value;
+ out:
 	kfree(element);
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Return value [%lu]\n", *data));
 
-	return_ACPI_STATUS(AE_OK);
+	return_ACPI_STATUS(status);
 }
 
 EXPORT_SYMBOL(acpi_evaluate_integer);

Re: [PATCH 2.6.17-rc6] Fix the memory leak in acpi_evaluate_integer()

[Michal Piotrowski]

Hi Catalin,

On 07/06/06, Catalin Marinas <catalin.marinas@arm.com> wrote:
> From: Catalin Marinas <catalin.marinas@arm.com>
>
> A leak can happen because of the early returns from this function
> (discovered by kmemleak).
>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---

all
orphan pointer 0xf7e97654 (size 40):
  c0151044: <kmem_cache_alloc>
  c01e8656: <acpi_os_acquire_object>
  c01fe087: <acpi_ut_allocate_object_desc_dbg>
  c01fdf1b: <acpi_ut_create_internal_object_dbg>
  c01ec2a0: <acpi_ev_execute_reg_method>
  c01ec898: <acpi_ev_reg_run>
  c01f7453: <acpi_ns_walk_namespace>
  c01ec859: <acpi_ev_execute_reg_methods>

are gone. But now I get a lot of

orphan pointer 0xf7ea1654 (size 40):
  c015208c: <kmem_cache_alloc>
  c01eb6c2: <acpi_os_acquire_object>
  c02010f7: <acpi_ut_allocate_object_desc_dbg>
  c0200f8b: <acpi_ut_create_internal_object_dbg>
  c01ef310: <acpi_ev_execute_reg_method>
  c01ef908: <acpi_ev_reg_run>
  c01fa4c3: <acpi_ns_walk_namespace>
  c01ef8c9: <acpi_ev_execute_reg_methods>
  c01ef293: <acpi_ev_initialize_op_regions>
  c02000d4: <acpi_initialize_objects>
  c03a0a37: <acpi_bus_init>
  c03a0ada: <acpi_init>
  c038e871: <do_initcalls>
  c038e91e: <do_basic_setup>
  c0100367: <init>

Regards,
Michal

-- 
Michal K. K. Piotrowski
LTG - Linux Testers Group
(http://www.stardust.webpages.pl/ltg/wiki/)

Re: [PATCH 2.6.17-rc6] Fix the memory leak in acpi_evaluate_integer()

[Michal Piotrowski]

On 07/06/06, Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:
> Hi Catalin,
>
> On 07/06/06, Catalin Marinas <catalin.marinas@arm.com> wrote:
> > From: Catalin Marinas <catalin.marinas@arm.com>
> >
> > A leak can happen because of the early returns from this function
> > (discovered by kmemleak).
> >
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
>
> all
> orphan pointer 0xf7e97654 (size 40):
>   c0151044: <kmem_cache_alloc>
>   c01e8656: <acpi_os_acquire_object>
>   c01fe087: <acpi_ut_allocate_object_desc_dbg>
>   c01fdf1b: <acpi_ut_create_internal_object_dbg>
>   c01ec2a0: <acpi_ev_execute_reg_method>
>   c01ec898: <acpi_ev_reg_run>
>   c01f7453: <acpi_ns_walk_namespace>
>   c01ec859: <acpi_ev_execute_reg_methods>
>
> are gone. But now I get a lot of
>
> orphan pointer 0xf7ea1654 (size 40):
>   c015208c: <kmem_cache_alloc>
>   c01eb6c2: <acpi_os_acquire_object>
>   c02010f7: <acpi_ut_allocate_object_desc_dbg>
>   c0200f8b: <acpi_ut_create_internal_object_dbg>
>   c01ef310: <acpi_ev_execute_reg_method>
>   c01ef908: <acpi_ev_reg_run>
>   c01fa4c3: <acpi_ns_walk_namespace>
>   c01ef8c9: <acpi_ev_execute_reg_methods>
>   c01ef293: <acpi_ev_initialize_op_regions>
>   c02000d4: <acpi_initialize_objects>
>   c03a0a37: <acpi_bus_init>
>   c03a0ada: <acpi_init>
>   c038e871: <do_initcalls>
>   c038e91e: <do_basic_setup>
>   c0100367: <init>

Ups, sorry for noise. I forgot about CONFIG_DEBUG_MEMLEAK_TRACE_LENGTH=16

Regards,
Michal

-- 
Michal K. K. Piotrowski
LTG - Linux Testers Group
(http://www.stardust.webpages.pl/ltg/wiki/)

Re: [PATCH 2.6.17-rc6] Fix the memory leak in acpi_evaluate_integer()

[Catalin Marinas]

On 07/06/06, Michal Piotrowski <michal.k.k.piotrowski@gmail.com> wrote:
> orphan pointer 0xf7ea1654 (size 40):
>   c015208c: <kmem_cache_alloc>
>   c01eb6c2: <acpi_os_acquire_object>
>   c02010f7: <acpi_ut_allocate_object_desc_dbg>
>   c0200f8b: <acpi_ut_create_internal_object_dbg>
>   c01ef310: <acpi_ev_execute_reg_method>
>   c01ef908: <acpi_ev_reg_run>
>   c01fa4c3: <acpi_ns_walk_namespace>
>   c01ef8c9: <acpi_ev_execute_reg_methods>
>   c01ef293: <acpi_ev_initialize_op_regions>
>   c02000d4: <acpi_initialize_objects>
>   c03a0a37: <acpi_bus_init>
>   c03a0ada: <acpi_init>
>   c038e871: <do_initcalls>
>   c038e91e: <do_basic_setup>
>   c0100367: <init>

I didn't manage to track this problem completely. It looks to me like
a real leak - two objects are allocated in acpi_ev_execute_reg_method
but, when returning from acpi_ns_evaluate_by_handle(), params[1] has
the reference_count = 2 (params[0] has this set to 1) and therefore
not released via acpi_ut_remove_reference().

-- 
Catalin