diff --git a/include/mimalloc-internal.h b/include/mimalloc-internal.h index 7bffb6ac..cf0252c6 100644 --- a/include/mimalloc-internal.h +++ b/include/mimalloc-internal.h @@ -46,7 +46,6 @@ void _mi_fatal_error(const char* fmt, ...) mi_attr_noreturn; extern mi_stats_t _mi_stats_main; extern const mi_page_t _mi_page_empty; bool _mi_is_main_thread(void); -uintptr_t _mi_ptr_cookie(const void* p); uintptr_t _mi_random_shuffle(uintptr_t x); uintptr_t _mi_random_init(uintptr_t seed /* can be zero */); bool _mi_preloading(); // true while the C runtime is not ready @@ -244,6 +243,10 @@ static inline bool mi_heap_is_initialized(mi_heap_t* heap) { return (heap != &_mi_heap_empty); } +static inline uintptr_t _mi_ptr_cookie(const void* p) { + return ((uintptr_t)p ^ _mi_heap_main.cookie); +} + /* ----------------------------------------------------------- Pages ----------------------------------------------------------- */ diff --git a/src/alloc.c b/src/alloc.c index f5208a0a..916b1f32 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -137,28 +137,32 @@ static bool mi_list_contains(const mi_page_t* page, const mi_block_t* list, cons return false; } -static mi_decl_noinline void mi_free_check_blockx(const mi_page_t* page, const mi_block_t* block, const mi_block_t* n) { +static mi_decl_noinline bool mi_check_double_freex(const mi_page_t* page, const mi_block_t* block, const mi_block_t* n) { size_t psize; uint8_t* pstart = _mi_page_start(_mi_page_segment(page), page, &psize); - if ((uint8_t*)n >= pstart && (uint8_t*)n < (pstart + psize)) { - // Suspicious: the decoded value is in the same page. + if (n == NULL || ((uint8_t*)n >= pstart && (uint8_t*)n < (pstart + psize))) { + // Suspicious: the decoded value is in the same page (or NULL). // Walk the free lists to see if it is already freed - if (mi_list_contains(page, page->free, n) || - mi_list_contains(page, page->local_free, n) || - mi_list_contains(page, (const mi_block_t*)mi_atomic_read_ptr_relaxed(mi_atomic_cast(void*,&page->thread_free)), n)) + if (mi_list_contains(page, page->free, block) || + mi_list_contains(page, page->local_free, block) || + mi_list_contains(page, (const mi_block_t*)mi_atomic_read_ptr_relaxed(mi_atomic_cast(void*,&page->thread_free)), block)) { _mi_fatal_error("double free detected of block %p with size %zu\n", block, page->block_size); + return true; } } + return false; } -static inline void mi_free_check_block(const mi_page_t* page, const mi_block_t* block) { +static inline bool mi_check_double_free(const mi_page_t* page, const mi_block_t* block) { mi_block_t* n = (mi_block_t*)(block->next ^ page->cookie); - if (n!=NULL && mi_is_in_same_segment(block, n)) { // quick check - // Suspicous: decoded value in block is in the same segment, maybe a double free? - mi_free_check_blockx(page, block, n); - } - return; + if (((uintptr_t)n & (MI_INTPTR_SIZE-1))==0 && // quick check + (n==NULL || mi_is_in_same_segment(block, n))) + { + // Suspicous: decoded value in block is in the same segment (or NULL) -- maybe a double free? + return mi_check_double_freex(page, block, n); + } + return false; } #endif @@ -320,8 +324,8 @@ void mi_free(void* p) mi_attr_noexcept if (mi_likely(tid == segment->thread_id && page->flags.full_aligned == 0)) { // the thread id matches and it is not a full page, nor has aligned blocks // local, and not full or aligned mi_block_t* block = (mi_block_t*)p; - #if MI_SECURE>=4 - mi_free_check_block(page,block); + #if MI_SECURE>=4 + if (mi_check_double_free(page,block)) return; #endif mi_block_set_next(page, block, page->local_free); page->local_free = block; diff --git a/src/init.c b/src/init.c index 75836aca..6514ce53 100644 --- a/src/init.c +++ b/src/init.c @@ -184,10 +184,6 @@ uintptr_t _mi_random_init(uintptr_t seed /* can be zero */) { return x; } -uintptr_t _mi_ptr_cookie(const void* p) { - return ((uintptr_t)p ^ _mi_heap_main.cookie); -} - /* ----------------------------------------------------------- Initialization and freeing of the thread local heaps ----------------------------------------------------------- */ diff --git a/test/main-override-static.c b/test/main-override-static.c index d8369389..ed5048e0 100644 --- a/test/main-override-static.c +++ b/test/main-override-static.c @@ -7,11 +7,13 @@ #include #include // redefines malloc etc. -static void double_free(); +static void double_free1(); +static void double_free2(); int main() { mi_version(); - double_free(); + //double_free1(); + //double_free2(); void* p1 = malloc(78); void* p2 = malloc(24); free(p1); @@ -34,7 +36,7 @@ int main() { return 0; } -static void double_free() { +static void double_free1() { void* p[256]; uintptr_t buf[256]; @@ -49,3 +51,21 @@ static void double_free() { // p[3]=0x429b2ea2000 (size=917504), p[1]=0x429b2e42000 (size=786432) fprintf(stderr, "p3: %p-%p, p1: %p-%p, p2: %p\n", p[3], (uint8_t*)(p[3]) + 786456, p[1], (uint8_t*)(p[1]) + 655362, p[2]); } + +static void double_free2() { + void* p[256]; + uintptr_t buf[256]; + // [INFO] Command buffer: 0x327b2000 + // [INFO] Input size: 182 + p[0] = malloc(712352); + p[1] = malloc(786432); + free(p[0]); + // [VULN] Double free + free(p[0]); + p[2] = malloc(786440); + p[3] = malloc(917504); + p[4] = malloc(786440); + // [BUG] Found overlap + // p[4]=0x433f1402000 (size=917504), p[1]=0x433f14c2000 (size=786432) + fprintf(stderr, "p1: %p-%p, p2: %p-%p\n", p[4], (uint8_t*)(p[4]) + 917504, p[1], (uint8_t*)(p[1]) + 786432); +}