mem: Add magic field to detect memory corruption

Very little checking is performed on the validity of freelist
metadata. Add a magic field to struct arena_header which can be used
to detect memory corruption or whether a user tries to free something
that isn't a malloc pointer.

This feature can be enabled with -DDEBUG_MALLOC. Turn it on for
development in mk/devel.mk.

Cc: H. Peter Anvin <hpa@zytor.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>

6 files changed, 38 insertions, 6 deletions

diff --git a/core/mem/free.c b/core/mem/free.c
index 9c28e144..e8a9b048 100644
--- a/core/mem/free.c
+++ b/core/mem/free.c
@@ -79,7 +79,11 @@ __export void free(void *ptr)
         ((struct arena_header *)ptr - 1);
 
 #ifdef DEBUG_MALLOC
-    assert( ARENA_TYPE_GET(ah->a.attrs) == ARENA_TYPE_USED );
+    if (ah->a.magic != ARENA_MAGIC)
+	dprintf("failed free() magic check: %p\n", ptr);
+
+    if (ARENA_TYPE_GET(ah->a.attrs) != ARENA_TYPE_USED)
+	dprintf("invalid arena type: %d\n", ARENA_TYPE_GET(ah->a.attrs));
 #endif
 
     __free_block(ah);
diff --git a/core/mem/init.c b/core/mem/init.c
index abfe23ae..d6a5f189 100644
--- a/core/mem/init.c
+++ b/core/mem/init.c
@@ -34,6 +34,9 @@ int scan_highmem_area(void *data, addr_t start, addr_t len, bool is_ram)
 	if (len >= 2 * sizeof(struct arena_header)) {
 		fp = (struct free_arena_header *)start;
 		fp->a.attrs = ARENA_TYPE_USED | (HEAP_MAIN << ARENA_HEAP_POS);
+#ifdef DEBUG_MALLOC
+		fp->a.magic = ARENA_MAGIC;
+#endif
 		ARENA_SIZE_SET(fp->a.attrs, len);
 		dprintf("will inject a block start:0x%x size 0x%x", start, len);
 		__inject_free_block(fp);
@@ -88,6 +91,9 @@ void mem_init(void)
 	fp = (struct free_arena_header *)__lowmem_heap;
 	fp->a.attrs = ARENA_TYPE_USED | (HEAP_LOWMEM << ARENA_HEAP_POS);
 	ARENA_SIZE_SET(fp->a.attrs, (*bios_free_mem << 10) - (uintptr_t)fp);
+#ifdef DEBUG_MALLOC
+	fp->a.magic = ARENA_MAGIC;
+#endif
 	__inject_free_block(fp);
 
 	/* Initialize the main heap */
diff --git a/core/mem/malloc.c b/core/mem/malloc.c
index 3825f2a6..6cd3b33d 100644
--- a/core/mem/malloc.c
+++ b/core/mem/malloc.c
@@ -32,6 +32,9 @@ static void *__malloc_from_block(struct free_arena_header *fp,
 	ARENA_HEAP_SET(nfp->a.attrs, heap);
         ARENA_SIZE_SET(nfp->a.attrs, fsize-size);
         nfp->a.tag = MALLOC_FREE;
+#ifdef DEBUG_MALLOC
+	nfp->a.magic = ARENA_MAGIC;
+#endif
         ARENA_TYPE_SET(fp->a.attrs, ARENA_TYPE_USED);
         ARENA_SIZE_SET(fp->a.attrs, size);
         fp->a.tag = tag;
@@ -127,6 +130,11 @@ __export void *realloc(void *ptr, size_t size)
 
 	head = &__core_malloc_head[ARENA_HEAP_GET(ah->a.attrs)];
 
+#ifdef DEBUG_MALLOC
+    if (ah->a.magic != ARENA_MAGIC)
+	dprintf("failed realloc() magic check: %p\n", ptr);
+#endif
+
     /* Actual size of the old block */
     //oldsize = ah->a.size;
     oldsize = ARENA_SIZE_GET(ah->a.attrs);
@@ -167,6 +175,10 @@ __export void *realloc(void *ptr, size_t size)
 		ARENA_SIZE_SET(ah->a.attrs, newsize);
 		ARENA_HEAP_SET(nah->a.attrs, ARENA_HEAP_GET(ah->a.attrs));
 
+#ifdef DEBUG_MALLOC
+		nah->a.magic = ARENA_MAGIC;
+#endif
+
 		//nah->a.type = ARENA_TYPE_FREE;
 		//nah->a.size = xsize - newsize;
 		//ah->a.size = newsize;
diff --git a/core/mem/malloc.h b/core/mem/malloc.h
index 3291cc25..d0d135bc 100644
--- a/core/mem/malloc.h
+++ b/core/mem/malloc.h
@@ -32,6 +32,8 @@ enum heap {
     NHEAP
 };
 
+#define ARENA_MAGIC 0x20130117
+
 struct free_arena_header;
 
 /*
@@ -44,8 +46,19 @@ struct arena_header {
 					        2..3:  Heap,
 						4..31: MSB of the size  */
     struct free_arena_header *next, *prev;
+
+#ifdef DEBUG_MALLOC
+    unsigned long _pad[3];
+    unsigned int magic;
+#endif
 };
 
+/* Pad to 2*sizeof(struct arena_header) */
+#define ARENA_PADDING ((2 * sizeof(struct arena_header)) - \
+		       (sizeof(struct arena_header) + \
+			sizeof(struct free_arena_header *) +	\
+			sizeof(struct free_arena_header *)))
+
 /*
  * This structure should be no more than twice the size of the
  * previous structure.
@@ -53,7 +66,7 @@ struct arena_header {
 struct free_arena_header {
     struct arena_header a;
     struct free_arena_header *next_free, *prev_free;
-    size_t _pad[2];		/* Pad to 2*sizeof(struct arena_header) */
+    size_t _pad[ARENA_PADDING];
 };
 
 #define ARENA_SIZE_MASK (~(uintptr_t)(sizeof(struct arena_header)-1))
diff --git a/dos/free.c b/dos/free.c
index 020dc157..b0b72ef7 100644
--- a/dos/free.c
+++ b/dos/free.c
@@ -67,10 +67,6 @@ void free(void *ptr)
     ah = (struct free_arena_header *)
 	((struct arena_header *)ptr - 1);
 
-#ifdef DEBUG_MALLOC
-    assert(ah->a.type == ARENA_TYPE_USED);
-#endif
-
     __free_block(ah);
 
     /* Here we could insert code to return memory to the system. */
diff --git a/mk/devel.mk b/mk/devel.mk
index 8184c30f..d1a5fff2 100644
--- a/mk/devel.mk
+++ b/mk/devel.mk
@@ -1,3 +1,4 @@
 # Useful while doing development, but not for production.
 GCCWARN += -Wno-clobbered 
+#GCCWARN += -DDEBUG_MALLOC
 # GCCWARN += -DDEBUG_PORT=0x3f8 -DDEBUG=1