Sanity check huge block sizes before using them as munmap() lengths - #23378
Open
jvoisin wants to merge 1 commit into
Open
Sanity check huge block sizes before using them as munmap() lengths#23378jvoisin wants to merge 1 commit into
jvoisin wants to merge 1 commit into
Conversation
zend_mm_huge_list nodes are allocated with zend_mm_alloc_heap(), from the very heap they describe, so a heap overflow can reach them. Their size field is then handed to munmap() in three places: - zend_mm_free_huge(), directly via zend_mm_chunk_free(), - the huge block loop in zend_mm_shutdown(), likewise, - zend_mm_realloc_huge(), which takes it as old_size and passes it to zend_mm_chunk_truncate(), which unmaps the tail with munmap(addr + new_size, old_size - new_size). The ptr is constrained a bit, as it has to match the pointer being freed and is checked for chunk alignment, but size is used as-is. Corrupting it turns a free of a legitimate huge block into an unmap of an arbitrary amount of adjacent address space, which a later mmap() can then occupy. This commit bounds it before use: A live huge block has to satisfy three cheap invariants: its size is not zero, it is a multiple of REAL_PAGE_SIZE (since it was produced by ZEND_MM_ALIGNED_SIZE_EX(size, REAL_PAGE_SIZE)), and it is still accounted for in heap->real_size, which is only decremented after the block has been freed. This narrows the primitive rather than removing it, as doing so would be more invasive. This commit was validated under gdb by tampering with size on a live 4MB block and freeing it: 0, 0x400001 (unaligned) and 0x800000 (exceeding a 6MB real_size) all abort with "zend_mm_heap corrupted", where all three were previously passed to munmap().
Contributor
Author
|
Part of #14083 |
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.
zend_mm_huge_list nodes are allocated with zend_mm_alloc_heap(), from the very heap they describe, so a heap overflow can reach them. Their size field is then handed to munmap() in three places:
The ptr is constrained a bit, as it has to match the pointer being freed and is checked for chunk alignment, but size is used as-is. Corrupting it turns a free of a legitimate huge block into an unmap of an arbitrary amount of adjacent address space, which a later mmap() can then occupy.
This commit bounds it before use: A live huge block has to satisfy three cheap invariants: its size is not zero, it is a multiple of REAL_PAGE_SIZE (since it was produced by ZEND_MM_ALIGNED_SIZE_EX(size, REAL_PAGE_SIZE)), and it is still accounted for in heap->real_size, which is only decremented after the block has been freed. This narrows the primitive rather than removing it, as doing so would be more invasive.
This commit was validated under gdb by tampering with size on a live 4MB block and freeing it: 0, 0x400001 (unaligned) and 0x800000 (exceeding a 6MB real_size) all abort with "zend_mm_heap corrupted", where all three were previously passed to munmap().