diff --git a/src/core/mem.c b/src/core/mem.c index 8a77a670..14bdd904 100644 --- a/src/core/mem.c +++ b/src/core/mem.c @@ -66,6 +66,11 @@ #include /* 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; diff --git a/test/unit/core/test_mem.c b/test/unit/core/test_mem.c index c362758d..8c6f7d0b 100644 --- a/test/unit/core/test_mem.c +++ b/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); } diff --git a/test/unit/lwipopts.h b/test/unit/lwipopts.h index 7b82fd68..953c5d09 100644 --- a/test/unit/lwipopts.h +++ b/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 */