Hi,
There are some possible kernel memory leaks discovered by kmemleak. I
didn't have time for investigating them. Please let me know if they are
not leaks so that I can improve kmemleak (or just add a false alarm call):
- acpi_evaluate_integer in drivers/acpi/utils.c - "element" is not freed
on the error path (if Coverity hasn't seen this, it was probably
confused by the return_* macros)
- acpi_ev_execute_reg_method in drivers/acpi/events/evregion.c - I'm not
sure about this but kmemleak reports an orphan pointer on the following
allocation path:
c0159372: <kmem_cache_alloc>
c01ffa07: <acpi_os_acquire_object>
c0215b3a: <acpi_ut_allocate_object_desc_dbg>
c02159ce: <acpi_ut_create_internal_object_dbg>
c0203784: <acpi_ev_execute_reg_method>
c0203db4: <acpi_ev_reg_run>
c020ed17: <acpi_ns_walk_namespace>
c0203d6b: <acpi_ev_execute_reg_methods>
Is acpi_ut_remove_reference actually removing the params[0/1]?
And some new false positives (kmemleak needs fixing):
- legacy_init_iomem_resources in arch/i386/kernel/setup.c - kmemleak is
probably right here in that "res" can never be freed since the pointer
was lost. However, there is no need to free this resource and that's why
I'll add a false alarm call
- there are a lot of false positives caused by module loading. I would
need to investigate these a bit more (it's missing a root memory block
that can lead to the reported pointers; it's also missing the static
variables in module)
Thanks,
Catalin
Catalin Marinas <[email protected]> wrote:
> - acpi_evaluate_integer in drivers/acpi/utils.c - "element" is not freed
> on the error path (if Coverity hasn't seen this, it was probably
> confused by the return_* macros)
This is simpler. I'll send a separate patch for it.
> - acpi_ev_execute_reg_method in drivers/acpi/events/evregion.c - I'm not
> sure about this but kmemleak reports an orphan pointer on the following
> allocation path:
> c0159372: <kmem_cache_alloc>
> c01ffa07: <acpi_os_acquire_object>
> c0215b3a: <acpi_ut_allocate_object_desc_dbg>
> c02159ce: <acpi_ut_create_internal_object_dbg>
> c0203784: <acpi_ev_execute_reg_method>
> c0203db4: <acpi_ev_reg_run>
> c020ed17: <acpi_ns_walk_namespace>
> c0203d6b: <acpi_ev_execute_reg_methods>
> Is acpi_ut_remove_reference actually removing the params[0/1]?
I'll need to enable the ACPI debug output as I can't find the leak by
only looking at the code. I'll let you know if there is a leak.
--
Catalin
From: Catalin Marinas <[email protected]>
A leak can happen because of the early returns from this function.
Signed-off-by: Catalin Marinas <[email protected]>
---
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);
Catalin Marinas <[email protected]> wrote:
> There are some possible kernel memory leaks discovered by kmemleak. I
> didn't have time for investigating them. Please let me know if they are
> not leaks so that I can improve kmemleak (or just add a false alarm call):
[...]
> - acpi_ev_execute_reg_method in drivers/acpi/events/evregion.c - I'm not
> sure about this but kmemleak reports an orphan pointer on the following
> allocation path:
> c0159372: <kmem_cache_alloc>
> c01ffa07: <acpi_os_acquire_object>
> c0215b3a: <acpi_ut_allocate_object_desc_dbg>
> c02159ce: <acpi_ut_create_internal_object_dbg>
> c0203784: <acpi_ev_execute_reg_method>
> c0203db4: <acpi_ev_reg_run>
> c020ed17: <acpi_ns_walk_namespace>
> c0203d6b: <acpi_ev_execute_reg_methods>
> Is acpi_ut_remove_reference actually removing the params[0/1]?
After a quick check, the reference counts after the
acpi_ns_evaluate_by_handle() call in acpi_ev_execute_reg_method look
like this (they were both 1 before this call):
params[0]->common.reference_count = 1
params[1]->common.reference_count = 2
and therefore acpi_ut_remove_reference() doesn't free
params[1]. Kmemleak, however, cannot find the params[1] value while
scanning the memory and therefore reports it as a leak.
Is this normal behaviour for the acpi_ev_execute_reg_method function?
There isn't anything obvious looking at the calling tree (which I
would say is pretty complex).
--
Catalin