From 8769082d63e2b7caf392c62e25fda75f36604f90 Mon Sep 17 00:00:00 2001 From: daan Date: Mon, 20 Jul 2020 14:33:03 -0700 Subject: [PATCH] add pointer validity check in debug mode for mi_usable_size/mi_realloc/mi_expand. Issue #269 --- src/alloc.c | 77 ++++++++++++++++++++++--------------- test/main-override-static.c | 9 ++++- 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/src/alloc.c b/src/alloc.c index 607d15b6..57034522 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -387,34 +387,45 @@ static void mi_decl_noinline mi_free_generic(const mi_segment_t* segment, bool l _mi_free_block(page, local, block); } +// Get the segment data belonging to a pointer +// This is just a single `and` in assembly but does further checks in debug mode +// (and secure mode) if this was a valid pointer. +static inline mi_segment_t* mi_checked_ptr_segment(const void* p, const char* msg) +{ + UNUSED(msg); +#if (MI_DEBUG>0) + if (mi_unlikely(((uintptr_t)p & (MI_INTPTR_SIZE - 1)) != 0)) { + _mi_error_message(EINVAL, "%s: invalid (unaligned) pointer: %p\n", msg, p); + return NULL; + } +#endif + + mi_segment_t* const segment = _mi_ptr_segment(p); + if (mi_unlikely(segment == NULL)) return NULL; // checks also for (p==NULL) + +#if (MI_DEBUG>0) + if (mi_unlikely(!mi_is_in_heap_region(p))) { + _mi_warning_message("%s: pointer might not point to a valid heap region: %p\n" + "(this may still be a valid very large allocation (over 64MiB))\n", msg, p); + if (mi_likely(_mi_ptr_cookie(segment) == segment->cookie)) { + _mi_warning_message("(yes, the previous pointer %p was valid after all)\n", p); + } + } +#endif +#if (MI_DEBUG>0 || MI_SECURE>=4) + if (mi_unlikely(_mi_ptr_cookie(segment) != segment->cookie)) { + _mi_error_message(EINVAL, "%s: pointer does not point to a valid heap space: %p\n", p); + } +#endif + return segment; +} + + // Free a block void mi_free(void* p) mi_attr_noexcept { -#if (MI_DEBUG>0) - if (mi_unlikely(((uintptr_t)p & (MI_INTPTR_SIZE - 1)) != 0)) { - _mi_error_message(EINVAL, "trying to free an invalid (unaligned) pointer: %p\n", p); - return; - } -#endif - - const mi_segment_t* const segment = _mi_ptr_segment(p); - if (mi_unlikely(segment == NULL)) return; // checks for (p==NULL) - -#if (MI_DEBUG!=0) - if (mi_unlikely(!mi_is_in_heap_region(p))) { - _mi_warning_message("possibly trying to free a pointer that does not point to a valid heap region: %p\n" - "(this may still be a valid very large allocation (over 64MiB))\n", p); - if (mi_likely(_mi_ptr_cookie(segment) == segment->cookie)) { - _mi_warning_message("(yes, the previous pointer %p was valid after all)\n", p); - } - } -#endif -#if (MI_DEBUG!=0 || MI_SECURE>=4) - if (mi_unlikely(_mi_ptr_cookie(segment) != segment->cookie)) { - _mi_error_message(EINVAL, "trying to free a pointer that does not point to a valid heap space: %p\n", p); - return; - } -#endif + const mi_segment_t* const segment = mi_checked_ptr_segment(p,"mi_free"); + if (mi_unlikely(segment == NULL)) return; const uintptr_t tid = _mi_thread_id(); mi_page_t* const page = _mi_segment_page_of(segment, p); @@ -473,9 +484,9 @@ bool _mi_free_delayed_block(mi_block_t* block) { } // Bytes available in a block -size_t mi_usable_size(const void* p) mi_attr_noexcept { - if (p==NULL) return 0; - const mi_segment_t* const segment = _mi_ptr_segment(p); +static size_t _mi_usable_size(const void* p, const char* msg) mi_attr_noexcept { + const mi_segment_t* const segment = mi_checked_ptr_segment(p,msg); + if (segment==NULL) return 0; const mi_page_t* const page = _mi_segment_page_of(segment, p); const mi_block_t* block = (const mi_block_t*)p; if (mi_unlikely(mi_page_has_aligned(page))) { @@ -490,6 +501,10 @@ size_t mi_usable_size(const void* p) mi_attr_noexcept { } } +size_t mi_usable_size(const void* p) mi_attr_noexcept { + return _mi_usable_size(p, "mi_usable_size"); +} + // ------------------------------------------------------ // ensure explicit external inline definitions are emitted! @@ -513,7 +528,7 @@ void* _mi_externs[] = { void mi_free_size(void* p, size_t size) mi_attr_noexcept { UNUSED_RELEASE(size); - mi_assert(p == NULL || size <= mi_usable_size(p)); + mi_assert(p == NULL || size <= _mi_usable_size(p,"mi_free_size")); mi_free(p); } @@ -553,14 +568,14 @@ mi_decl_restrict void* mi_mallocn(size_t count, size_t size) mi_attr_noexcept { // Expand in place or fail void* mi_expand(void* p, size_t newsize) mi_attr_noexcept { if (p == NULL) return NULL; - size_t size = mi_usable_size(p); + size_t size = _mi_usable_size(p,"mi_expand"); if (newsize > size) return NULL; return p; // it fits } void* _mi_heap_realloc_zero(mi_heap_t* heap, void* p, size_t newsize, bool zero) { if (p == NULL) return _mi_heap_malloc_zero(heap,newsize,zero); - size_t size = mi_usable_size(p); + size_t size = _mi_usable_size(p,"mi_realloc"); if (newsize <= size && newsize >= (size / 2)) { return p; // reallocation still fits and not more than 50% waste } diff --git a/test/main-override-static.c b/test/main-override-static.c index 9243fd21..f738c517 100644 --- a/test/main-override-static.c +++ b/test/main-override-static.c @@ -11,6 +11,7 @@ static void double_free1(); static void double_free2(); static void corrupt_free(); static void block_overflow1(); +static void invalid_free(); int main() { mi_version(); @@ -19,7 +20,8 @@ int main() { // double_free1(); // double_free2(); // corrupt_free(); - block_overflow1(); + // block_overflow1(); + invalid_free(); void* p1 = malloc(78); void* p2 = malloc(24); @@ -43,6 +45,11 @@ int main() { return 0; } +static void invalid_free() { + free((void*)0xBADBEEF); + realloc((void*)0xBADBEEF,10); +} + static void block_overflow1() { uint8_t* p = (uint8_t*)mi_malloc(17); p[18] = 0;