Possible kernel memory leaks

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

This is simpler. I'll send a separate patch for it.

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.

---

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

index 6458c47..71afcd3 100644
@@ -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);

[...]

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

I'll keep investigating this as I think its a real object leak. Looking at why params[1] has a different reference_count from params[0], led me to the following backtrace on the ref count increment (that's getting really complicated):

acpi_ut_add_reference
acpi_ds_method_data_get_value acpi_ex_resolve_object_to_value
acpi_ex_resolve_to_value
acpi_ex_resolve_operands
(I have a suspicion that the above function should call
acpi_ut_remove_reference(obj_desc) on an error return path but it actually doesn't and, therefore, the ref count remains incremented. In this function, params[0] ref count is 3 but the one for params[1] becomes 4)
acpi_ds_exec_end_op (called via walk_state->ascending_callback) acpi_ps_parse_loop
acpi_ps_parse_aml
acpi_ps_execute_pass
acpi_ps_execute_method
acpi_ns_execute_control_method
acpi_ns_evaluate_by_handle
acpi_ev_execute_reg_method

Any suggestions/hints? I hope to get to the bottom of this in the next few days.

Thanks.

Catalin