[pull] master from php:master - #1243
Merged
Merged
Conversation
The shadow of a free list pointer is currently BSWAP(next) ^ shadow_key,
which does not depend on where it is stored, meaning that:
- zend_mm_free_small() encodes whatever heap->free_slot[bin] happens to
be, including NULL when the bin has been drained, meaning
the slot ends up holding shadow_key verbatim.
- A (next, shadow) pair harvested from one free slot is valid in every
other slot of every bin. An attacker who can read one free slot can
therefore forge a link anywhere in the heap without needing the key.
This commit adds the address of the shadow itself into the mix:
shadow = BSWAP(next) ^ shadow_key ^ (uintptr_t)&shadow
The holder term cancels on decode, so this is one extra xor on a register
that is already live, with no branch. Encoding NULL now yields
shadow_key ^ holder rather than the key, and a shadow only verifies in the
slot it was written for.
Using the address of the shadow rather than the address of the slot means
that zend_mm_get_next_free_slot() does not need to keep the slot alive
after fetching next, which keeps the register pressure unchanged.
This was verified under GDB: Freeing into a drained bin used to store shadow_key
exactly; it now stores shadow_key ^ holder (xoring the two back gives the
address of the shadow). Naïvely replaying a valid (next, shadow) pair from one
slot into another and traversing from it is accepted before this change and
aborts with "zend_mm_heap corrupted" after.
Performance-wise, the impact is in the noise level, which is expected as it more
or less adds a single `xor` instruction per `zend_mm_set_next_free_slot()`.
This commit is a follow up on 25360ef and c561f7d.
Co-authored-by: Arnaud Le Blanc <arnaud.lb@gmail.com>
Empty chunks are not always unmapped. zend_mm_delete_chunk() may retain
them in heap->cached_chunks so they can be reused without another mmap().
The list is linked through chunk headers that stay mapped and writable, so
an overwrite of a link controls the value that zend_mm_alloc_pages()
removes from the cache and hands to zend_mm_chunk_init(), which writes
through it and links it into the active chunk list.
Protect the list with the same key material as the small allocation
freelists. chunk->next keeps the plain pointer and the new
chunk->next_shadow holds an encoded copy:
next_shadow = BSWAPPTR(next) ^ heap->shadow_key ^ &chunk->next_shadow
The byte swap makes a small overwrite corrupt the most significant bytes
of the address, which is unlikely to yield another valid chunk. Mixing in
the address of next_shadow prevents a valid (next, next_shadow) pair from
being replayed into another chunk. The shadow is an integrity check, not a
secret; the secret remains heap->shadow_key.
Reading a cached link decodes the shadow, checks that the result is
chunk-aligned and that it matches chunk->next, and only then dereferences
it. The head of the list is stored in the heap rather than in a chunk
header, so it gets an alignment check of its own when it is popped.
Cached chunks outlive request resets and forks, so their shadows are
recomputed by zend_mm_rekey_cached_chunks() whenever zend_mm_refresh_key()
or zend_mm_refresh_key_child() changes the key. That walk validates every
link against its old shadow, so corruption is detected rather than
silently re-encoded.
The next_shadow field is carved out of the chunk header's reserve field,
so the header is still 64 bytes, and chunk->next remains the ordinary
doubly-linked-list pointer while the chunk is active.
create_segments() reserves requested_size bytes with MAP_32BIT, frees them, rounds the address up to the 2 MB huge page boundary, and then MAP_FIXED-maps requested_size bytes at the new address. The address goes up but the size stays the same, so the mapping ends up to 2 MB above the memory we reserved, and MAP_FIXED discards what is mapped there. If huge pages are available the remap succeeds and replaces that memory. If they are not, mmap() fails, but the kernel has already removed it and leaves a hole (mm/vma.c, vms_abort_munmap_vmas). On a normal host there is usually nothing above the reservation, so this is not visible. Under Rosetta 2 MAP_32BIT is not honored, the reservation lands directly below libc, and the overshoot unmaps its first pages: php-fpm then dies with SIGSEGV shortly after start. Reserve one extra huge page, so the aligned range always stays inside the reservation. zend_mm_chunk_alloc_int() already does this for 2 MB aligned chunks.
* PHP-8.4: ext/opcache: keep huge page remap inside the reserved range (#23554)
* PHP-8.5: ext/opcache: keep huge page remap inside the reserved range (#23554)
Use unique temporary file names for tests that may run concurrently, remove a CLEAN section that deletes another test's file, and replace fixed server ports with ephemeral ports.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )