From 0c87849da5a67a6f4cac4b6225cf026a7db5377b Mon Sep 17 00:00:00 2001 From: Julien Voisin Date: Wed, 26 Aug 2026 18:31:00 +0200 Subject: [PATCH 1/2] Validate large run map entries instead of assuming them (#23366) 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. --- Zend/zend_alloc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index fc7bc1f4d9d4..575b54b11a24 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -1527,9 +1527,11 @@ static zend_always_inline void zend_mm_free_heap(zend_mm_heap *heap, void *ptr Z ZEND_MM_CHECK(chunk->heap == heap, "zend_mm_heap corrupted"); if (EXPECTED(info & ZEND_MM_IS_SRUN)) { zend_mm_free_small(heap, ptr, ZEND_MM_SRUN_BIN_NUM(info)); - } else /* if (info & ZEND_MM_IS_LRUN) */ { - int pages_count = ZEND_MM_LRUN_PAGES(info); + } else { + /* A freed large run has a zeroed map entry, so this also rejects double frees. */ + ZEND_MM_CHECK(info & ZEND_MM_IS_LRUN, "zend_mm_heap corrupted"); + int pages_count = ZEND_MM_LRUN_PAGES(info); ZEND_MM_CHECK(ZEND_MM_ALIGNED_OFFSET(page_offset, ZEND_MM_PAGE_SIZE) == 0, "zend_mm_heap corrupted"); zend_mm_free_large(heap, chunk, page_num, pages_count); } @@ -1557,7 +1559,8 @@ static size_t zend_mm_size(zend_mm_heap *heap, void *ptr ZEND_FILE_LINE_DC ZEND_ ZEND_MM_CHECK(chunk->heap == heap, "zend_mm_heap corrupted"); if (EXPECTED(info & ZEND_MM_IS_SRUN)) { return bin_data_size[ZEND_MM_SRUN_BIN_NUM(info)]; - } else /* if (info & ZEND_MM_IS_LARGE_RUN) */ { + } else { + ZEND_MM_CHECK(info & ZEND_MM_IS_LRUN, "zend_mm_heap corrupted"); return ZEND_MM_LRUN_PAGES(info) * ZEND_MM_PAGE_SIZE; } #endif @@ -1752,7 +1755,8 @@ static zend_always_inline void *zend_mm_realloc_heap(zend_mm_heap *heap, void *p return ret; } while (0); - } else /* if (info & ZEND_MM_IS_LARGE_RUN) */ { + } else { + ZEND_MM_CHECK(info & ZEND_MM_IS_LRUN, "zend_mm_heap corrupted"); ZEND_MM_CHECK(ZEND_MM_ALIGNED_OFFSET(page_offset, ZEND_MM_PAGE_SIZE) == 0, "zend_mm_heap corrupted"); old_size = ZEND_MM_LRUN_PAGES(info) * ZEND_MM_PAGE_SIZE; if (size > ZEND_MM_MAX_SMALL_SIZE && size <= ZEND_MM_MAX_LARGE_SIZE) { From 7d6523170eaf5e0dcc8296eb8fe175232cd11ffb Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 24 Aug 2026 12:48:52 -0400 Subject: [PATCH 2/2] [intl] fix leak when iterating IntlBreakIterator parts iterators getPartsIterator() leaked because wrapping_obj was a counted self-reference, so the iterator never reached destruction and the current element was retained. wrapping_obj stays UNDEF; current and the backing BreakIterator are released from the iterator dtor, not the IntlIterator object dtor, so iterating a temporary parts iterator does not dangle. The string enumeration iterator still self-references because move_forward/rewind need the owner. Closes GH-23464 --- NEWS | 2 ++ .../breakiterator/breakiterator_iterators.cpp | 2 +- ext/intl/common/common_enum.cpp | 3 +- ...breakiter_parts_iterator_current_leak.phpt | 31 +++++++++++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 ext/intl/tests/breakiter_parts_iterator_current_leak.phpt diff --git a/NEWS b/NEWS index 935b31234898..c9c96543120d 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,8 @@ PHP NEWS . Fixed bug GH-19320 (FPM UID and GID overflow). (Pratik Bhujel) - Intl: + . Fixed a memory leak when iterating IntlBreakIterator::getPartsIterator() + results. (iliaal) . Fixed a double-free when IntlGregorianCalendar construction fails after the ICU constructor adopts the TimeZone. (iliaal) . Fixed bug GH-23094 (NumberFormatter parsing offsets use UTF-16 positions diff --git a/ext/intl/breakiterator/breakiterator_iterators.cpp b/ext/intl/breakiterator/breakiterator_iterators.cpp index 6817f52ffb02..be98a1ea3e04 100644 --- a/ext/intl/breakiterator/breakiterator_iterators.cpp +++ b/ext/intl/breakiterator/breakiterator_iterators.cpp @@ -242,7 +242,7 @@ void IntlIterator_from_BreakIterator_parts(zval *break_iter_zv, ii->iterator->index = 0; ((zoi_with_current*)ii->iterator)->destroy_it = _breakiterator_parts_destroy_it; - ZVAL_OBJ_COPY(&((zoi_with_current*)ii->iterator)->wrapping_obj, Z_OBJ_P(object)); + ZVAL_UNDEF(&((zoi_with_current*)ii->iterator)->wrapping_obj); ZVAL_UNDEF(&((zoi_with_current*)ii->iterator)->current); ((zoi_break_iter_parts*)ii->iterator)->bio = Z_INTL_BREAKITERATOR_P(break_iter_zv); diff --git a/ext/intl/common/common_enum.cpp b/ext/intl/common/common_enum.cpp index 58ebeabcb406..79853f7f6ef5 100644 --- a/ext/intl/common/common_enum.cpp +++ b/ext/intl/common/common_enum.cpp @@ -35,6 +35,8 @@ zend_object_handlers IntlIterator_handlers; void zoi_with_current_dtor(zend_object_iterator *iter) { zoi_with_current *zoiwc = (zoi_with_current*)iter; + iter->funcs->invalidate_current(iter); + zoiwc->destroy_it(iter); zval_ptr_dtor(&zoiwc->wrapping_obj); ZVAL_UNDEF(&zoiwc->wrapping_obj); } @@ -147,7 +149,6 @@ static void IntlIterator_objects_dtor(zend_object *object) { IntlIterator_object *ii = php_intl_iterator_fetch_object(object); if (ii->iterator) { - ((zoi_with_current*)ii->iterator)->destroy_it(ii->iterator); OBJ_RELEASE(&ii->iterator->std); ii->iterator = NULL; } diff --git a/ext/intl/tests/breakiter_parts_iterator_current_leak.phpt b/ext/intl/tests/breakiter_parts_iterator_current_leak.phpt new file mode 100644 index 000000000000..a006d7f2f792 --- /dev/null +++ b/ext/intl/tests/breakiter_parts_iterator_current_leak.phpt @@ -0,0 +1,31 @@ +--TEST-- +IntlPartsIterator must not leak, and a temporary one must not dangle +--EXTENSIONS-- +intl +--FILE-- +setText('hello world'); + return $bi->getPartsIterator(); +} + +foreach (parts() as $part) { + echo "[$part]\n"; +} + +$bi = IntlBreakIterator::createWordInstance('en'); +$bi->setText('hello world foo bar baz'); +$m0 = memory_get_usage(); +for ($i = 0; $i < 20000; $i++) { + foreach ($bi->getPartsIterator() as $v) { + break; + } +} +var_dump(memory_get_usage() - $m0 < 1024 * 1024); +?> +--EXPECT-- +[hello] +[ ] +[world] +bool(true)