Skip to content

Commit 74dbfc3

Browse files
daanxdaanx
committed
improved security by encoding NULL values; double free mitigation on by default; more precise free list corruption detection
1 parent 1674d55 commit 74dbfc3

File tree

6 files changed

+49
-36
lines changed

6 files changed

+49
-36
lines changed

‎CMakeLists.txt‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@ set(CMAKE_CXX_STANDARD 17)
77
option(MI_OVERRIDE "Override the standard malloc interface" ON)
88
option(MI_INTERPOSE "Use interpose to override standard malloc on macOS" ON)
99
option(MI_DEBUG_FULL "Use full internal heap invariant checking in DEBUG mode" OFF)
10-
option(MI_SECURE "Use security mitigations (like guard pages, allocation randomization, and free-list corruption detection)" OFF)
11-
option(MI_SECURE_FULL "Use full security mitigations, may be more expensive (includes double-free mitigation)" OFF)
10+
option(MI_SECURE "Use full security mitigations (like guard pages, allocation randomization, double-free mitigation, and free-list corruption detection)" OFF)
1211
option(MI_USE_CXX "Use the C++ compiler to compile the library" OFF)
1312
option(MI_SEE_ASM "Generate assembly files" OFF)
1413
option(MI_LOCAL_DYNAMIC_TLS "Use slightly slower, dlopen-compatible TLS mechanism (Unix)" OFF)
@@ -70,15 +69,9 @@ if(MI_OVERRIDE MATCHES "ON")
7069
endif()
7170
endif()
7271

73-
if(MI_SECURE_FULL MATCHES "ON")
74-
message(STATUS "Set full secure build (may be more expensive) (MI_SECURE_FULL=ON)")
72+
if(MI_SECURE MATCHES "ON")
73+
message(STATUS "Set full secure build (MI_SECURE=ON)")
7574
list(APPEND mi_defines MI_SECURE=4)
76-
set(MI_SECURE "ON")
77-
else()
78-
if(MI_SECURE MATCHES "ON")
79-
message(STATUS "Set secure build (MI_SECURE=ON)")
80-
list(APPEND mi_defines MI_SECURE=3)
81-
endif()
8275
endif()
8376

8477
if(MI_SEE_ASM MATCHES "ON")
@@ -92,7 +85,7 @@ if(MI_CHECK_FULL MATCHES "ON")
9285
endif()
9386

9487
if(MI_DEBUG_FULL MATCHES "ON")
95-
message(STATUS "Set debug level to full invariant checking (MI_DEBUG_FULL=ON)")
88+
message(STATUS "Set debug level to full internal invariant checking (MI_DEBUG_FULL=ON)")
9689
list(APPEND mi_defines MI_DEBUG=3) # full invariant checking
9790
endif()
9891

‎include/mimalloc-internal.h‎

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -275,14 +275,20 @@ static inline mi_segment_t* _mi_page_segment(const mi_page_t* page) {
275275
return segment;
276276
}
277277

278-
// Get the page containing the pointer
279-
static inline mi_page_t* _mi_segment_page_of(const mi_segment_t* segment, const void* p) {
278+
// used internally
279+
static inline uintptr_t _mi_segment_page_idx_of(const mi_segment_t* segment, const void* p) {
280280
// if (segment->page_size > MI_SEGMENT_SIZE) return &segment->pages[0]; // huge pages
281281
ptrdiff_t diff = (uint8_t*)p - (uint8_t*)segment;
282282
mi_assert_internal(diff >= 0 && diff < MI_SEGMENT_SIZE);
283283
uintptr_t idx = (uintptr_t)diff >> segment->page_shift;
284284
mi_assert_internal(idx < segment->capacity);
285285
mi_assert_internal(segment->page_kind <= MI_PAGE_MEDIUM || idx == 0);
286+
return idx;
287+
}
288+
289+
// Get the page containing the pointer
290+
static inline mi_page_t* _mi_segment_page_of(const mi_segment_t* segment, const void* p) {
291+
uintptr_t idx = _mi_segment_page_idx_of(segment, p);
286292
return &((mi_segment_t*)segment)->pages[idx];
287293
}
288294

@@ -373,53 +379,67 @@ static inline void mi_page_set_has_aligned(mi_page_t* page, bool has_aligned) {
373379

374380
// -------------------------------------------------------------------
375381
// Encoding/Decoding the free list next pointers
382+
// Note: we pass a `null` value to be used as the `NULL` value for the
383+
// end of a free list. This is to prevent the cookie itself to ever
384+
// be present among user blocks (as `cookie^0==cookie`).
376385
// -------------------------------------------------------------------
377386

378387
static inline bool mi_is_in_same_segment(const void* p, const void* q) {
379388
return (_mi_ptr_segment(p) == _mi_ptr_segment(q));
380389
}
381390

382-
static inline mi_block_t* mi_block_nextx( uintptr_t cookie, const mi_block_t* block ) {
391+
static inline bool mi_is_in_same_page(const void* p, const void* q) {
392+
mi_segment_t* segmentp = _mi_ptr_segment(p);
393+
mi_segment_t* segmentq = _mi_ptr_segment(q);
394+
if (segmentp != segmentq) return false;
395+
uintptr_t idxp = _mi_segment_page_idx_of(segmentp, p);
396+
uintptr_t idxq = _mi_segment_page_idx_of(segmentq, q);
397+
return (idxp == idxq);
398+
}
399+
400+
static inline mi_block_t* mi_block_nextx( const void* null, const mi_block_t* block, uintptr_t cookie ) {
383401
#ifdef MI_ENCODE_FREELIST
384-
return (mi_block_t*)(block->next ^ cookie);
402+
mi_block_t* b = (mi_block_t*)(block->next ^ cookie);
403+
if (mi_unlikely((void*)b==null)) { b = NULL; }
404+
return b;
385405
#else
386-
UNUSED(cookie);
406+
UNUSED(cookie); UNUSED(null);
387407
return (mi_block_t*)block->next;
388408
#endif
389409
}
390410

391-
static inline void mi_block_set_nextx(uintptr_t cookie, mi_block_t* block, const mi_block_t* next) {
411+
static inline void mi_block_set_nextx(const void* null, mi_block_t* block, const mi_block_t* next, uintptr_t cookie) {
392412
#ifdef MI_ENCODE_FREELIST
413+
if (mi_unlikely(next==NULL)) { next = (mi_block_t*)null; }
393414
block->next = (mi_encoded_t)next ^ cookie;
394415
#else
395-
UNUSED(cookie);
416+
UNUSED(cookie); UNUSED(null);
396417
block->next = (mi_encoded_t)next;
397418
#endif
398419
}
399420

400421
static inline mi_block_t* mi_block_next(const mi_page_t* page, const mi_block_t* block) {
401422
#ifdef MI_ENCODE_FREELIST
402-
mi_block_t* next = mi_block_nextx(page->cookie,block);
423+
mi_block_t* next = mi_block_nextx(page,block,page->cookie);
403424
// check for free list corruption: is `next` at least in our segment range?
404-
// TODO: it is better to check if it is actually inside our page but that is more expensive
405-
// to calculate. Perhaps with a relative free list this becomes feasible?
406-
if (next!=NULL && !mi_is_in_same_segment(block, next)) {
425+
// TODO: check if `next` is `page->block_size` aligned?
426+
if (next!=NULL && !mi_is_in_same_page(block, next)) {
407427
_mi_fatal_error("corrupted free list entry of size %zub at %p: value 0x%zx\n", page->block_size, block, (uintptr_t)next);
408428
next = NULL;
409429
}
410430
return next;
411431
#else
412432
UNUSED(page);
413-
return mi_block_nextx(0, block);
433+
return mi_block_nextx(page,block,0);
414434
#endif
415435
}
416436

417437
static inline void mi_block_set_next(const mi_page_t* page, mi_block_t* block, const mi_block_t* next) {
418438
#ifdef MI_ENCODE_FREELIST
419-
mi_block_set_nextx(page->cookie,block,next);
439+
mi_block_set_nextx(page,block,next, page->cookie);
420440
#else
421441
UNUSED(page);
422-
mi_block_set_nextx(0, block, next);
442+
mi_block_set_nextx(page,block, next,0);
423443
#endif
424444
}
425445

‎include/mimalloc-types.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,16 @@ terms of the MIT license. A copy of the license can be found in the file
2626
// #define MI_SECURE 1 // guard page around metadata
2727
// #define MI_SECURE 2 // guard page around each mimalloc page
2828
// #define MI_SECURE 3 // encode free lists (detect corrupted free list (buffer overflow), and invalid pointer free)
29-
// #define MI_SECURE 4 // experimental, may be more expensive: checks for double free. (cmake -DMI_SECURE_FULL=ON)
29+
// #define MI_SECURE 4 // checks for double free. (may be more expensive)
3030

3131
#if !defined(MI_SECURE)
32-
#define MI_SECURE 0
32+
#define MI_SECURE 4
3333
#endif
3434

3535
// Define MI_DEBUG for debug mode
3636
// #define MI_DEBUG 1 // basic assertion checks and statistics, check double free, corrupted free list, and invalid pointer free.
3737
// #define MI_DEBUG 2 // + internal assertion checks
38-
// #define MI_DEBUG 3 // + extensive internal invariant checking (cmake -DMI_CHECK_FULL=ON)
38+
// #define MI_DEBUG 3 // + extensive internal invariant checking (cmake -DMI_DEBUG_FULL=ON)
3939
#if !defined(MI_DEBUG)
4040
#if !defined(NDEBUG) || defined(_DEBUG)
4141
#define MI_DEBUG 2

‎src/alloc.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ static mi_decl_noinline bool mi_check_is_double_freex(const mi_page_t* page, con
157157
}
158158

159159
static inline bool mi_check_is_double_free(const mi_page_t* page, const mi_block_t* block) {
160-
mi_block_t* n = mi_block_nextx(page->cookie, block); // pretend it is freed, and get the decoded first field
160+
mi_block_t* n = mi_block_nextx(page, block, page->cookie); // pretend it is freed, and get the decoded first field
161161
if (((uintptr_t)n & (MI_INTPTR_SIZE-1))==0 && // quick check: aligned pointer?
162162
(n==NULL || mi_is_in_same_segment(block, n))) // quick check: in same segment or NULL?
163163
{
@@ -242,7 +242,7 @@ static mi_decl_noinline void _mi_free_block_mt(mi_page_t* page, mi_block_t* bloc
242242
mi_block_t* dfree;
243243
do {
244244
dfree = (mi_block_t*)heap->thread_delayed_free;
245-
mi_block_set_nextx(heap->cookie,block,dfree);
245+
mi_block_set_nextx(heap,block,dfree, heap->cookie);
246246
} while (!mi_atomic_cas_ptr_weak(mi_atomic_cast(void*,&heap->thread_delayed_free), block, dfree));
247247
}
248248

‎src/page.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,15 +283,15 @@ void _mi_heap_delayed_free(mi_heap_t* heap) {
283283

284284
// and free them all
285285
while(block != NULL) {
286-
mi_block_t* next = mi_block_nextx(heap->cookie,block);
286+
mi_block_t* next = mi_block_nextx(heap,block, heap->cookie);
287287
// use internal free instead of regular one to keep stats etc correct
288288
if (!_mi_free_delayed_block(block)) {
289289
// we might already start delayed freeing while another thread has not yet
290290
// reset the delayed_freeing flag; in that case delay it further by reinserting.
291291
mi_block_t* dfree;
292292
do {
293293
dfree = (mi_block_t*)heap->thread_delayed_free;
294-
mi_block_set_nextx(heap->cookie, block, dfree);
294+
mi_block_set_nextx(heap, block, dfree, heap->cookie);
295295
} while (!mi_atomic_cas_ptr_weak(mi_atomic_cast(void*,&heap->thread_delayed_free), block, dfree));
296296

297297
}
@@ -356,7 +356,7 @@ void _mi_page_abandon(mi_page_t* page, mi_page_queue_t* pq) {
356356

357357
#if MI_DEBUG>1
358358
// check there are no references left..
359-
for (mi_block_t* block = (mi_block_t*)pheap->thread_delayed_free; block != NULL; block = mi_block_nextx(pheap->cookie, block)) {
359+
for (mi_block_t* block = (mi_block_t*)pheap->thread_delayed_free; block != NULL; block = mi_block_nextx(pheap, block, pheap->cookie)) {
360360
mi_assert_internal(_mi_ptr_page(block) != page);
361361
}
362362
#endif

‎test/main-override-static.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ int main() {
1515
mi_version();
1616

1717
// detect double frees and heap corruption
18-
//double_free1();
19-
//double_free2();
20-
//corrupt_free();
18+
// double_free1();
19+
// double_free2();
20+
// corrupt_free();
2121

2222
void* p1 = malloc(78);
2323
void* p2 = malloc(24);

0 commit comments

Comments
 (0)