Skip to content

Commit

Permalink
ACPICA: fix leak of acpi_os_validate_address
Browse files Browse the repository at this point in the history
http://bugzilla.kernel.org/show_bug.cgi?id=13620

If the dynamic region is created and added to resource list over and over again,
it has the potential to be a memory leak by growing the list every time.

This patch fixes the memory leak, as below

1) add a new field "count" to struct acpi_res_list.

   When inserting, if the region(addr, len) is already in the resource
   list, we just increase "count", otherwise, the region is inserted
   with count=1.

   When deleting, the "count" is decreased, if it's decreased to 0,
   the region is deleted from the resource list.

   With "count", the region with same address and length can only be
   inserted to the resource list once, so prevent potential memory leak.

2) add a new function acpi_os_invalidate_address, which is called when
   region is deleted.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
  • Loading branch information
Lin Ming authored and Len Brown committed Aug 27, 2009
1 parent 422bef8 commit a5fe1a0
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 3 deletions.
6 changes: 6 additions & 0 deletions drivers/acpi/acpica/utdelete.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@ static void acpi_ut_delete_internal_obj(union acpi_operand_object *object)
ACPI_DEBUG_PRINT((ACPI_DB_ALLOCATIONS,
"***** Region %p\n", object));

/* Invalidate the region address/length via the host OS */

acpi_os_invalidate_address(object->region.space_id,
object->region.address,
(acpi_size) object->region.length);

second_desc = acpi_ns_get_secondary_object(object);
if (second_desc) {
/*
Expand Down
94 changes: 91 additions & 3 deletions drivers/acpi/osl.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ struct acpi_res_list {
char name[5]; /* only can have a length of 4 chars, make use of this
one instead of res->name, no need to kalloc then */
struct list_head resource_list;
int count;
};

static LIST_HEAD(resource_list_head);
Expand Down Expand Up @@ -1358,6 +1359,89 @@ acpi_os_validate_interface (char *interface)
return AE_SUPPORT;
}

static inline int acpi_res_list_add(struct acpi_res_list *res)
{
struct acpi_res_list *res_list_elem;

list_for_each_entry(res_list_elem, &resource_list_head,
resource_list) {

if (res->resource_type == res_list_elem->resource_type &&
res->start == res_list_elem->start &&
res->end == res_list_elem->end) {

/*
* The Region(addr,len) already exist in the list,
* just increase the count
*/

res_list_elem->count++;
return 0;
}
}

res->count = 1;
list_add(&res->resource_list, &resource_list_head);
return 1;
}

static inline void acpi_res_list_del(struct acpi_res_list *res)
{
struct acpi_res_list *res_list_elem;

list_for_each_entry(res_list_elem, &resource_list_head,
resource_list) {

if (res->resource_type == res_list_elem->resource_type &&
res->start == res_list_elem->start &&
res->end == res_list_elem->end) {

/*
* If the res count is decreased to 0,
* remove and free it
*/

if (--res_list_elem->count == 0) {
list_del(&res_list_elem->resource_list);
kfree(res_list_elem);
}
return;
}
}
}

acpi_status
acpi_os_invalidate_address(
u8 space_id,
acpi_physical_address address,
acpi_size length)
{
struct acpi_res_list res;

switch (space_id) {
case ACPI_ADR_SPACE_SYSTEM_IO:
case ACPI_ADR_SPACE_SYSTEM_MEMORY:
/* Only interference checks against SystemIO and SytemMemory
are needed */
res.start = address;
res.end = address + length - 1;
res.resource_type = space_id;
spin_lock(&acpi_res_lock);
acpi_res_list_del(&res);
spin_unlock(&acpi_res_lock);
break;
case ACPI_ADR_SPACE_PCI_CONFIG:
case ACPI_ADR_SPACE_EC:
case ACPI_ADR_SPACE_SMBUS:
case ACPI_ADR_SPACE_CMOS:
case ACPI_ADR_SPACE_PCI_BAR_TARGET:
case ACPI_ADR_SPACE_DATA_TABLE:
case ACPI_ADR_SPACE_FIXED_HARDWARE:
break;
}
return AE_OK;
}

/******************************************************************************
*
* FUNCTION: acpi_os_validate_address
Expand All @@ -1382,6 +1466,7 @@ acpi_os_validate_address (
char *name)
{
struct acpi_res_list *res;
int added;
if (acpi_enforce_resources == ENFORCE_RESOURCES_NO)
return AE_OK;

Expand All @@ -1399,14 +1484,17 @@ acpi_os_validate_address (
res->end = address + length - 1;
res->resource_type = space_id;
spin_lock(&acpi_res_lock);
list_add(&res->resource_list, &resource_list_head);
added = acpi_res_list_add(res);
spin_unlock(&acpi_res_lock);
pr_debug("Added %s resource: start: 0x%llx, end: 0x%llx, "
"name: %s\n", (space_id == ACPI_ADR_SPACE_SYSTEM_IO)
pr_debug("%s %s resource: start: 0x%llx, end: 0x%llx, "
"name: %s\n", added ? "Added" : "Already exist",
(space_id == ACPI_ADR_SPACE_SYSTEM_IO)
? "SystemIO" : "System Memory",
(unsigned long long)res->start,
(unsigned long long)res->end,
res->name);
if (!added)
kfree(res);
break;
case ACPI_ADR_SPACE_PCI_CONFIG:
case ACPI_ADR_SPACE_EC:
Expand Down
3 changes: 3 additions & 0 deletions include/acpi/acpiosxf.h
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ acpi_status acpi_osi_invalidate(char* interface);
acpi_status
acpi_os_validate_address(u8 space_id, acpi_physical_address address,
acpi_size length, char *name);
acpi_status
acpi_os_invalidate_address(u8 space_id, acpi_physical_address address,
acpi_size length);

u64 acpi_os_get_timer(void);

Expand Down

0 comments on commit a5fe1a0

Please sign in to comment.