From a72c87b03cb47311995ce5cebf0004d51bcd17d3 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 25 Sep 2024 18:51:14 +0200 Subject: [PATCH 1/2] Fix GH-16054: Segmentation fault when resizing hash table iterator list while adding zend_array_dup_ht_iterators() loops over the hash table iterators and can call zend_hash_iterator_add(). zend_hash_iterator_add() can resize the array causing a crash in zend_array_dup_ht_iterators(). We solve this by refetching the iter pointer after an add happened. --- Zend/zend_hash.c | 7 ++++++- ext/spl/tests/gh16054.phpt | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 ext/spl/tests/gh16054.phpt diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 6668c4c17c66..6fa743d0af72 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -2347,11 +2347,16 @@ static zend_always_inline bool zend_array_dup_element(HashTable *source, HashTab // We need to duplicate iterators to be able to search through all copy-on-write copies to find the actually iterated HashTable and position back static void zend_array_dup_ht_iterators(HashTable *source, HashTable *target) { HashTableIterator *iter = EG(ht_iterators); - HashTableIterator *end = iter + EG(ht_iterators_used); + uint32_t nr_iterators_used = EG(ht_iterators_used); + HashTableIterator *end = iter + nr_iterators_used; while (iter != end) { if (iter->ht == source) { + size_t iter_index = iter - EG(ht_iterators); uint32_t copy_idx = zend_hash_iterator_add(target, iter->pos); + /* Refetch iter and recompute end because the memory may be reallocated. */ + end = EG(ht_iterators) + nr_iterators_used; + iter = &EG(ht_iterators)[iter_index]; HashTableIterator *copy_iter = EG(ht_iterators) + copy_idx; copy_iter->next_copy = iter->next_copy; iter->next_copy = copy_idx; diff --git a/ext/spl/tests/gh16054.phpt b/ext/spl/tests/gh16054.phpt new file mode 100644 index 000000000000..cef6e547afbf --- /dev/null +++ b/ext/spl/tests/gh16054.phpt @@ -0,0 +1,15 @@ +--TEST-- +GH-16054 (Segmentation fault when resizing hash table iterator list while adding) +--FILE-- + $v) { + if (++$counter > 200) break; +} +echo "ok\n"; +?> +--EXPECT-- +ok From c6c2941e5ccaf7f65c4c8cdab042b2220744c04a Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 25 Sep 2024 19:56:41 +0200 Subject: [PATCH 2/2] Use indices --- Zend/zend_hash.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 6fa743d0af72..ea1bfa3afc0f 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -2346,22 +2346,20 @@ static zend_always_inline bool zend_array_dup_element(HashTable *source, HashTab // We need to duplicate iterators to be able to search through all copy-on-write copies to find the actually iterated HashTable and position back static void zend_array_dup_ht_iterators(HashTable *source, HashTable *target) { - HashTableIterator *iter = EG(ht_iterators); - uint32_t nr_iterators_used = EG(ht_iterators_used); - HashTableIterator *end = iter + nr_iterators_used; + uint32_t iter_index = 0; + uint32_t end_index = EG(ht_iterators_used); - while (iter != end) { + while (iter_index != end_index) { + HashTableIterator *iter = &EG(ht_iterators)[iter_index]; if (iter->ht == source) { - size_t iter_index = iter - EG(ht_iterators); uint32_t copy_idx = zend_hash_iterator_add(target, iter->pos); - /* Refetch iter and recompute end because the memory may be reallocated. */ - end = EG(ht_iterators) + nr_iterators_used; + /* Refetch iter because the memory may be reallocated. */ iter = &EG(ht_iterators)[iter_index]; HashTableIterator *copy_iter = EG(ht_iterators) + copy_idx; copy_iter->next_copy = iter->next_copy; iter->next_copy = copy_idx; } - iter++; + iter_index++; } }