fix: free(NULL) crashes before initialization with the 2-level page map - #29
Closed
LucFauvel wants to merge 1 commit into
Closed
fix: free(NULL) crashes before initialization with the 2-level page map#29LucFauvel wants to merge 1 commit into
free(NULL) crashes before initialization with the 2-level page map#29LucFauvel wants to merge 1 commit into
Conversation
… 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 microsoft#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 <noreply@anthropic.com>
3 tasks
Author
|
Can't request review so @Jarred-Sumner if you can take a look. Seems to be breaking for more and more people on glibc 2.44. Upstream has made this bug unreachable (but latent) so an upstream sync might be better. |
Author
|
Duplicate to changes in #30 Closing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes anthropics/claude-code#89370
Issue not reachable on upstream microsoft/mimalloc as _mi_aligned_ptr_page0 early NULL returns, while this fork is always via page map.
The static
mi_page_map_emptyused before_mi_page_map_initruns hassubmaps[0] == NULL. The default (non-secure, non-debug) build resolves a pointer through_mi_unchecked_ptr_page, which loadssubmaps[idx][sub_idx]without testing the sub-map for NULL, sofree(NULL)-- where both indices are 0 -- dereferences address 0 instead of yielding a NULL page._mi_checked_ptr_pagedoes 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 microsoft#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 unconditionalfree()of a buffer that is NULL on the common path, and the C++ runtime callsnewlocale()from a static initializer while buildingstd::locale::classic(). With MI_OVERRIDE thatfree(NULL)reaches mimalloc before any allocation has happened, so before the page map exists, and the process dies in a static initializer beforemain.Codegen is unchanged (
mi_freeis byte-identical); the fix costs 8 bytes of BSS.