Added two simple unit tests for illegal calls to mem_free()

src/core/mem.c

@@ -66,6 +66,11 @@
 #include <stdlib.h> /* for malloc()/free() */
 #endif
 
+/* This is overridable for tests only... */
+#ifndef LWIP_MEM_ILLEGAL_FREE
+#define LWIP_MEM_ILLEGAL_FREE(msg)         LWIP_ASSERT(msg, 0)
+#endif
+
 #define MEM_STATS_INC_LOCKED(x)         SYS_ARCH_LOCKED(MEM_STATS_INC(x))
 #define MEM_STATS_INC_USED_LOCKED(x, y) SYS_ARCH_LOCKED(MEM_STATS_INC_USED(x, y))
 #define MEM_STATS_DEC_USED_LOCKED(x, y) SYS_ARCH_LOCKED(MEM_STATS_DEC_USED(x, y))
@@ -429,12 +434,20 @@ mem_free(void *rmem)
     LWIP_DEBUGF(MEM_DEBUG | LWIP_DBG_TRACE | LWIP_DBG_LEVEL_SERIOUS, ("mem_free(p == NULL) was called.\n"));
     return;
   }
-  LWIP_ASSERT("mem_free: sanity check alignment", (((mem_ptr_t)rmem) & (MEM_ALIGNMENT - 1)) == 0);
+  if ((((mem_ptr_t)rmem) & (MEM_ALIGNMENT - 1)) != 0) {
+    LWIP_MEM_ILLEGAL_FREE("mem_free: sanity check alignment");
+    LWIP_DEBUGF(MEM_DEBUG | LWIP_DBG_LEVEL_SEVERE, ("mem_free: sanity check alignment\n"));
+    /* protect mem stats from concurrent access */
+    MEM_STATS_INC_LOCKED(illegal);
+    return;
+  }
 
-  LWIP_ASSERT("mem_free: legal memory", (u8_t *)rmem >= (u8_t *)ram &&
-              (u8_t *)rmem < (u8_t *)ram_end);
+  /* Get the corresponding struct mem: */
+  /* cast through void* to get rid of alignment warnings */
+  mem = (struct mem *)(void *)((u8_t *)rmem - SIZEOF_STRUCT_MEM);
 
-  if ((u8_t *)rmem < (u8_t *)ram || (u8_t *)rmem >= (u8_t *)ram_end) {
+  if ((u8_t *)mem < ram || (u8_t *)rmem + MIN_SIZE_ALIGNED > (u8_t *)ram_end) {
+    LWIP_MEM_ILLEGAL_FREE("mem_free: illegal memory");
     LWIP_DEBUGF(MEM_DEBUG | LWIP_DBG_LEVEL_SEVERE, ("mem_free: illegal memory\n"));
     /* protect mem stats from concurrent access */
     MEM_STATS_INC_LOCKED(illegal);
@@ -442,11 +455,15 @@ mem_free(void *rmem)
   }
   /* protect the heap from concurrent access */
   LWIP_MEM_FREE_PROTECT();
-  /* Get the corresponding struct mem ... */
-  /* cast through void* to get rid of alignment warnings */
-  mem = (struct mem *)(void *)((u8_t *)rmem - SIZEOF_STRUCT_MEM);
-  /* ... which has to be in a used state ... */
-  LWIP_ASSERT("mem_free: mem->used", mem->used);
+  /* mem has to be in a used state ... */
+  if (!mem->used) {
+    LWIP_MEM_ILLEGAL_FREE("mem_free: illegal memory: double free");
+    LWIP_MEM_FREE_UNPROTECT();
+    LWIP_DEBUGF(MEM_DEBUG | LWIP_DBG_LEVEL_SEVERE, ("mem_free: illegal memory: double free?\n"));
+    /* protect mem stats from concurrent access */
+    MEM_STATS_INC_LOCKED(illegal);
+    return;
+  }
   /* ... and is now unused. */
   mem->used = 0;
 

test/unit/core/test_mem.c

@@ -111,13 +111,62 @@ START_TEST(test_mem_random)
 }
 END_TEST
 
+START_TEST(test_mem_invalid_free)
+{
+  u8_t *ptr, *ptr_low, *ptr_high;
+  LWIP_UNUSED_ARG(_i);
+
+  fail_unless(lwip_stats.mem.used == 0);
+  fail_unless(lwip_stats.mem.illegal == 0);
+
+  ptr = (u8_t *)mem_malloc(1);
+  fail_unless(ptr != NULL);
+
+  ptr_low = ptr - 0x10;
+  mem_free(ptr_low);
+  fail_unless(lwip_stats.mem.illegal == 1);
+  lwip_stats.mem.illegal = 0;
+
+  ptr_high = ptr + (MEM_SIZE * 2);
+  mem_free(ptr_high);
+  fail_unless(lwip_stats.mem.illegal == 1);
+  lwip_stats.mem.illegal = 0;
+
+  mem_free(ptr);
+  fail_unless(lwip_stats.mem.illegal == 0);
+  fail_unless(lwip_stats.mem.used == 0);
+}
+END_TEST
+
+START_TEST(test_mem_double_free)
+{
+  u8_t *ptr;
+  LWIP_UNUSED_ARG(_i);
+
+  fail_unless(lwip_stats.mem.used == 0);
+
+  ptr = (u8_t *)mem_malloc(1);
+  fail_unless(ptr != NULL);
+
+  mem_free(ptr);
+  fail_unless(lwip_stats.mem.illegal == 0);
+  fail_unless(lwip_stats.mem.used == 0);
+
+  mem_free(ptr);
+  fail_unless(lwip_stats.mem.illegal == 1);
+  fail_unless(lwip_stats.mem.used == 0);
+}
+END_TEST
+
 /** Create the suite including all tests for this module */
 Suite *
 mem_suite(void)
 {
   testfunc tests[] = {
     TESTFUNC(test_mem_one),
-    TESTFUNC(test_mem_random)
+    TESTFUNC(test_mem_random),
+    TESTFUNC(test_mem_invalid_free),
+    TESTFUNC(test_mem_double_free)
   };
   return create_suite("MEM", tests, sizeof(tests)/sizeof(testfunc), mem_setup, mem_teardown);
 }

test/unit/lwipopts.h

@@ -70,4 +70,7 @@
 /* MIB2 stats are required to check IPv4 reassembly results */
 #define MIB2_STATS                      1
 
+/* Check lwip_stats.mem.illegal instead of asserting */
+#define LWIP_MEM_ILLEGAL_FREE(msg)      /* to nothing */
+
 #endif /* LWIP_HDR_LWIPOPTS_H */