From f38816d4ed3fb4af588c6addf8ec11ec47b79c55 Mon Sep 17 00:00:00 2001 From: Philip Brown <122590765+asdf-bro@users.noreply.github.com> Date: Sun, 6 Oct 2024 15:42:46 -0500 Subject: [PATCH 1/5] Musl needs __libc* functions too --- src/alloc-override.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/alloc-override.c b/src/alloc-override.c index 12837cdd..82a98b80 100644 --- a/src/alloc-override.c +++ b/src/alloc-override.c @@ -289,8 +289,8 @@ mi_decl_weak int reallocarr(void* p, size_t count, size_t size) { return mi_r void __libc_free(void* p) MI_FORWARD0(mi_free, p) void* __libc_memalign(size_t alignment, size_t size) { return mi_memalign(alignment, size); } -#elif defined(__GLIBC__) && defined(__linux__) - // forward __libc interface (needed for glibc-based Linux distributions) +#elif defined(__linux__) + // forward __libc interface (needed for glibc-based and musl-based Linux distributions) void* __libc_malloc(size_t size) MI_FORWARD1(mi_malloc,size) void* __libc_calloc(size_t count, size_t size) MI_FORWARD2(mi_calloc,count,size) void* __libc_realloc(void* p, size_t size) MI_FORWARD2(mi_realloc,p,size) From bf251b27b187d36bec964e9d6e830dcf0e5b75a4 Mon Sep 17 00:00:00 2001 From: ArtSin Date: Tue, 15 Oct 2024 13:39:28 +0400 Subject: [PATCH 2/5] Fix int and long handling and the use of (u)intptr_t in _mi_vsnprintf 'va_arg' with type 'long' was used for both 'int' and 'long' arguments, but x86_64 Linux has 64-bit 'long', so passing an 'int' argument to '_mi_vsnprintf` results in undefined behaviour. 'intptr_t' was used as the largest integer type, but on 32-bit systems it is 32-bit wide, while 'long long' is 64-bit. 'intmax_t' can be used instead. --- src/libc.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/libc.c b/src/libc.c index dd6b4007..ce541f1b 100644 --- a/src/libc.c +++ b/src/libc.c @@ -130,7 +130,7 @@ static void mi_out_alignright(char fill, char* start, size_t len, size_t extra, } -static void mi_out_num(uintptr_t x, size_t base, char prefix, char** out, char* end) +static void mi_out_num(uintmax_t x, size_t base, char prefix, char** out, char* end) { if (x == 0 || base == 0 || base > 16) { if (prefix != 0) { mi_outc(prefix, out, end); } @@ -206,12 +206,13 @@ void _mi_vsnprintf(char* buf, size_t bufsize, const char* fmt, va_list args) { } else if (c == 'p' || c == 'x' || c == 'u') { // unsigned - uintptr_t x = 0; + uintmax_t x = 0; if (c == 'x' || c == 'u') { if (numtype == 'z') x = va_arg(args, size_t); else if (numtype == 't') x = va_arg(args, uintptr_t); // unsigned ptrdiff_t - else if (numtype == 'L') x = (uintptr_t)va_arg(args, unsigned long long); - else x = va_arg(args, unsigned long); + else if (numtype == 'L') x = va_arg(args, unsigned long long); + else if (numtype == 'l') x = va_arg(args, unsigned long); + else x = va_arg(args, unsigned int); } else if (c == 'p') { x = va_arg(args, uintptr_t); @@ -228,20 +229,21 @@ void _mi_vsnprintf(char* buf, size_t bufsize, const char* fmt, va_list args) { } else if (c == 'i' || c == 'd') { // signed - intptr_t x = 0; + intmax_t x = 0; if (numtype == 'z') x = va_arg(args, intptr_t ); else if (numtype == 't') x = va_arg(args, ptrdiff_t); - else if (numtype == 'L') x = (intptr_t)va_arg(args, long long); - else x = va_arg(args, long); + else if (numtype == 'L') x = va_arg(args, long long); + else if (numtype == 'l') x = va_arg(args, long); + else x = va_arg(args, int); char pre = 0; if (x < 0) { pre = '-'; - if (x > INTPTR_MIN) { x = -x; } + if (x > INTMAX_MIN) { x = -x; } } else if (numplus != 0) { pre = numplus; } - mi_out_num((uintptr_t)x, 10, pre, &out, end); + mi_out_num((uintmax_t)x, 10, pre, &out, end); } else if (c >= ' ' && c <= '~') { // unknown format From 17802e6528a5629f62bd1219227544199ba34b3f Mon Sep 17 00:00:00 2001 From: Jim-Wang Date: Mon, 28 Oct 2024 02:06:58 +0000 Subject: [PATCH 3/5] fix build error on linux --- src/arena-abandon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arena-abandon.c b/src/arena-abandon.c index 9c3356fc..48e37794 100644 --- a/src/arena-abandon.c +++ b/src/arena-abandon.c @@ -148,7 +148,7 @@ static void mi_arena_segment_os_mark_abandoned(mi_segment_t* segment) { void _mi_arena_segment_mark_abandoned(mi_segment_t* segment) { mi_assert_internal(segment->used == segment->abandoned); - mi_atomic_store_release(&segment->thread_id, 0); // mark as abandoned for multi-thread free's + mi_atomic_store_release(&segment->thread_id, (uintptr_t)0); // mark as abandoned for multi-thread free's if mi_unlikely(segment->memid.memkind != MI_MEM_ARENA) { mi_arena_segment_os_mark_abandoned(segment); return; From 5f3593333107066434e77177e0e79d56aba5189d Mon Sep 17 00:00:00 2001 From: Daan Date: Sun, 27 Oct 2024 21:39:07 -0700 Subject: [PATCH 4/5] add 0 byte to canary to prevent spurious read overflow to read the canary (issue #951, pr #953) --- include/mimalloc/internal.h | 10 ++++++++++ src/alloc.c | 2 +- src/free.c | 2 +- test/main-override-static.c | 13 ++++++++++++- 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/include/mimalloc/internal.h b/include/mimalloc/internal.h index 63a1e6f0..82e8c766 100644 --- a/include/mimalloc/internal.h +++ b/include/mimalloc/internal.h @@ -667,6 +667,16 @@ static inline mi_encoded_t mi_ptr_encode(const void* null, const void* p, const return mi_rotl(x ^ keys[1], keys[0]) + keys[0]; } +static inline uint32_t mi_ptr_encode_canary(const void* null, const void* p, const uintptr_t* keys) { + const uint32_t x = (uint32_t)(mi_ptr_encode(null,p,keys)); + // make the lowest byte 0 to prevent spurious read overflows which could be a security issue (issue #951) + #ifdef MI_BIG_ENDIAN + return (x & 0x00FFFFFF); + #else + return (x & 0xFFFFFF00); + #endif +} + static inline mi_block_t* mi_block_nextx( const void* null, const mi_block_t* block, const uintptr_t* keys ) { mi_track_mem_defined(block,sizeof(mi_block_t)); mi_block_t* next; diff --git a/src/alloc.c b/src/alloc.c index ca60c11a..119dfe75 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -99,7 +99,7 @@ extern inline void* _mi_page_malloc_zero(mi_heap_t* heap, mi_page_t* page, size_ mi_assert_internal(delta >= 0 && mi_page_usable_block_size(page) >= (size - MI_PADDING_SIZE + delta)); #endif mi_track_mem_defined(padding,sizeof(mi_padding_t)); // note: re-enable since mi_page_usable_block_size may set noaccess - padding->canary = (uint32_t)(mi_ptr_encode(page,block,page->keys)); + padding->canary = mi_ptr_encode_canary(page,block,page->keys); padding->delta = (uint32_t)(delta); #if MI_PADDING_CHECK if (!mi_page_is_huge(page)) { diff --git a/src/free.c b/src/free.c index a85baa55..046a34e2 100644 --- a/src/free.c +++ b/src/free.c @@ -414,7 +414,7 @@ static bool mi_page_decode_padding(const mi_page_t* page, const mi_block_t* bloc uintptr_t keys[2]; keys[0] = page->keys[0]; keys[1] = page->keys[1]; - bool ok = ((uint32_t)mi_ptr_encode(page,block,keys) == canary && *delta <= *bsize); + bool ok = (mi_ptr_encode_canary(page,block,keys) == canary && *delta <= *bsize); mi_track_mem_noaccess(padding,sizeof(mi_padding_t)); return ok; } diff --git a/test/main-override-static.c b/test/main-override-static.c index 1ac9bf87..4ad76d6a 100644 --- a/test/main-override-static.c +++ b/test/main-override-static.c @@ -19,6 +19,7 @@ static void test_reserved(void); static void negative_stat(void); static void alloc_huge(void); static void test_heap_walk(void); +static void test_canary_leak(void); // static void test_large_pages(void); @@ -31,7 +32,8 @@ int main() { // double_free2(); // corrupt_free(); // block_overflow1(); - block_overflow2(); + // block_overflow2(); + test_canary_leak(); // test_aslr(); // invalid_free(); // test_reserved(); @@ -226,6 +228,15 @@ static void test_heap_walk(void) { mi_heap_visit_blocks(heap, true, &test_visit, NULL); } +static void test_canary_leak(void) { + char* p = mi_mallocn_tp(char,23); + for(int i = 0; i < 23; i++) { + p[i] = '0'+i; + } + puts(p); + free(p); +} + // Experiment with huge OS pages #if 0 From b3828bba9e983e4a61cc82b1acc9965f7bb15add Mon Sep 17 00:00:00 2001 From: Daan Date: Sun, 27 Oct 2024 21:58:20 -0700 Subject: [PATCH 5/5] disable aligned hinting or SV39 mmu's, issue #939, and pr #949 --- CMakeLists.txt | 11 +++++++++++ src/os.c | 7 ++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e0aa59b0..4729e5b5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -319,6 +319,17 @@ if(MI_WIN_USE_FLS) list(APPEND mi_defines MI_WIN_USE_FLS=1) endif() + + # Check /proc/cpuinfo for an SV39 MMU and define a constant if one is + # found. We will want to skip the aligned hinting in that case. Issue #939, #949 + if (EXISTS /proc/cpuinfo) + file(STRINGS /proc/cpuinfo mi_sv39_mmu REGEX "^mmu[ \t]+:[ \t]+sv39$") + if (mi_sv39_mmu) + MESSAGE( STATUS "Disable aligned hints (SV39 MMU detected)" ) + list(APPEND mi_defines MI_NO_ALIGNED_HINT=1) + endif() + endif() + # On Haiku use `-DCMAKE_INSTALL_PREFIX` instead, issue #788 # if(CMAKE_SYSTEM_NAME MATCHES "Haiku") # SET(CMAKE_INSTALL_LIBDIR ~/config/non-packaged/lib) diff --git a/src/os.c b/src/os.c index 4babd8da..4b9d6125 100644 --- a/src/os.c +++ b/src/os.c @@ -92,8 +92,9 @@ static void* mi_align_down_ptr(void* p, size_t alignment) { -------------------------------------------------------------- */ // On 64-bit systems, we can do efficient aligned allocation by using -// the 2TiB to 30TiB area to allocate those. -#if (MI_INTPTR_SIZE >= 8) +// the 2TiB to 30TiB area to allocate those. We assume we have +// at least 48 bits of virtual address space on 64-bit systems (but see issue #939) +#if (MI_INTPTR_SIZE >= 8) && !defined(MI_NO_ALIGNED_HINT) static mi_decl_cache_align _Atomic(uintptr_t)aligned_base; // Return a MI_SEGMENT_SIZE aligned address that is probably available. @@ -239,7 +240,7 @@ static void* mi_os_prim_alloc_aligned(size_t size, size_t alignment, bool commit if (!(alignment >= _mi_os_page_size() && ((alignment & (alignment - 1)) == 0))) return NULL; size = _mi_align_up(size, _mi_os_page_size()); - // try first with a hint (this will be aligned directly on Win 10+ or BSD) + // try first with a requested alignment hint (this will usually be aligned directly on Win 10+ or BSD) void* p = mi_os_prim_alloc(size, alignment, commit, allow_large, is_large, is_zero, stats); if (p == NULL) return NULL;