mirrors/mimalloc
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

stronger secure mode when defining MI_SECURE=4: checks for double free, corrupted free list, and invalid pointer frees. Performance is impacted but not too much -- more perf testing is needed

Browse Source
This commit is contained in:
daan 2019-10-18 18:11:04 -07:00
parent fdfa6ed260
commit 2affdbbd2e
8 changed files with 121 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CMakeLists.txt
View File

@ -68,7 +68,7 @@ endif()
if(MI_SECURE MATCHES "ON")
message(STATUS "Set secure build (MI_SECURE=ON)")
list(APPEND mi_defines MI_SECURE=2)
list(APPEND mi_defines MI_SECURE=3)
endif()
if(MI_SEE_ASM MATCHES "ON")

4
ide/vs2019/mimalloc.vcxproj
View File

@ -111,12 +111,12 @@
</ItemDefinitionGroup>
<ItemDefinitionGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'">
<ClCompile>
<WarningLevel>Level3</WarningLevel>
<WarningLevel>Level2</WarningLevel>
<Optimization>Disabled</Optimization>
<SDLCheck>true</SDLCheck>
<ConformanceMode>true</ConformanceMode>
<AdditionalIncludeDirectories>../../include</AdditionalIncludeDirectories>
<PreprocessorDefinitions>MI_DEBUG=3;%(PreprocessorDefinitions);</PreprocessorDefinitions>
<PreprocessorDefinitions>MI_DEBUG=1;%(PreprocessorDefinitions);</PreprocessorDefinitions>
<CompileAs>CompileAsCpp</CompileAs>
<SupportJustMyCode>false</SupportJustMyCode>
<LanguageStandard>stdcpp17</LanguageStandard>

45
include/mimalloc-internal.h
View File

@ -20,6 +20,18 @@ terms of the MIT license. A copy of the license can be found in the file
#define mi_trace_message(...)
#endif
#if defined(_MSC_VER)
#define mi_decl_noinline __declspec(noinline)
#define mi_attr_noreturn
#elif defined(__GNUC__) || defined(__clang__)
#define mi_decl_noinline __attribute__((noinline))
#define mi_attr_noreturn __attribute__((noreturn))
#else
#define mi_decl_noinline
#define mi_attr_noreturn
#endif
// "options.c"
void _mi_fputs(mi_output_fun* out, const char* prefix, const char* message);
void _mi_fprintf(mi_output_fun* out, const char* fmt, ...);
@ -28,6 +40,7 @@ void _mi_warning_message(const char* fmt, ...);
void _mi_verbose_message(const char* fmt, ...);
void _mi_trace_message(const char* fmt, ...);
void _mi_options_init(void);
void _mi_fatal_error(const char* fmt, ...) mi_attr_noreturn;
// "init.c"
extern mi_stats_t _mi_stats_main;
@ -124,13 +137,6 @@ bool _mi_page_is_valid(mi_page_t* page);
#define __has_builtin(x) 0
#endif
#if defined(_MSC_VER)
#define mi_decl_noinline __declspec(noinline)
#elif defined(__GNUC__) || defined(__clang__)
#define mi_decl_noinline __attribute__((noinline))
#else
#define mi_decl_noinline
#endif
/* -----------------------------------------------------------
@ -365,8 +371,12 @@ static inline void mi_page_set_has_aligned(mi_page_t* page, bool has_aligned) {
// Encoding/Decoding the free list next pointers
// -------------------------------------------------------------------
static inline mi_block_t* mi_block_nextx( uintptr_t cookie, mi_block_t* block ) {
#if MI_SECURE
static inline bool mi_is_in_same_segment(const void* p, const void* q) {
return (_mi_ptr_segment(p) == _mi_ptr_segment(q));
}
static inline mi_block_t* mi_block_nextx( uintptr_t cookie, const mi_block_t* block ) {
#if MI_SECURE
return (mi_block_t*)(block->next ^ cookie);
#else
UNUSED(cookie);
@ -374,7 +384,7 @@ static inline mi_block_t* mi_block_nextx( uintptr_t cookie, mi_block_t* block )
#endif
}
static inline void mi_block_set_nextx(uintptr_t cookie, mi_block_t* block, mi_block_t* next) {
static inline void mi_block_set_nextx(uintptr_t cookie, mi_block_t* block, const mi_block_t* next) {
#if MI_SECURE
block->next = (mi_encoded_t)next ^ cookie;
#else
@ -383,16 +393,25 @@ static inline void mi_block_set_nextx(uintptr_t cookie, mi_block_t* block, mi_bl
#endif
}
static inline mi_block_t* mi_block_next(mi_page_t* page, mi_block_t* block) {
static inline mi_block_t* mi_block_next(const mi_page_t* page, const mi_block_t* block) {
#if MI_SECURE
return mi_block_nextx(page->cookie,block);
mi_block_t* next = mi_block_nextx(page->cookie,block);
#if MI_SECURE >= 4
// check if next is at least in our segment range
// TODO: it is better to check if it is actually inside our page but that is more expensive
// to calculate. Perhaps with a relative free list this becomes feasible?
if (next!=NULL && !mi_is_in_same_segment(block, next)) {
_mi_fatal_error("corrupted free list entry at %p: %zx\n", block, (uintptr_t)next);
}
#endif
return next;
#else
UNUSED(page);
return mi_block_nextx(0, block);
#endif
}
static inline void mi_block_set_next(mi_page_t* page, mi_block_t* block, mi_block_t* next) {
static inline void mi_block_set_next(const mi_page_t* page, mi_block_t* block, const mi_block_t* next) {
#if MI_SECURE
mi_block_set_nextx(page->cookie,block,next);
#else

7
include/mimalloc-types.h
View File

@ -22,8 +22,11 @@ terms of the MIT license. A copy of the license can be found in the file
// Define MI_STAT as 1 to maintain statistics; set it to 2 to have detailed statistics (but costs some performance).
// #define MI_STAT 1
// Define MI_SECURE as 1 to encode free lists
// #define MI_SECURE 1
// Define MI_SECURE to enable security mitigations
// #define MI_SECURE 1 // guard page around metadata
// #define MI_SECURE 2 // guard page around each mimalloc page
// #define MI_SECURE 3 // encode free lists
// #define MI_SECURE 4 // all security enabled (checks for double free, corrupted free list and invalid pointer free)
#if !defined(MI_SECURE)
#define MI_SECURE 0

49
src/alloc.c
View File

@ -124,10 +124,50 @@ mi_decl_allocator void* mi_zalloc(size_t size) mi_attr_noexcept {
}
// ------------------------------------------------------
// Check for double free in secure mode
// ------------------------------------------------------
#if MI_SECURE>=4
static bool mi_list_contains(const mi_page_t* page, const mi_block_t* list, const mi_block_t* elem) {
while (list != NULL) {
if (elem==list) return true;
list = mi_block_next(page, list);
}
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) {
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.
// 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))
{
_mi_fatal_error("double free detected of block %p with size %zu\n", block, page->block_size);
}
}
}
static inline void mi_free_check_block(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;
}
#endif
// ------------------------------------------------------
// Free
// ------------------------------------------------------
// multi-threaded free
static mi_decl_noinline void _mi_free_block_mt(mi_page_t* page, mi_block_t* block)
{
@ -251,14 +291,16 @@ void mi_free(void* p) mi_attr_noexcept
#if (MI_DEBUG>0)
if (mi_unlikely(!mi_is_in_heap_region(p))) {
_mi_warning_message("possibly trying to mi_free a pointer that does not point to a valid heap region: 0x%p\n"
_mi_warning_message("possibly trying to free a pointer that does not point to a valid heap region: 0x%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 0x%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("trying to mi_free a pointer that does not point to a valid heap space: %p\n", p);
_mi_error_message("trying to free a pointer that does not point to a valid heap space: %p\n", p);
return;
}
#endif
@ -278,6 +320,9 @@ 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);
#endif
mi_block_set_next(page, block, page->local_free);
page->local_free = block;
page->used--;

8
src/options.c
View File

@ -285,6 +285,14 @@ void _mi_assert_fail(const char* assertion, const char* fname, unsigned line, co
}
#endif
mi_attr_noreturn void _mi_fatal_error(const char* fmt, ...) {
va_list args;
va_start(args, fmt);
mi_vfprintf(NULL, "mimalloc: fatal: ", fmt, args);
va_end(args);
exit(99);
}
// --------------------------------------------------------
// Initialize options by checking the environment
// --------------------------------------------------------

20
test/main-override-static.c
View File

@ -2,12 +2,16 @@
#include <stdio.h>
#include <assert.h>
#include <string.h>
#include <stdint.h>
#include <mimalloc.h>
#include <mimalloc-override.h> // redefines malloc etc.
static void double_free();
int main() {
mi_version();
double_free();
void* p1 = malloc(78);
void* p2 = malloc(24);
free(p1);
@ -29,3 +33,19 @@ int main() {
mi_stats_print(NULL);
return 0;
}
static void double_free() {
void* p[256];
uintptr_t buf[256];
p[0] = mi_malloc(622616);
p[1] = mi_malloc(655362);
p[2] = mi_malloc(786432);
mi_free(p[2]);
// [VULN] Double free
mi_free(p[2]);
p[3] = mi_malloc(786456);
// [BUG] Found overlap
// 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]);
}

6
test/main-override.cpp
View File

@ -2,10 +2,13 @@
#include <stdio.h>
#include <assert.h>
#include <string.h>
#include <stdint.h>
#include <mimalloc.h>
#include <new>
static void double_free();
static void* p = malloc(8);
void free_p() {
@ -24,6 +27,7 @@ public:
int main() {
//mi_stats_reset(); // ignore earlier allocations
double_free();
atexit(free_p);
void* p1 = malloc(78);
void* p2 = mi_malloc_aligned(16,24);
@ -66,3 +70,5 @@ public:
};
static Static s = Static();