leak-detective: Add an option to ignore frees of unknown memory blocks
authorTobias Brunner <tobias@strongswan.org>
Wed, 8 Aug 2018 15:06:15 +0000 (17:06 +0200)
committerTobias Brunner <tobias@strongswan.org>
Wed, 12 Sep 2018 14:25:00 +0000 (16:25 +0200)
This also changes how unknown/corrupted memory is handled in the free()
and realloc() hooks in general.

Incorporates changes provided by Thomas Egerer who ran into a similar
issue.

src/libstrongswan/utils/leak_detective.c

index 08fcc0e..efeb0f4 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2014 Tobias Brunner
+ * Copyright (C) 2013-2018 Tobias Brunner
  * Copyright (C) 2006-2013 Martin Willi
  * HSR Hochschule fuer Technik Rapperswil
  *
@@ -162,7 +162,12 @@ static spinlock_t *lock;
 /**
  * Is leak detection currently enabled?
  */
-static bool enabled = FALSE;
+static bool enabled;
+
+/**
+ * Whether to report calls to free() with memory not allocated by us
+ */
+static bool ignore_unknown;
 
 /**
  * Is leak detection disabled for the current thread?
@@ -888,7 +893,7 @@ HOOK(void, free, void *ptr)
                return;
        }
        /* allow freeing of NULL */
-       if (ptr == NULL)
+       if (!ptr)
        {
                return;
        }
@@ -899,21 +904,47 @@ HOOK(void, free, void *ptr)
        if (hdr->magic != MEMORY_HEADER_MAGIC ||
                tail->magic != MEMORY_TAIL_MAGIC)
        {
+               bool bt = TRUE;
+
+               /* check if memory appears to be allocated by our hooks */
                if (has_hdr(hdr))
                {
-                       /* memory was allocated by our hooks but is corrupted */
                        fprintf(stderr, "freeing corrupted memory (%p): "
-                                       "header magic 0x%x, tail magic 0x%x:\n",
-                                       ptr, hdr->magic, tail->magic);
+                                       "%u bytes, header magic 0x%x, tail magic 0x%x:\n",
+                                       ptr, hdr->bytes, hdr->magic, tail->magic);
+                       remove_hdr(hdr);
+
+                       if (hdr->magic == MEMORY_HEADER_MAGIC)
+                       {       /* only access the old backtrace if header magic is valid */
+                               hdr->backtrace->log(hdr->backtrace, stderr, TRUE);
+                               hdr->backtrace->destroy(hdr->backtrace);
+                       }
+                       else
+                       {
+                               fprintf(stderr, " header magic invalid, ignore backtrace of "
+                                               "allocation\n");
+                       }
                }
                else
                {
-                       /* memory was not allocated by our hooks */
-                       fprintf(stderr, "freeing invalid memory (%p)\n", ptr);
+                       /* just free this block of unknown memory */
+                       hdr = ptr;
+
+                       if (ignore_unknown)
+                       {
+                               bt = FALSE;
+                       }
+                       else
+                       {
+                               fprintf(stderr, "freeing unknown memory (%p):\n", ptr);
+                       }
+               }
+               if (bt)
+               {
+                       backtrace = backtrace_create(2);
+                       backtrace->log(backtrace, stderr, TRUE);
+                       backtrace->destroy(backtrace);
                }
-               backtrace = backtrace_create(2);
-               backtrace->log(backtrace, stderr, TRUE);
-               backtrace->destroy(backtrace);
        }
        else
        {
@@ -921,12 +952,11 @@ HOOK(void, free, void *ptr)
 
                hdr->backtrace->destroy(hdr->backtrace);
 
-               /* clear MAGIC, set mem to something remarkable */
+               /* set mem to something remarkable */
                memset(hdr, MEMORY_FREE_PATTERN,
                           sizeof(memory_header_t) + hdr->bytes + sizeof(memory_tail_t));
-
-               real_free(hdr);
        }
+       real_free(hdr);
        enable_thread(before);
 }
 
@@ -938,19 +968,19 @@ HOOK(void*, realloc, void *old, size_t bytes)
        memory_header_t *hdr;
        memory_tail_t *tail;
        backtrace_t *backtrace;
-       bool before;
+       bool before, have_backtrace = TRUE;
 
        if (!enabled || thread_disabled->get(thread_disabled))
        {
                return real_realloc(old, bytes);
        }
        /* allow reallocation of NULL */
-       if (old == NULL)
+       if (!old)
        {
                return malloc(bytes);
        }
        /* handle zero size as a free() */
-       if (bytes == 0)
+       if (!bytes)
        {
                free(old);
                return NULL;
@@ -959,22 +989,64 @@ HOOK(void*, realloc, void *old, size_t bytes)
        hdr = old - sizeof(memory_header_t);
        tail = old + hdr->bytes;
 
-       remove_hdr(hdr);
-
+       before = enable_thread(FALSE);
        if (hdr->magic != MEMORY_HEADER_MAGIC ||
                tail->magic != MEMORY_TAIL_MAGIC)
        {
-               fprintf(stderr, "reallocating invalid memory (%p):\n"
-                               "header magic 0x%x:\n", old, hdr->magic);
-               backtrace = backtrace_create(2);
-               backtrace->log(backtrace, stderr, TRUE);
-               backtrace->destroy(backtrace);
+               bool bt = TRUE;
+
+               /* check if memory appears to be allocated by our hooks */
+               if (has_hdr(hdr))
+               {
+                       fprintf(stderr, "reallocating corrupted memory (%p, %u bytes): "
+                                       "%zu bytes, header magic 0x%x, tail magic 0x%x:\n",
+                                       old, hdr->bytes, bytes, hdr->magic, tail->magic);
+                       remove_hdr(hdr);
+
+                       if (hdr->magic == MEMORY_HEADER_MAGIC)
+                       {       /* only access header fields (backtrace, bytes) if header magic
+                                * is still valid */
+                               hdr->backtrace->log(hdr->backtrace, stderr, TRUE);
+                               memset(&tail->magic, MEMORY_ALLOC_PATTERN, sizeof(tail->magic));
+                       }
+                       else
+                       {
+                               fprintf(stderr, " header magic invalid, ignore backtrace of "
+                                               "allocation\n");
+                               have_backtrace = FALSE;
+                               hdr->magic = MEMORY_HEADER_MAGIC;
+                       }
+               }
+               else
+               {
+                       /* adopt this block of unknown memory */
+                       hdr = old;
+                       have_backtrace = FALSE;
+
+                       if (ignore_unknown)
+                       {
+                               bt = FALSE;
+                       }
+                       else
+                       {
+                               fprintf(stderr, "reallocating unknown memory (%p): %zu bytes:\n",
+                                               old, bytes);
+                       }
+               }
+               if (bt)
+               {
+                       backtrace = backtrace_create(2);
+                       backtrace->log(backtrace, stderr, TRUE);
+                       backtrace->destroy(backtrace);
+               }
        }
        else
        {
+               remove_hdr(hdr);
                /* clear tail magic, allocate, set tail magic */
                memset(&tail->magic, MEMORY_ALLOC_PATTERN, sizeof(tail->magic));
        }
+
        hdr = real_realloc(hdr,
                                           sizeof(memory_header_t) + bytes + sizeof(memory_tail_t));
        tail = ((void*)hdr) + bytes + sizeof(memory_header_t);
@@ -983,8 +1055,10 @@ HOOK(void*, realloc, void *old, size_t bytes)
        /* update statistics */
        hdr->bytes = bytes;
 
-       before = enable_thread(FALSE);
-       hdr->backtrace->destroy(hdr->backtrace);
+       if (have_backtrace)
+       {
+               hdr->backtrace->destroy(hdr->backtrace);
+       }
        hdr->backtrace = backtrace_create(2);
        enable_thread(before);
 
@@ -1027,6 +1101,7 @@ leak_detective_t *leak_detective_create()
                free(this);
                return NULL;
        }
+       ignore_unknown = getenv("LEAK_DETECTIVE_IGNORE_UNKNOWN") != NULL;
 
        lock = spinlock_create();
        thread_disabled = thread_value_create(NULL);