From 8962be69c700224983af4effd2cd086f7f5800b0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 6 Nov 2008 20:48:13 -0800 Subject: Make us clean under valgrind --leak-check=full by using talloc_autofree_context() instead of NULL. Remove the code in memcache that does a TALLOC_FREE on stored pointers. That's a disaster waiting to happen. If you're storing talloc'ed pointers, you can't know their lifecycle and they should be deleted when their parent context is deleted, so freeing them at some arbitrary point later will be a double-free. Jeremy. --- source3/lib/memcache.c | 26 -------------------------- 1 file changed, 26 deletions(-) (limited to 'source3/lib/memcache.c') diff --git a/source3/lib/memcache.c b/source3/lib/memcache.c index 9c892fedfa..d586f707fa 100644 --- a/source3/lib/memcache.c +++ b/source3/lib/memcache.c @@ -40,37 +40,11 @@ struct memcache { static void memcache_element_parse(struct memcache_element *e, DATA_BLOB *key, DATA_BLOB *value); -static bool memcache_is_talloc(enum memcache_number n) -{ - bool result; - - switch (n) { - case GETPWNAM_CACHE: - case PDB_GETPWSID_CACHE: - case SINGLETON_CACHE_TALLOC: - result = true; - break; - default: - result = false; - break; - } - - return result; -} - static int memcache_destructor(struct memcache *cache) { struct memcache_element *e, *next; for (e = cache->mru; e != NULL; e = next) { next = e->next; - if (memcache_is_talloc((enum memcache_number)e->n) - && (e->valuelength == sizeof(void *))) { - DATA_BLOB key, value; - void *ptr; - memcache_element_parse(e, &key, &value); - memcpy(&ptr, value.data, sizeof(ptr)); - TALLOC_FREE(ptr); - } SAFE_FREE(e); } return 0; -- cgit From f50ad767505cc8847f5f574767b664b57326e468 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 13 Nov 2008 23:50:19 +0100 Subject: Actually finish memcache_add_talloc This fixes a memleak found by Martin Zielinski . Thanks for looking closely! Volker (cherry picked from commit a31a84a078100819809e6d40dbc3df207a50a0b2) --- source3/lib/memcache.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) (limited to 'source3/lib/memcache.c') diff --git a/source3/lib/memcache.c b/source3/lib/memcache.c index d586f707fa..1951b4abf9 100644 --- a/source3/lib/memcache.c +++ b/source3/lib/memcache.c @@ -40,6 +40,24 @@ struct memcache { static void memcache_element_parse(struct memcache_element *e, DATA_BLOB *key, DATA_BLOB *value); +static bool memcache_is_talloc(enum memcache_number n) +{ + bool result; + + switch (n) { + case GETPWNAM_CACHE: + case PDB_GETPWSID_CACHE: + case SINGLETON_CACHE_TALLOC: + result = true; + break; + default: + result = false; + break; + } + + return result; +} + static int memcache_destructor(struct memcache *cache) { struct memcache_element *e, *next; @@ -188,6 +206,16 @@ static void memcache_delete_element(struct memcache *cache, } DLIST_REMOVE(cache->mru, e); + if (memcache_is_talloc(e->n)) { + DATA_BLOB cache_key, cache_value; + void *ptr; + + memcache_element_parse(e, &cache_key, &cache_value); + SMB_ASSERT(cache_value.length == sizeof(ptr)); + memcpy(&ptr, cache_value.data, sizeof(ptr)); + TALLOC_FREE(ptr); + } + cache->size -= memcache_element_size(e->keylength, e->valuelength); SAFE_FREE(e); @@ -250,6 +278,12 @@ void memcache_add(struct memcache *cache, enum memcache_number n, memcache_element_parse(e, &cache_key, &cache_value); if (value.length <= cache_value.length) { + if (memcache_is_talloc(e->n)) { + void *ptr; + SMB_ASSERT(cache_value.length == sizeof(ptr)); + memcpy(&ptr, cache_value.data, sizeof(ptr)); + TALLOC_FREE(ptr); + } /* * We can reuse the existing record */ @@ -308,7 +342,8 @@ void memcache_add(struct memcache *cache, enum memcache_number n, void memcache_add_talloc(struct memcache *cache, enum memcache_number n, DATA_BLOB key, void *ptr) { - memcache_add(cache, n, key, data_blob_const(&ptr, sizeof(ptr))); + void *p = talloc_move(cache, &ptr); + memcache_add(cache, n, key, data_blob_const(&p, sizeof(p))); } void memcache_flush(struct memcache *cache, enum memcache_number n) -- cgit From 3c98d5bd987358b1cbeb81fa8db37b97492cf0cc Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Fri, 14 Nov 2008 13:42:54 +0100 Subject: Make memcache_add_talloc NULL out the source pointer This is an orthogonality measure to make clear this pointer now belongs to the cache. (cherry picked from commit e6080c6e87d6fe3995b121a772bf3f6343fa666f) --- source3/lib/memcache.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) (limited to 'source3/lib/memcache.c') diff --git a/source3/lib/memcache.c b/source3/lib/memcache.c index 1951b4abf9..eaff15deda 100644 --- a/source3/lib/memcache.c +++ b/source3/lib/memcache.c @@ -340,9 +340,19 @@ void memcache_add(struct memcache *cache, enum memcache_number n, } void memcache_add_talloc(struct memcache *cache, enum memcache_number n, - DATA_BLOB key, void *ptr) + DATA_BLOB key, void **pptr) { - void *p = talloc_move(cache, &ptr); + void **ptr = (void **)pptr; + void *p; + + if (cache == NULL) { + cache = global_cache; + } + if (cache == NULL) { + return; + } + + p = talloc_move(cache, ptr); memcache_add(cache, n, key, data_blob_const(&p, sizeof(p))); } -- cgit From df21095ce8981827767e8646b6aefb053beb29a8 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Sat, 15 Nov 2008 00:50:33 +0100 Subject: Attempt to fix the build I have no idea how this could have happened. Probably done a make and make test in a different tree than the one I have done the push from. Sorry. Volker --- source3/lib/memcache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source3/lib/memcache.c') diff --git a/source3/lib/memcache.c b/source3/lib/memcache.c index eaff15deda..606d72ab5a 100644 --- a/source3/lib/memcache.c +++ b/source3/lib/memcache.c @@ -340,7 +340,7 @@ void memcache_add(struct memcache *cache, enum memcache_number n, } void memcache_add_talloc(struct memcache *cache, enum memcache_number n, - DATA_BLOB key, void **pptr) + DATA_BLOB key, void *pptr) { void **ptr = (void **)pptr; void *p; -- cgit