[GIT-PULLS] [php-src] PR #23366: Validate large run map entries instead of assuming them
[email protected] (jvoisin)
| Newsgroups | php.git-pulls |
|---|---|
| Message-ID | <f9vqL6a7OKlvwaKY70vg07EEfLT3VGef7qIIhwtNLdQ@main.internal.php.net> |
Pull Request: https://github.com/php/php-src/pull/23366
Author: jvoisin
Three places dispatch on the page map entry of a pointer, and reach the large-run case by elimination, with the assumption written down as a comment rather than checked:
if (EXPECTED(info & ZEND_MM_IS_SRUN)) {
...
} else /* if (info & ZEND_MM_IS_LRUN) */ {
The assumption does not always hold: when ZEND_MM_IS_FRUN is 0 and zend_mm_free_pages_ex() zeroes chunk->map[page_num], a pointer to a large run that has already been freed has info == 0, so it fails the SRUN test, and falls into the large-run branch. There, ZEND_MM_LRUN_PAGES(0) is 0, and the three callers quietly degrade:
- zend_mm_free_heap() frees a run of zero pages, i.e. a double free of a large block is accepted and does nothing at all.
- zend_mm_size() reports a block size of 0.
- zend_mm_realloc_heap() takes old_size 0 and reallocates from there.
A large-block double free or a use of a freed pointer is silently absorbed by the allocator instead of being a hard failure. This commit promotes the comment to a real ZEND_MM_CHECK() in all three. The value is already in a register at that point, so it costs a test and a branch.
This was checked under GDB by allocating a large block, freeing it, and then reusing the pointer. Before, _efree() returned normally, _zend_mem_block_size() returned 0 and _erealloc() returned a new pointer. After this commit, each of the three aborts with "zend_mm_heap corrupted".
Amusingly, the two comments naming ZEND_MM_IS_LARGE_RUN referred to a macro that does not exist: the real name is ZEND_MM_IS_LRUN.