From 085053278207d3d4bf78452928ca584b0be66786 Mon Sep 17 00:00:00 2001 From: Luc Fauvel Date: Mon, 24 Aug 2026 21:56:00 -0400 Subject: [PATCH] fix: `free(NULL)` crashes before initialization with the 2-level page map The static `mi_page_map_empty` used before `_mi_page_map_init` runs has `submaps[0] == NULL`. The default (non-secure, non-debug) build resolves a pointer through `_mi_unchecked_ptr_page`, which loads `submaps[idx][sub_idx]` without testing the sub-map for NULL, so `free(NULL)` -- where both indices are 0 -- dereferences address 0 instead of yielding a NULL page. `_mi_checked_ptr_page` does test for it, which is why only the default release build is affected; MI_SECURE / MI_DEBUG builds are fine. The flat page map got this right in #1341 by pointing its empty map at a real one-element array; this does the same for the 2-level map. Reachable in practice: glibc >= 2.44 ends `newlocale()` with an unconditional `free()` of a buffer that is NULL on the common path, and the C++ runtime calls `newlocale()` from a static initializer while building `std::locale::classic()`. With MI_OVERRIDE that `free(NULL)` reaches mimalloc before any allocation has happened, so before the page map exists, and the process dies in a static initializer before `main`. Codegen is unchanged (`mi_free` is byte-identical); the fix costs 8 bytes of BSS. Co-Authored-By: Claude Opus 5 --- CMakeLists.txt | 2 +- src/page-map.c | 9 ++++++- test/test-free-null-preinit.c | 44 +++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 test/test-free-null-preinit.c diff --git a/CMakeLists.txt b/CMakeLists.txt index a1cf41348..8ff15718f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -877,7 +877,7 @@ if (MI_BUILD_TESTS) enable_testing() # static link tests - set(mi_static_tests api api-fill stress-heaps stress-subprocs stress heap-mt heap-teardown heap-delete-race heap-churn heap-aba fork-user-heap snapshot prof prof-adversarial purge-zero park-handoff) + set(mi_static_tests api api-fill stress-heaps stress-subprocs stress heap-mt heap-teardown heap-delete-race heap-churn heap-aba fork-user-heap snapshot prof prof-adversarial purge-zero park-handoff free-null-preinit) if(NOT (MI_DEBUG_TSAN OR MI_TRACK_ASAN OR MI_DEBUG_UBSAN)) list(APPEND mi_static_tests thp-optout) # counts madvise calls by interposing it, which a sanitizer runtime does first endif() diff --git a/src/page-map.c b/src/page-map.c index b4ee0189a..e977eeb12 100644 --- a/src/page-map.c +++ b/src/page-map.c @@ -215,12 +215,19 @@ mi_decl_nodiscard mi_decl_export bool mi_is_in_heap_region(const void* p) mi_att #define MI_PAGE_MAP_SUB_SIZE (MI_PAGE_MAP_SUB_COUNT * sizeof(mi_page_t*)) // Use an initial empty page map so `free(NULL)` works even if mimalloc is not yet initialized (issue #1341) +// The sub-map for the NULL address must itself be a valid (zero initialized) array rather than +// NULL: the default (non-secure, non-debug) build resolves pointers via `_mi_unchecked_ptr_page`, +// which loads `submaps[idx][sub_idx]` without checking the sub-map for NULL. A single entry is +// enough since `_mi_page_map_index(NULL)` is 0 at both levels, and no other pointer can belong +// to mimalloc before it is initialized (any `malloc` initializes the page map first). +static mi_page_t* mi_page_map_empty_sub[1] = { NULL }; // so that `_mi_ptr_page(NULL) == NULL` + static mi_page_map_t mi_page_map_empty = { MI_ATOMIC_VAR_INIT(1), sizeof(mi_page_map_t), MI_MEMID_STATIC, MI_LOCK_INITIALIZER, - { MI_ATOMIC_VAR_INIT(NULL) } + { MI_ATOMIC_VAR_INIT(mi_page_map_empty_sub) } }; mi_decl_hidden mi_decl_cache_align _Atomic(mi_page_map_t*) __mi_page_map = MI_ATOMIC_VAR_INIT(&mi_page_map_empty); diff --git a/test/test-free-null-preinit.c b/test/test-free-null-preinit.c new file mode 100644 index 000000000..2581f8f3f --- /dev/null +++ b/test/test-free-null-preinit.c @@ -0,0 +1,44 @@ +// `free(NULL)` must be a no-op even before mimalloc is initialized (issue #1341). +// +// This is not hypothetical: glibc >= 2.44 calls `free(NULL)` unconditionally at the end of +// `newlocale()`, and the C++ runtime calls `newlocale()` from a static initializer while +// constructing `std::locale::classic()`. With a global malloc override (MI_OVERRIDE) that +// `free` lands in mimalloc before any allocation has happened, so before the page map exists. +// +// The 2-level page map resolves a pointer as `submaps[idx][sub_idx]`; for NULL both indices +// are 0. The default (non-secure, non-debug) build uses `_mi_unchecked_ptr_page`, which does +// not test the sub-map for NULL -- so the static "empty" page map must carry a real (zeroed) +// sub-map at entry 0 rather than a NULL one, or this dereferences address 0. +#include +#include +#include + +static int early_ran = 0; + +#if defined(__GNUC__) || defined(__clang__) +// priority 101 runs before mimalloc's own (default priority) constructor +__attribute__((constructor(101))) +static void free_null_before_init(void) { + mi_free(NULL); // goes to mimalloc whether or not the malloc override is enabled + free(NULL); // and via the override, when it is + early_ran = 1; +} +#endif + +int main(void) { + int failures = 0; + + // and once more now that mimalloc is certainly initialized + mi_free(NULL); + free(NULL); + +#if defined(__GNUC__) || defined(__clang__) + if (!early_ran) { + printf(" FAIL: the early constructor did not run\n"); + failures++; + } +#endif + + printf(failures == 0 ? "ok\n" : "failed\n"); + return (failures == 0 ? 0 : 1); +}