Commit e781a9ab authored by Catalin Marinas's avatar Catalin Marinas Committed by Linus Torvalds

mm: kmemleak: fix delete_object_*() race when called on the same memory block

Calling delete_object_*() on the same pointer is not a standard use case
(unless there is a bug in the code calling kmemleak_free()).  However,
during kmemleak disabling (error or user triggered via /sys), there is a
potential race between kmemleak_free() calls on a CPU and
__kmemleak_do_cleanup() on a different CPU.

The current delete_object_*() implementation first performs a look-up
holding kmemleak_lock, increments the object->use_count and then
re-acquires kmemleak_lock to remove the object from object_tree_root and
object_list.

This patch simplifies the delete_object_*() mechanism to both look up
and remove an object from the object_tree_root and object_list
atomically (guarded by kmemleak_lock).  This allows safe concurrent
calls to delete_object_*() on the same pointer without additional
locking for synchronising the kmemleak_free_enabled flag.

A side effect is a slight improvement in the delete_object_*() performance
by avoiding acquiring kmemleak_lock twice and incrementing/decrementing
object->use_count.
Signed-off-by: default avatarCatalin Marinas <catalin.marinas@arm.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent c5f3b1a5
...@@ -497,6 +497,27 @@ static struct kmemleak_object *find_and_get_object(unsigned long ptr, int alias) ...@@ -497,6 +497,27 @@ static struct kmemleak_object *find_and_get_object(unsigned long ptr, int alias)
return object; return object;
} }
/*
* Look up an object in the object search tree and remove it from both
* object_tree_root and object_list. The returned object's use_count should be
* at least 1, as initially set by create_object().
*/
static struct kmemleak_object *find_and_remove_object(unsigned long ptr, int alias)
{
unsigned long flags;
struct kmemleak_object *object;
write_lock_irqsave(&kmemleak_lock, flags);
object = lookup_object(ptr, alias);
if (object) {
rb_erase(&object->rb_node, &object_tree_root);
list_del_rcu(&object->object_list);
}
write_unlock_irqrestore(&kmemleak_lock, flags);
return object;
}
/* /*
* Save stack trace to the given array of MAX_TRACE size. * Save stack trace to the given array of MAX_TRACE size.
*/ */
...@@ -600,20 +621,14 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size, ...@@ -600,20 +621,14 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
} }
/* /*
* Remove the metadata (struct kmemleak_object) for a memory block from the * Mark the object as not allocated and schedule RCU freeing via put_object().
* object_list and object_tree_root and decrement its use_count.
*/ */
static void __delete_object(struct kmemleak_object *object) static void __delete_object(struct kmemleak_object *object)
{ {
unsigned long flags; unsigned long flags;
write_lock_irqsave(&kmemleak_lock, flags);
rb_erase(&object->rb_node, &object_tree_root);
list_del_rcu(&object->object_list);
write_unlock_irqrestore(&kmemleak_lock, flags);
WARN_ON(!(object->flags & OBJECT_ALLOCATED)); WARN_ON(!(object->flags & OBJECT_ALLOCATED));
WARN_ON(atomic_read(&object->use_count) < 2); WARN_ON(atomic_read(&object->use_count) < 1);
/* /*
* Locking here also ensures that the corresponding memory block * Locking here also ensures that the corresponding memory block
...@@ -633,7 +648,7 @@ static void delete_object_full(unsigned long ptr) ...@@ -633,7 +648,7 @@ static void delete_object_full(unsigned long ptr)
{ {
struct kmemleak_object *object; struct kmemleak_object *object;
object = find_and_get_object(ptr, 0); object = find_and_remove_object(ptr, 0);
if (!object) { if (!object) {
#ifdef DEBUG #ifdef DEBUG
kmemleak_warn("Freeing unknown object at 0x%08lx\n", kmemleak_warn("Freeing unknown object at 0x%08lx\n",
...@@ -642,7 +657,6 @@ static void delete_object_full(unsigned long ptr) ...@@ -642,7 +657,6 @@ static void delete_object_full(unsigned long ptr)
return; return;
} }
__delete_object(object); __delete_object(object);
put_object(object);
} }
/* /*
...@@ -655,7 +669,7 @@ static void delete_object_part(unsigned long ptr, size_t size) ...@@ -655,7 +669,7 @@ static void delete_object_part(unsigned long ptr, size_t size)
struct kmemleak_object *object; struct kmemleak_object *object;
unsigned long start, end; unsigned long start, end;
object = find_and_get_object(ptr, 1); object = find_and_remove_object(ptr, 1);
if (!object) { if (!object) {
#ifdef DEBUG #ifdef DEBUG
kmemleak_warn("Partially freeing unknown object at 0x%08lx " kmemleak_warn("Partially freeing unknown object at 0x%08lx "
...@@ -663,7 +677,6 @@ static void delete_object_part(unsigned long ptr, size_t size) ...@@ -663,7 +677,6 @@ static void delete_object_part(unsigned long ptr, size_t size)
#endif #endif
return; return;
} }
__delete_object(object);
/* /*
* Create one or two objects that may result from the memory block * Create one or two objects that may result from the memory block
...@@ -681,7 +694,7 @@ static void delete_object_part(unsigned long ptr, size_t size) ...@@ -681,7 +694,7 @@ static void delete_object_part(unsigned long ptr, size_t size)
create_object(ptr + size, end - ptr - size, object->min_count, create_object(ptr + size, end - ptr - size, object->min_count,
GFP_KERNEL); GFP_KERNEL);
put_object(object); __delete_object(object);
} }
static void __paint_it(struct kmemleak_object *object, int color) static void __paint_it(struct kmemleak_object *object, int color)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment