Skip to contentSkip to conversation

kmemleak: do not leak object after tree insertion error (v2, fixed)

PATCH In review Sergey Senozhatsky 2 participants 3 messages Apr 2, 2012 – Apr 18, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kmemleak: do not leak object after tree insertion error (v2, fixed)

[PATCH] kmemleak: do not leak object after tree insertion error

In case when tree insertion fails due to already existing object error, pointer to allocated object gets lost due to lookup_object() overwrite. Free allocated object and return the existing one, obtained from lookup_object().

---

 mm/kmemleak.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)
mm/kmemleak.c +12 -7
index 45eb621..4177d83 100644
@@ -516,7 +516,7 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
 					     int min_count, gfp_t gfp)
 {
 	unsigned long flags;
-	struct kmemleak_object *object;
+	struct kmemleak_object *object, *ex_object;
 	struct prio_tree_node *node;
 
 	object = kmem_cache_alloc(object_cache, gfp_kmemleak_mask(gfp));
@@ -578,17 +578,22 @@ static struct kmemleak_object *create_object(unsigned long ptr, size_t size,
 	if (node != &object->tree_node) {
 		kmemleak_stop("Cannot insert 0x%lx into the object search tree "
 			      "(already existing)\n", ptr);
-		object = lookup_object(ptr, 1);
-		spin_lock(&object->lock);
-		dump_object_info(object);
-		spin_unlock(&object->lock);
+		ex_object = lookup_object(ptr, 1);
+		spin_lock(&ex_object->lock);
+		dump_object_info(ex_object);
+		spin_unlock(&ex_object->lock);
 
-		goto out;
+		goto out_error;
 	}
 	list_add_tail_rcu(&object->object_list, &object_list);
-out:
+
 	write_unlock_irqrestore(&kmemleak_lock, flags);
 	return object;
+out_error:
+	write_unlock_irqrestore(&kmemleak_lock, flags);
+	object->flags &= ~OBJECT_ALLOCATED;
+	put_object(object);
+	return ex_object;
 }
 
 /*

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#1 lore raw
Quoted 6 lines · Sergey Senozhatsky

[PATCH] kmemleak: do not leak object after tree insertion error

In case when tree insertion fails due to already existing object error, pointer to allocated object gets lost due to lookup_object() overwrite. Free allocated object and return the existing one, obtained from lookup_object().

We really need to return NULL if the tree insertion fails as kmemleak is disabled in this case (fatal condition for kmemleak). So we could just call kmem_cache_free(object_cache, object) in the 'if' block.

Signature · 8 lines
Catalin

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#2 lore raw
Quoted 12 lines · Catalin Marinas
Quoted 6 lines · Sergey Senozhatsky

[PATCH] kmemleak: do not leak object after tree insertion error

In case when tree insertion fails due to already existing object error, pointer to allocated object gets lost due to lookup_object() overwrite. Free allocated object and return the existing one, obtained from lookup_object().

We really need to return NULL if the tree insertion fails as kmemleak is disabled in this case (fatal condition for kmemleak). So we could just call kmem_cache_free(object_cache, object) in the 'if' block.

Good point. Thanks a lot for your review!
I was chasing two bugs and sort of messed things up. I'll send v3 shortly.

Sergey

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>