mem-pool: invalidate memory on GF_FREE to aid debugging

Debugging where memory gets free'd with help from overwriting the memory
before it is free'd with some structures (repeatedly). The struct
mem_invalid starts with a magic value (0xdeadc0de), followed by a
pointer to the xlator, the mem-type. the size of the GF_?ALLOC()
requested area and the baseaddr pointer to what GF_?ALLOC() returned.

With these details, and the 'struct mem_header' that is placed when
calling GF_?ALLOC(), it is possible to identify overruns and possible
use-after-free. A memory dump (core) or running with a debugger is
needed to read the surrounding memory of corrupt structures.

This additional memory invalidation/poisoning needs to be enabled by
passing --enable-debug to ./configure.

Change-Id: I9f5f37dc4b5b59142adefc90897d32e89be67b82
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: http://review.gluster.org/10019
Reviewed-by: Venky Shankar <vshankar@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Emmanuel Dreyfus <manu@netbsd.org>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>

2 files changed, 103 insertions, 54 deletions

diff --git a/libglusterfs/src/mem-pool.c b/libglusterfs/src/mem-pool.c
index 3bba30fc72e..0f9d792c0f1 100644
--- a/libglusterfs/src/mem-pool.c
+++ b/libglusterfs/src/mem-pool.c
@@ -49,12 +49,13 @@ gf_mem_set_acct_info (xlator_t *xl, char **alloc_ptr, size_t size,
 		      uint32_t type, const char *typestr)
 {
 
-        char    *ptr = NULL;
+        void              *ptr    = NULL;
+        struct mem_header *header = NULL;
 
         if (!alloc_ptr)
                 return -1;
 
-        ptr = (char *) (*alloc_ptr);
+        ptr = *alloc_ptr;
 
         GF_ASSERT (xl != NULL);
 
@@ -78,18 +79,18 @@ gf_mem_set_acct_info (xlator_t *xl, char **alloc_ptr, size_t size,
         }
         UNLOCK(&xl->mem_acct.rec[type].lock);
 
-        *(uint32_t *)(ptr) = type;
-        ptr = ptr + 4;
-        memcpy (ptr, &size, sizeof(size_t));
-        ptr += sizeof (size_t);
-        memcpy (ptr, &xl, sizeof(xlator_t *));
-        ptr += sizeof (xlator_t *);
-        *(uint32_t *)(ptr) = GF_MEM_HEADER_MAGIC;
-        ptr = ptr + 4;
-        ptr = ptr + 8; //padding
+        header = (struct mem_header *) ptr;
+        header->type = type;
+        header->size = size;
+        header->xlator = xl;
+        header->magic = GF_MEM_HEADER_MAGIC;
+
+        ptr += sizeof (struct mem_header);
+
+        /* data follows in this gap of 'size' bytes */
         *(uint32_t *) (ptr + size) = GF_MEM_TRAILER_MAGIC;
 
-        *alloc_ptr = (void *)ptr;
+        *alloc_ptr = ptr;
         return 0;
 }
 
@@ -148,11 +149,11 @@ __gf_malloc (size_t size, uint32_t type, const char *typestr)
 void *
 __gf_realloc (void *ptr, size_t size)
 {
-        size_t          tot_size = 0;
-        char            *orig_ptr = NULL;
-        xlator_t        *xl = NULL;
-        uint32_t        type = 0;
-        char            *new_ptr;
+        uint32_t           type = 0;
+        size_t             tot_size = 0;
+        xlator_t          *xl = NULL;
+        char              *new_ptr;
+        struct mem_header *header = NULL;
 
         if (!THIS->ctx->mem_acct_enable)
                 return REALLOC (ptr, size);
@@ -161,17 +162,14 @@ __gf_realloc (void *ptr, size_t size)
 
         tot_size = size + GF_MEM_HEADER_SIZE + GF_MEM_TRAILER_SIZE;
 
-        orig_ptr = (char *)ptr - 8 - 4;
-
-        GF_ASSERT (*(uint32_t *)orig_ptr == GF_MEM_HEADER_MAGIC);
+        header = (struct mem_header *) (ptr - GF_MEM_HEADER_SIZE);
 
-        orig_ptr = orig_ptr - sizeof(xlator_t *);
-        xl = *((xlator_t **)orig_ptr);
+        GF_ASSERT (header->magic == GF_MEM_HEADER_MAGIC);
 
-        orig_ptr = (char *)ptr - GF_MEM_HEADER_SIZE;
-        type = *(uint32_t *)orig_ptr;
+        xl = (xlator_t *) header->xlator;
+        type = header->type;
 
-        new_ptr = realloc (orig_ptr, tot_size);
+        new_ptr = realloc (header, tot_size);
         if (!new_ptr) {
                 gf_msg_nomem ("", GF_LOG_ALERT, tot_size);
                 return NULL;
@@ -228,13 +226,53 @@ gf_asprintf (char **string_ptr, const char *format, ...)
         return rv;
 }
 
+#ifdef DEBUG
+void
+__gf_mem_invalidate (void *ptr)
+{
+        struct mem_header *header = ptr;
+        void              *end    = NULL;
+
+        struct mem_invalid inval = {
+                .magic = GF_MEM_INVALID_MAGIC,
+                .xlator = header->xlator,
+                .type = header->type,
+                .size = header->size,
+                .baseaddr = ptr + GF_MEM_HEADER_SIZE,
+        };
+
+        /* calculate the last byte of the allocated area */
+        end = ptr + GF_MEM_HEADER_SIZE + inval.size + GF_MEM_TRAILER_SIZE;
+
+        /* overwrite the old mem_header */
+        memcpy (ptr, &inval, sizeof (inval));
+        ptr += sizeof (inval);
+
+        /* zero out remaining (old) mem_header bytes) */
+        memset (ptr, 0x00, sizeof (*header) - sizeof (inval));
+        ptr += sizeof (*header) - sizeof (inval);
+
+        /* zero out the first byte of data */
+        *(uint32_t *)(ptr) = 0x00;
+        ptr += 1;
+
+        /* repeated writes of invalid structurein data area */
+        while ((ptr + (sizeof (inval))) < (end - 1)) {
+                memcpy (ptr, &inval, sizeof (inval));
+                ptr += sizeof (inval);
+        }
+
+        /* fill out remaining data area with 0xff */
+        memset (ptr, 0xff, end - ptr);
+}
+#endif /* DEBUG */
+
 void
 __gf_free (void *free_ptr)
 {
-        size_t          req_size = 0;
-        char            *ptr = NULL;
-        uint32_t        type = 0;
-        xlator_t        *xl = NULL;
+        void              *ptr = NULL;
+        xlator_t          *xl = NULL;
+        struct mem_header *header = NULL;
 
         if (!THIS->ctx->mem_acct_enable) {
                 FREE (free_ptr);
@@ -244,47 +282,39 @@ __gf_free (void *free_ptr)
         if (!free_ptr)
                 return;
 
-        ptr = (char *)free_ptr - 8 - 4;
+        ptr = free_ptr - GF_MEM_HEADER_SIZE;
+        header = (struct mem_header *) ptr;
 
         //Possible corruption, assert here
-        GF_ASSERT (GF_MEM_HEADER_MAGIC == *(uint32_t *)ptr);
-
-        *(uint32_t *)ptr = 0;
-
-        ptr = ptr - sizeof(xlator_t *);
-        memcpy (&xl, ptr, sizeof(xlator_t *));
+        GF_ASSERT (GF_MEM_HEADER_MAGIC == header->magic);
 
         //gf_free expects xl to be available
-        GF_ASSERT (xl != NULL);
+        GF_ASSERT (header->xlator != NULL);
+        xl = header->xlator;
 
         if (!xl->mem_acct.rec) {
-                ptr = (char *)free_ptr - GF_MEM_HEADER_SIZE;
                 goto free;
         }
 
-
-        ptr = ptr - sizeof(size_t);
-        memcpy (&req_size, ptr, sizeof (size_t));
-        ptr = ptr - 4;
-        type = *(uint32_t *)ptr;
-
         // This points to a memory overrun
         GF_ASSERT (GF_MEM_TRAILER_MAGIC ==
-                *(uint32_t *)((char *)free_ptr + req_size));
-
-        *(uint32_t *) ((char *)free_ptr + req_size) = 0;
+                *(uint32_t *)((char *)free_ptr + header->size));
 
-        LOCK (&xl->mem_acct.rec[type].lock);
+        LOCK (&xl->mem_acct.rec[header->type].lock);
         {
-                xl->mem_acct.rec[type].size -= req_size;
-                xl->mem_acct.rec[type].num_allocs--;
+                xl->mem_acct.rec[header->type].size -= header->size;
+                xl->mem_acct.rec[header->type].num_allocs--;
                 /* If all the instaces are freed up then ensure typestr is
                  * set to NULL */
-                if (!xl->mem_acct.rec[type].num_allocs)
-                        xl->mem_acct.rec[type].typestr = NULL;
+                if (!xl->mem_acct.rec[header->type].num_allocs)
+                        xl->mem_acct.rec[header->type].typestr = NULL;
         }
-        UNLOCK (&xl->mem_acct.rec[type].lock);
+        UNLOCK (&xl->mem_acct.rec[header->type].lock);
 free:
+#ifdef DEBUG
+        __gf_mem_invalidate (ptr);
+#endif
+
         FREE (ptr);
 }
 
diff --git a/libglusterfs/src/mem-pool.h b/libglusterfs/src/mem-pool.h
index 2bbb45ae8a7..81fb579a0ab 100644
--- a/libglusterfs/src/mem-pool.h
+++ b/libglusterfs/src/mem-pool.h
@@ -31,10 +31,10 @@
 #include <cmocka.h>
 #endif
 
-#define GF_MEM_HEADER_SIZE  (4 + sizeof (size_t) + sizeof (xlator_t *) + 4 + 8)
 #define GF_MEM_TRAILER_SIZE 8
 #define GF_MEM_HEADER_MAGIC  0xCAFEBABE
 #define GF_MEM_TRAILER_MAGIC 0xBAADF00D
+#define GF_MEM_INVALID_MAGIC 0xDEADC0DE
 
 struct mem_acct {
         uint32_t            num_types;
@@ -51,6 +51,25 @@ struct mem_acct_rec {
         gf_lock_t       lock;
 };
 
+struct mem_header {
+        uint32_t  type;
+        size_t    size;
+        void     *xlator;
+        uint32_t  magic;
+        int       padding[8];
+};
+
+#define GF_MEM_HEADER_SIZE  (sizeof (struct mem_header))
+
+#ifdef DEBUG
+struct mem_invalid {
+        uint32_t  magic;
+        void     *xlator;
+        uint32_t  type;
+        size_t    size;
+        void     *baseaddr;
+};
+#endif
 
 void *
 __gf_calloc (size_t cnt, size_t size, uint32_t type, const char *typestr);