leak-detective: override malloc functions instead of using deprecated hooks
authorMartin Willi <martin@revosec.ch>
Tue, 2 Apr 2013 11:37:06 +0000 (13:37 +0200)
committerMartin Willi <martin@revosec.ch>
Mon, 6 May 2013 13:15:24 +0000 (15:15 +0200)
malloc hooks have become deprecated, and their use has always been problematic,
especially in multi-threaded applications. Replace the functionality by
overriding all malloc functions and query the system allocator functions
using dlsym() with RTLD_NEXT.

src/libstrongswan/utils/leak_detective.c

index 6bf4d63..69e539d 100644 (file)
@@ -1,5 +1,6 @@
 /*
- * Copyright (C) 2006-2008 Martin Willi
+ * Copyright (C) 2013 Tobias Brunner
+ * Copyright (C) 2006-2013 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
  * This program is free software; you can redistribute it and/or modify it
  */
 
 #define _GNU_SOURCE
-#include <sched.h>
 #include <stddef.h>
 #include <string.h>
 #include <stdio.h>
-#include <malloc.h>
 #include <signal.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <unistd.h>
 #include <syslog.h>
-#include <pthread.h>
 #include <netdb.h>
 #include <locale.h>
+#include <dlfcn.h>
 
 #include "leak_detective.h"
 
@@ -35,6 +34,8 @@
 #include <utils/debug.h>
 #include <utils/backtrace.h>
 #include <collections/hashtable.h>
+#include <threading/thread_value.h>
+#include <threading/spinlock.h>
 
 typedef struct private_leak_detective_t private_leak_detective_t;
 
@@ -69,17 +70,6 @@ struct private_leak_detective_t {
  */
 #define MEMORY_ALLOC_PATTERN 0xEE
 
-
-static void install_hooks(void);
-static void uninstall_hooks(void);
-static void *malloc_hook(size_t, const void *);
-static void *realloc_hook(void *, size_t, const void *);
-static void free_hook(void*, const void *);
-
-void *(*old_malloc_hook)(size_t, const void *);
-void *(*old_realloc_hook)(void *, size_t, const void *);
-void (*old_free_hook)(void*, const void *);
-
 static u_int count_malloc = 0;
 static u_int count_free = 0;
 static u_int count_realloc = 0;
@@ -136,47 +126,146 @@ struct memory_tail_t {
  * the others on it...
  */
 static memory_header_t first_header = {
-       magic: MEMORY_HEADER_MAGIC,
-       bytes: 0,
-       backtrace: NULL,
-       previous: NULL,
-       next: NULL
+       .magic = MEMORY_HEADER_MAGIC,
 };
 
 /**
- * are the hooks currently installed?
+ * Spinlock to access header linked list
  */
-static bool installed = FALSE;
+static spinlock_t *lock;
+
+/**
+ * Is leak detection currently enabled?
+ */
+static bool enabled = FALSE;
+
+/**
+ * Is leak detection disabled for the current thread?
+ */
+static thread_value_t *thread_disabled;
 
 /**
  * Installs the malloc hooks, enables leak detection
  */
-static void install_hooks()
+static void enable_leak_detective()
 {
-       if (!installed)
+       enabled = TRUE;
+}
+
+/**
+ * Uninstalls the malloc hooks, disables leak detection
+ */
+static void disable_leak_detective()
+{
+       enabled = FALSE;
+}
+
+/**
+ * Enable/Disable leak detective for the current thread
+ *
+ * @return Previous value
+ */
+static bool enable_thread(bool enable)
+{
+       bool before;
+
+       before = thread_disabled->get(thread_disabled) == NULL;
+       thread_disabled->set(thread_disabled, enable ? NULL : (void*)TRUE);
+       return before;
+}
+
+/**
+ * dlsym() might do a malloc(), but we can't do one before we get the malloc()
+ * function pointer. Use this minimalistic malloc implementation instead.
+ */
+static void* malloc_for_dlsym(size_t size)
+{
+       static char buf[1024] = {};
+       static size_t used = 0;
+       char *ptr;
+
+       /* roundup to a multiple of 32 */
+       size = (size - 1) / 32 * 32 + 32;
+
+       if (used + size > sizeof(buf))
        {
-               old_malloc_hook = __malloc_hook;
-               old_realloc_hook = __realloc_hook;
-               old_free_hook = __free_hook;
-               __malloc_hook = malloc_hook;
-               __realloc_hook = realloc_hook;
-               __free_hook = free_hook;
-               installed = TRUE;
+               return NULL;
        }
+       ptr = buf + used;
+       used += size;
+       return ptr;
 }
 
 /**
- * Uninstalls the malloc hooks, disables leak detection
+ * Lookup a malloc function, while disabling wrappers
+ */
+static void* get_malloc_fn(char *name)
+{
+       bool before = FALSE;
+       void *fn;
+
+       if (enabled)
+       {
+               before = enable_thread(FALSE);
+       }
+       fn = dlsym(RTLD_NEXT, name);
+       if (enabled)
+       {
+               enable_thread(before);
+       }
+       return fn;
+}
+
+/**
+ * Call original malloc()
+ */
+static void* real_malloc(size_t size)
+{
+       static void* (*fn)(size_t size);
+       static int recursive = 0;
+
+       if (!fn)
+       {
+               /* checking recursiveness should actually be thread-specific. But as
+                * it is very likely that the first allocation is done before we go
+                * multi-threaded, we keep it simple. */
+               if (recursive)
+               {
+                       return malloc_for_dlsym(size);
+               }
+               recursive++;
+               fn = get_malloc_fn("malloc");
+               recursive--;
+       }
+       return fn(size);
+}
+
+/**
+ * Call original free()
  */
-static void uninstall_hooks()
+static void real_free(void *ptr)
 {
-       if (installed)
+       static void (*fn)(void *ptr);
+
+       if (!fn)
        {
-               __malloc_hook = old_malloc_hook;
-               __free_hook = old_free_hook;
-               __realloc_hook = old_realloc_hook;
-               installed = FALSE;
+               fn = get_malloc_fn("free");
        }
+       return fn(ptr);
+}
+
+/**
+ * Call original realloc()
+ */
+static void* real_realloc(void *ptr, size_t size)
+{
+       static void* (*fn)(void *ptr, size_t size);
+
+       if (!fn)
+       {
+               fn = get_malloc_fn("realloc");
+       }
+       return fn(ptr, size);
 }
 
 /**
@@ -323,11 +412,13 @@ static int print_traces(private_leak_detective_t *this,
                /** number of allocations */
                u_int count;
        } *entry;
+       bool before;
 
-       uninstall_hooks();
+       before = enable_thread(FALSE);
 
        entries = hashtable_create((hashtable_hash_t)hash,
                                                           (hashtable_equals_t)equals, 1024);
+       lock->lock(lock);
        for (hdr = first_header.next; hdr != NULL; hdr = hdr->next)
        {
                if (whitelisted &&
@@ -354,6 +445,7 @@ static int print_traces(private_leak_detective_t *this,
                }
                leaks++;
        }
+       lock->unlock(lock);
        enumerator = entries->create_enumerator(entries);
        while (enumerator->enumerate(enumerator, NULL, &entry))
        {
@@ -368,7 +460,7 @@ static int print_traces(private_leak_detective_t *this,
        enumerator->destroy(enumerator);
        entries->destroy(entries);
 
-       install_hooks();
+       enable_thread(before);
        return leaks;
 }
 
@@ -403,85 +495,66 @@ METHOD(leak_detective_t, report, void,
 METHOD(leak_detective_t, set_state, bool,
        private_leak_detective_t *this, bool enable)
 {
-       static struct sched_param oldparams;
-       static int oldpolicy;
-       struct sched_param params;
-       pthread_t thread_id;
-
-       if (enable == installed)
+       if (enable == enabled)
        {
-               return installed;
+               return enabled;
        }
-       thread_id = pthread_self();
        if (enable)
        {
-               install_hooks();
-               pthread_setschedparam(thread_id, oldpolicy, &oldparams);
+               enable_leak_detective();
        }
        else
        {
-               pthread_getschedparam(thread_id, &oldpolicy, &oldparams);
-               params.__sched_priority = sched_get_priority_max(SCHED_FIFO);
-               pthread_setschedparam(thread_id, SCHED_FIFO, &params);
-               uninstall_hooks();
+               disable_leak_detective();
        }
-       installed = enable;
-       return !installed;
+       return !enabled;
 }
 
 METHOD(leak_detective_t, usage, void,
        private_leak_detective_t *this, FILE *out)
 {
-       int oldpolicy, thresh;
        bool detailed;
-       pthread_t thread_id = pthread_self();
-       struct sched_param oldparams, params;
+       int thresh;
 
        thresh = lib->settings->get_int(lib->settings,
                                        "libstrongswan.leak_detective.usage_threshold", 10240);
        detailed = lib->settings->get_bool(lib->settings,
                                        "libstrongswan.leak_detective.detailed", TRUE);
 
-       pthread_getschedparam(thread_id, &oldpolicy, &oldparams);
-       params.__sched_priority = sched_get_priority_max(SCHED_FIFO);
-       pthread_setschedparam(thread_id, SCHED_FIFO, &params);
-
        print_traces(this, out, thresh, detailed, NULL);
-
-       pthread_setschedparam(thread_id, oldpolicy, &oldparams);
 }
 
 /**
- * Hook function for malloc()
+ * Wrapped malloc() function
  */
-void *malloc_hook(size_t bytes, const void *caller)
+void* malloc(size_t bytes)
 {
        memory_header_t *hdr;
        memory_tail_t *tail;
-       pthread_t thread_id = pthread_self();
-       int oldpolicy;
-       struct sched_param oldparams, params;
+       bool before;
 
-       pthread_getschedparam(thread_id, &oldpolicy, &oldparams);
-
-       params.__sched_priority = sched_get_priority_max(SCHED_FIFO);
-       pthread_setschedparam(thread_id, SCHED_FIFO, &params);
+       if (!enabled || thread_disabled->get(thread_disabled))
+       {
+               return real_malloc(bytes);
+       }
 
        count_malloc++;
-       uninstall_hooks();
-       hdr = malloc(sizeof(memory_header_t) + bytes + sizeof(memory_tail_t));
+       hdr = real_malloc(sizeof(memory_header_t) + bytes + sizeof(memory_tail_t));
        tail = ((void*)hdr) + bytes + sizeof(memory_header_t);
        /* set to something which causes crashes */
        memset(hdr, MEMORY_ALLOC_PATTERN,
                   sizeof(memory_header_t) + bytes + sizeof(memory_tail_t));
 
+       before = enable_thread(FALSE);
+       hdr->backtrace = backtrace_create(2);
+       enable_thread(before);
+
        hdr->magic = MEMORY_HEADER_MAGIC;
        hdr->bytes = bytes;
-       hdr->backtrace = backtrace_create(2);
        tail->magic = MEMORY_TAIL_MAGIC;
-       install_hooks();
 
        /* insert at the beginning of the list */
+       lock->lock(lock);
        hdr->next = first_header.next;
        if (hdr->next)
        {
@@ -489,25 +562,40 @@ void *malloc_hook(size_t bytes, const void *caller)
        }
        hdr->previous = &first_header;
        first_header.next = hdr;
-
-       pthread_setschedparam(thread_id, oldpolicy, &oldparams);
+       lock->unlock(lock);
 
        return hdr + 1;
 }
 
 /**
- * Hook function for free()
+ * Wrapped calloc() function
  */
-void free_hook(void *ptr, const void *caller)
+void* calloc(size_t nmemb, size_t size)
+{
+       void *ptr;
+
+       size *= nmemb;
+       ptr = malloc(size);
+       memset(ptr, 0, size);
+
+       return ptr;
+}
+
+/**
+ * Wrapped free() function
+ */
+void free(void *ptr)
 {
        memory_header_t *hdr, *current;
        memory_tail_t *tail;
        backtrace_t *backtrace;
-       pthread_t thread_id = pthread_self();
-       int oldpolicy;
-       struct sched_param oldparams, params;
-       bool found = FALSE;
+       bool found = FALSE, before;
 
+       if (!enabled || thread_disabled->get(thread_disabled))
+       {
+               real_free(ptr);
+               return;
+       }
        /* allow freeing of NULL */
        if (ptr == NULL)
        {
@@ -516,16 +604,12 @@ void free_hook(void *ptr, const void *caller)
        hdr = ptr - sizeof(memory_header_t);
        tail = ptr + hdr->bytes;
 
-       pthread_getschedparam(thread_id, &oldpolicy, &oldparams);
-
-       params.__sched_priority = sched_get_priority_max(SCHED_FIFO);
-       pthread_setschedparam(thread_id, SCHED_FIFO, &params);
-
        count_free++;
-       uninstall_hooks();
+       before = enable_thread(FALSE);
        if (hdr->magic != MEMORY_HEADER_MAGIC ||
                tail->magic != MEMORY_TAIL_MAGIC)
        {
+               lock->lock(lock);
                for (current = &first_header; current != NULL; current = current->next)
                {
                        if (current == hdr)
@@ -534,6 +618,7 @@ void free_hook(void *ptr, const void *caller)
                                break;
                        }
                }
+               lock->unlock(lock);
                if (found)
                {
                        /* memory was allocated by our hooks but is corrupted */
@@ -544,7 +629,7 @@ void free_hook(void *ptr, const void *caller)
                else
                {
                        /* memory was not allocated by our hooks */
-                       fprintf(stderr, "freeing invalid memory (%p)", ptr);
+                       fprintf(stderr, "freeing invalid memory (%p)\n", ptr);
                }
                backtrace = backtrace_create(2);
                backtrace->log(backtrace, stderr, TRUE);
@@ -553,52 +638,49 @@ void free_hook(void *ptr, const void *caller)
        else
        {
                /* remove item from list */
+               lock->lock(lock);
                if (hdr->next)
                {
                        hdr->next->previous = hdr->previous;
                }
                hdr->previous->next = hdr->next;
+               lock->unlock(lock);
+
                hdr->backtrace->destroy(hdr->backtrace);
 
                /* clear MAGIC, set mem to something remarkable */
                memset(hdr, MEMORY_FREE_PATTERN,
                           sizeof(memory_header_t) + hdr->bytes + sizeof(memory_tail_t));
 
-               free(hdr);
+               real_free(hdr);
        }
-
-       install_hooks();
-       pthread_setschedparam(thread_id, oldpolicy, &oldparams);
+       enable_thread(before);
 }
 
 /**
- * Hook function for realloc()
+ * Wrapped realloc() function
  */
-void *realloc_hook(void *old, size_t bytes, const void *caller)
+void* realloc(void *old, size_t bytes)
 {
        memory_header_t *hdr;
        memory_tail_t *tail;
        backtrace_t *backtrace;
-       pthread_t thread_id = pthread_self();
-       int oldpolicy;
-       struct sched_param oldparams, params;
+       bool before;
 
+       if (!enabled || thread_disabled->get(thread_disabled))
+       {
+               return real_realloc(old, bytes);
+       }
        /* allow reallocation of NULL */
        if (old == NULL)
        {
-               return malloc_hook(bytes, caller);
+               return malloc(bytes);
        }
 
        hdr = old - sizeof(memory_header_t);
        tail = old + hdr->bytes;
 
-       pthread_getschedparam(thread_id, &oldpolicy, &oldparams);
-
-       params.__sched_priority = sched_get_priority_max(SCHED_FIFO);
-       pthread_setschedparam(thread_id, SCHED_FIFO, &params);
-
        count_realloc++;
-       uninstall_hooks();
        if (hdr->magic != MEMORY_HEADER_MAGIC ||
                tail->magic != MEMORY_TAIL_MAGIC)
        {
@@ -613,33 +695,37 @@ void *realloc_hook(void *old, size_t bytes, const void *caller)
                /* clear tail magic, allocate, set tail magic */
                memset(&tail->magic, MEMORY_ALLOC_PATTERN, sizeof(tail->magic));
        }
-       hdr = realloc(hdr, sizeof(memory_header_t) + bytes + sizeof(memory_tail_t));
+       hdr = real_realloc(hdr,
+                                          sizeof(memory_header_t) + bytes + sizeof(memory_tail_t));
        tail = ((void*)hdr) + bytes + sizeof(memory_header_t);
        tail->magic = MEMORY_TAIL_MAGIC;
 
        /* update statistics */
        hdr->bytes = bytes;
+
+       before = enable_thread(FALSE);
        hdr->backtrace->destroy(hdr->backtrace);
        hdr->backtrace = backtrace_create(2);
+       enable_thread(before);
 
        /* update header of linked list neighbours */
+       lock->lock(lock);
        if (hdr->next)
        {
                hdr->next->previous = hdr;
        }
        hdr->previous->next = hdr;
-       install_hooks();
-       pthread_setschedparam(thread_id, oldpolicy, &oldparams);
+       lock->unlock(lock);
+
        return hdr + 1;
 }
 
 METHOD(leak_detective_t, destroy, void,
        private_leak_detective_t *this)
 {
-       if (installed)
-       {
-               uninstall_hooks();
-       }
+       disable_leak_detective();
+       lock->destroy(lock);
+       thread_disabled->destroy(thread_disabled);
        free(this);
 }
 
@@ -659,20 +745,12 @@ leak_detective_t *leak_detective_create()
                },
        );
 
+       lock = spinlock_create();
+       thread_disabled = thread_value_create(NULL);
+
        if (getenv("LEAK_DETECTIVE_DISABLE") == NULL)
        {
-               cpu_set_t mask;
-
-               CPU_ZERO(&mask);
-               CPU_SET(0, &mask);
-
-               if (sched_setaffinity(0, sizeof(cpu_set_t), &mask) != 0)
-               {
-                       fprintf(stderr, "setting CPU affinity failed: %m");
-               }
-
-               install_hooks();
+               enable_leak_detective();
        }
        return &this->public;
 }
-