From 8c04558af824f787eda135a215e5429f2cc48afe Mon Sep 17 00:00:00 2001 From: Daan Leijen Date: Thu, 9 Dec 2021 16:04:22 -0800 Subject: [PATCH] improve padding extra --- include/mimalloc-internal.h | 2 +- include/mimalloc-types.h | 34 ++++++++++++++++++---------- src/alloc.c | 44 +++++++++++++++++++++++++++---------- src/init.c | 2 +- test/main-override-static.c | 2 +- 5 files changed, 58 insertions(+), 26 deletions(-) diff --git a/include/mimalloc-internal.h b/include/mimalloc-internal.h index 497fc1ed..34a9be35 100644 --- a/include/mimalloc-internal.h +++ b/include/mimalloc-internal.h @@ -395,7 +395,7 @@ static inline uintptr_t _mi_ptr_cookie(const void* p) { ----------------------------------------------------------- */ static inline mi_page_t* _mi_heap_get_free_small_page(mi_heap_t* heap, size_t size) { - mi_assert_internal(size <= (MI_SMALL_SIZE_MAX)); + mi_assert_internal(size <= (MI_SMALL_SIZE_MAX + 2*sizeof(void*))); // +2 for the minimal padding (see MI_PAGES_DIRECT) const size_t idx = _mi_wsize_from_size(size); mi_assert_internal(idx < MI_PAGES_DIRECT); return heap->pages_free_direct[idx]; diff --git a/include/mimalloc-types.h b/include/mimalloc-types.h index 730d353a..f480ad7d 100644 --- a/include/mimalloc-types.h +++ b/include/mimalloc-types.h @@ -60,16 +60,20 @@ terms of the MIT license. A copy of the license can be found in the file #define MI_PADDING 1 #endif -#if !defined(MI_PADDING_EXTRA) // use extra padding bytes? -#define MI_PADDING_EXTRA (64) +#if !defined(MI_DEBUG_TRACE) // store stack trace at each allocation +#define MI_DEBUG_TRACE MI_DEBUG #endif -#if !defined(MI_DEBUG_TRACE) // store stack trace at each allocation -#define MI_DEBUG_TRACE 1 +#if !defined(MI_DEBUG_TRACE_LEN) +#define MI_DEBUG_TRACE_LEN (7) // store up to N frames if tracing is enabled #endif -#if !defined(MI_DEBUG_TRACE_LEN) // store stack trace at each allocation -#define MI_DEBUG_TRACE_LEN (6) // store up to N frames +#if !defined(MI_PADDING_EXTRA) // use extra padding bytes? (so a stack trace can be preserved or next block corruption prevented) +#if MI_DEBUG_TRACE > 0 +#define MI_PADDING_EXTRA (128) +#else +#define MI_PADDING_EXTRA (0) +#endif #endif @@ -366,18 +370,26 @@ typedef struct mi_random_cxt_s { // In debug mode there is a padding structure at the end of the blocks to check for buffer overflows #if (MI_PADDING) typedef struct mi_padding_s { - uint32_t canary; // encoded block value to check validity of the padding (in case of overflow) - uint32_t delta; // padding bytes before the block. (mi_usable_size(p) - delta == exact allocated bytes) + #if MI_PADDING_EXTRA > 0 + uint32_t canary_lo; + uint8_t extra[MI_PADDING_EXTRA]; + #endif + uint32_t canary; // encoded block value to check validity of the delat (in case of overflow) + uint32_t delta; // padding bytes before the block. (mi_usable_size(p) - delta == exact allocated bytes) #if (MI_DEBUG_TRACE > 0) void* strace[MI_DEBUG_TRACE_LEN]; // stack trace at allocation time #endif } mi_padding_t; -#define MI_PADDING_SIZE (sizeof(mi_padding_t) + MI_PADDING_EXTRA) +#define MI_PADDING_MINSIZE (8) // 2*sizeof(uint32_t) +#define MI_PADDING_SIZE (sizeof(mi_padding_t)) #else -#define MI_PADDING_SIZE 0 +#define MI_PADDING_MINSIZE (0) +#define MI_PADDING_SIZE (0) #endif -#define MI_PAGES_DIRECT (MI_SMALL_WSIZE_MAX + 1) +// add 2 more for minimal padding (MI_PADDING && !MI_DEBUG_TRACE && MI_PADDING_EXTRA==0) +// see `init.c` where all are initialized with an empty page and the check at `heap_malloc_small`. +#define MI_PAGES_DIRECT (MI_SMALL_WSIZE_MAX + 1 + 2) // A heap owns a set of pages. diff --git a/src/alloc.c b/src/alloc.c index bf52952b..4e7f6052 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -61,9 +61,13 @@ extern inline void* _mi_page_malloc(mi_heap_t* heap, mi_page_t* page, size_t siz mi_assert_internal(delta >= 0 && mi_page_usable_block_size(page) >= (size - MI_PADDING_SIZE + delta)); padding->canary = (uint32_t)(mi_ptr_encode(page,block,page->keys)); padding->delta = (uint32_t)(delta); + #if MI_PADDING_EXTRA > 0 + padding->canary_lo = padding->canary; + memset(padding->extra, 0, sizeof(padding->extra)); + #endif #if (MI_DEBUG_TRACE) _mi_stack_trace_capture(padding->strace, MI_DEBUG_TRACE_LEN, 2); - #endif + #endif uint8_t* fill = (uint8_t*)padding - delta; const size_t maxpad = (delta > MI_MAX_ALIGN_SIZE ? MI_MAX_ALIGN_SIZE : delta); // set at most N initial padding bytes for (size_t i = 0; i < maxpad; i++) { fill[i] = MI_DEBUG_PADDING; } @@ -77,13 +81,23 @@ extern inline mi_decl_restrict void* mi_heap_malloc_small(mi_heap_t* heap, size_ mi_assert(heap!=NULL); mi_assert(heap->thread_id == 0 || heap->thread_id == _mi_thread_id()); // heaps are thread local mi_assert(size <= MI_SMALL_SIZE_MAX); + void* p; #if (MI_PADDING) if (size == 0) { size = sizeof(void*); } #endif - mi_page_t* page = _mi_heap_get_free_small_page(heap, size + MI_PADDING_SIZE); - void* p = _mi_page_malloc(heap, page, size + MI_PADDING_SIZE); + #if (MI_PADDING_EXTRA > 0 || MI_DEBUG_TRACE > 0) + // with extra padding it is not guaranteed the size + MI_PADDING_SIZE <= MI_SMALL_SIZE_MAX, so we need to check + if (size + MI_PADDING_SIZE > MI_SMALL_SIZE_MAX) { + p = _mi_malloc_generic(heap, size + MI_PADDING_SIZE); + } + else + #endif + { + mi_page_t* page = _mi_heap_get_free_small_page(heap, size + MI_PADDING_SIZE); + p = _mi_page_malloc(heap, page, size + MI_PADDING_SIZE); + } mi_assert_internal(p==NULL || mi_usable_size(p) >= size); #if MI_STAT>1 if (p != NULL) { @@ -100,7 +114,7 @@ extern inline mi_decl_restrict void* mi_malloc_small(size_t size) mi_attr_noexce // The main allocation function extern inline mi_decl_restrict void* mi_heap_malloc(mi_heap_t* heap, size_t size) mi_attr_noexcept { - if (mi_likely(size + MI_PADDING_SIZE <= MI_SMALL_SIZE_MAX)) { + if (mi_likely(size + MI_PADDING_SIZE - MI_PADDING_MINSIZE <= MI_SMALL_SIZE_MAX)) { return mi_heap_malloc_small(heap, size); } else @@ -220,9 +234,9 @@ static inline bool mi_check_is_double_free(const mi_page_t* page, const mi_block // --------------------------------------------------------------------------- #if (MI_PADDING>0) && defined(MI_ENCODE_FREELIST) -static const mi_padding_t* mi_page_decode_padding(const mi_page_t* page, const mi_block_t* block, size_t* delta, size_t* bsize) { +static mi_padding_t* mi_page_decode_padding(const mi_page_t* page, const mi_block_t* block, size_t* delta, size_t* bsize) { *bsize = mi_page_usable_block_size(page); - const mi_padding_t* const padding = (mi_padding_t*)((uint8_t*)block + *bsize); + mi_padding_t* const padding = (mi_padding_t*)((uint8_t*)block + *bsize); *delta = padding->delta; if ((uint32_t)mi_ptr_encode(page, block, page->keys) == padding->canary && *delta <= *bsize) { return padding; @@ -259,9 +273,9 @@ static size_t mi_page_usable_size_of(const mi_page_t* page, const mi_block_t* bl static bool mi_verify_padding(const mi_page_t* page, const mi_block_t* block, size_t* size, size_t* wrong) { size_t bsize; size_t delta; - bool ok = (mi_page_decode_padding(page, block, &delta, &bsize) != NULL); + const mi_padding_t* padding = mi_page_decode_padding(page, block, &delta, &bsize); *size = *wrong = bsize; - if (!ok) return false; + if (padding == NULL) return false; mi_assert_internal(bsize >= delta); *size = bsize - delta; uint8_t* fill = (uint8_t*)block + bsize - delta; @@ -272,6 +286,12 @@ static bool mi_verify_padding(const mi_page_t* page, const mi_block_t* block, si return false; } } + #if MI_PADDING_EXTRA > 0 + if (padding->canary_lo != padding->canary) { + *wrong = bsize; + return false; + } + #endif return true; } @@ -291,14 +311,14 @@ static void mi_check_padding(const mi_page_t* page, const mi_block_t* block) { static void mi_padding_shrink(const mi_page_t* page, const mi_block_t* block, const size_t min_size) { size_t bsize; size_t delta; - bool ok = mi_page_decode_padding(page, block, &delta, &bsize); - mi_assert_internal(ok); - if (!ok || (bsize - delta) >= min_size) return; // usually already enough space + mi_padding_t* padding = mi_page_decode_padding(page, block, &delta, &bsize); + mi_assert_internal(padding!=NULL); + if (padding == NULL) return; + if ((bsize - delta) >= min_size) return; // usually already enough space mi_assert_internal(bsize >= min_size); if (bsize < min_size) return; // should never happen size_t new_delta = (bsize - min_size); mi_assert_internal(new_delta < bsize); - mi_padding_t* padding = (mi_padding_t*)((uint8_t*)block + bsize); padding->delta = (uint32_t)new_delta; } #else diff --git a/src/init.c b/src/init.c index ef658c24..3d4b3ce5 100644 --- a/src/init.c +++ b/src/init.c @@ -31,7 +31,7 @@ const mi_page_t _mi_page_empty = { }; #define MI_PAGE_EMPTY() ((mi_page_t*)&_mi_page_empty) -#define MI_SMALL_PAGES_EMPTY { MI_INIT128(MI_PAGE_EMPTY), MI_PAGE_EMPTY() } +#define MI_SMALL_PAGES_EMPTY { MI_INIT128(MI_PAGE_EMPTY), MI_PAGE_EMPTY(), MI_PAGE_EMPTY(), MI_PAGE_EMPTY() } // Empty page queues for every bin diff --git a/test/main-override-static.c b/test/main-override-static.c index 78b4abbd..12c785ad 100644 --- a/test/main-override-static.c +++ b/test/main-override-static.c @@ -117,7 +117,7 @@ static void double_free2() { // Try to corrupt the heap through buffer overflow #define N 256 #define SZ 64 -#define OVF_SZ 100 +#define OVF_SZ 32 static void corrupt_free() { void* p[N];