From 05bd1423eee788c3fda31fbaa99f9be2f3da1df3 Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Fri, 5 May 2023 12:00:32 +0200 Subject: [PATCH] Fix GH-11189: Exceeding memory limit in zend_hash_do_resize leaves the array in an invalid state There are more places in zend_hash.c where the resize happened after some values on the HashTable struct were set. I reordered them all, but writing a test for these would rely on the particular amount of bytes allocated at given points in time. --- NEWS | 2 ++ Zend/tests/gh11189.phpt | 29 +++++++++++++++++++++++++++++ Zend/tests/gh11189_1.phpt | 29 +++++++++++++++++++++++++++++ Zend/zend_hash.c | 17 ++++++++++------- 4 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 Zend/tests/gh11189.phpt create mode 100644 Zend/tests/gh11189_1.phpt diff --git a/NEWS b/NEWS index 91e22d72523..89384217236 100644 --- a/NEWS +++ b/NEWS @@ -5,6 +5,8 @@ PHP NEWS - Core: . Fixed bug GH-9068 (Conditional jump or move depends on uninitialised value(s)). (nielsdos) + . Fixed bug GH-11189 (Exceeding memory limit in zend_hash_do_resize leaves + the array in an invalid state). (Bob) - Opcache: . Fixed bug GH-11134 (Incorrect match default branch optimization). (ilutov) diff --git a/Zend/tests/gh11189.phpt b/Zend/tests/gh11189.phpt new file mode 100644 index 00000000000..f1c877f20ee --- /dev/null +++ b/Zend/tests/gh11189.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-11189: Exceeding memory limit in zend_hash_do_resize leaves the array in an invalid state (packed array) +--SKIPIF-- + +--INI-- +memory_limit=2M +--FILE-- + 0; --$i) { + $a[] = 2; + } + fwrite(STDOUT, "Success"); +}); + +$a = []; +// trigger OOM in a resize operation +while (1) { + $a[] = 1; +} + +?> +--EXPECTF-- +Success +Fatal error: Allowed memory size of %s bytes exhausted%s(tried to allocate %s bytes) in %s on line %d diff --git a/Zend/tests/gh11189_1.phpt b/Zend/tests/gh11189_1.phpt new file mode 100644 index 00000000000..53727908e5e --- /dev/null +++ b/Zend/tests/gh11189_1.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-11189: Exceeding memory limit in zend_hash_do_resize leaves the array in an invalid state (not packed array) +--SKIPIF-- + +--INI-- +memory_limit=2M +--FILE-- + 0; --$i) { + $a[] = 2; + } + fwrite(STDOUT, "Success"); +}); + +$a = ["not packed" => 1]; +// trigger OOM in a resize operation +while (1) { + $a[] = 1; +} + +?> +--EXPECTF-- +Success +Fatal error: Allowed memory size of %s bytes exhausted%s(tried to allocate %s bytes) in %s on line %d diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index a13bb196924..49c0df61436 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -309,8 +309,9 @@ ZEND_API void ZEND_FASTCALL zend_hash_packed_grow(HashTable *ht) if (ht->nTableSize >= HT_MAX_SIZE) { zend_error_noreturn(E_ERROR, "Possible integer overflow in memory allocation (%u * %zu + %zu)", ht->nTableSize * 2, sizeof(Bucket), sizeof(Bucket)); } - ht->nTableSize += ht->nTableSize; - HT_SET_DATA_ADDR(ht, perealloc2(HT_GET_DATA_ADDR(ht), HT_SIZE_EX(ht->nTableSize, HT_MIN_MASK), HT_USED_SIZE(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT)); + uint32_t newTableSize = ht->nTableSize * 2; + HT_SET_DATA_ADDR(ht, perealloc2(HT_GET_DATA_ADDR(ht), HT_SIZE_EX(newTableSize, HT_MIN_MASK), HT_USED_SIZE(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT)); + ht->nTableSize = newTableSize; } ZEND_API void ZEND_FASTCALL zend_hash_real_init(HashTable *ht, bool packed) @@ -346,8 +347,9 @@ ZEND_API void ZEND_FASTCALL zend_hash_packed_to_hash(HashTable *ht) ZEND_ASSERT(HT_SIZE_TO_MASK(nSize)); HT_ASSERT_RC1(ht); - HT_FLAGS(ht) &= ~HASH_FLAG_PACKED; + // Alloc before assign to avoid inconsistencies on OOM new_data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); + HT_FLAGS(ht) &= ~HASH_FLAG_PACKED; ht->nTableMask = HT_SIZE_TO_MASK(ht->nTableSize); HT_SET_DATA_ADDR(ht, new_data); memcpy(ht->arData, old_buckets, sizeof(Bucket) * ht->nNumUsed); @@ -387,8 +389,9 @@ ZEND_API void ZEND_FASTCALL zend_hash_extend(HashTable *ht, uint32_t nSize, bool if (packed) { ZEND_ASSERT(HT_FLAGS(ht) & HASH_FLAG_PACKED); if (nSize > ht->nTableSize) { - ht->nTableSize = zend_hash_check_size(nSize); - HT_SET_DATA_ADDR(ht, perealloc2(HT_GET_DATA_ADDR(ht), HT_SIZE_EX(ht->nTableSize, HT_MIN_MASK), HT_USED_SIZE(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT)); + uint32_t newTableSize = zend_hash_check_size(nSize); + HT_SET_DATA_ADDR(ht, perealloc2(HT_GET_DATA_ADDR(ht), HT_SIZE_EX(newTableSize, HT_MIN_MASK), HT_USED_SIZE(ht), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT)); + ht->nTableSize = newTableSize; } } else { ZEND_ASSERT(!(HT_FLAGS(ht) & HASH_FLAG_PACKED)); @@ -396,8 +399,8 @@ ZEND_API void ZEND_FASTCALL zend_hash_extend(HashTable *ht, uint32_t nSize, bool void *new_data, *old_data = HT_GET_DATA_ADDR(ht); Bucket *old_buckets = ht->arData; nSize = zend_hash_check_size(nSize); - ht->nTableSize = nSize; new_data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); + ht->nTableSize = nSize; ht->nTableMask = HT_SIZE_TO_MASK(ht->nTableSize); HT_SET_DATA_ADDR(ht, new_data); memcpy(ht->arData, old_buckets, sizeof(Bucket) * ht->nNumUsed); @@ -1217,8 +1220,8 @@ static void ZEND_FASTCALL zend_hash_do_resize(HashTable *ht) ZEND_ASSERT(HT_SIZE_TO_MASK(nSize)); - ht->nTableSize = nSize; new_data = pemalloc(HT_SIZE_EX(nSize, HT_SIZE_TO_MASK(nSize)), GC_FLAGS(ht) & IS_ARRAY_PERSISTENT); + ht->nTableSize = nSize; ht->nTableMask = HT_SIZE_TO_MASK(ht->nTableSize); HT_SET_DATA_ADDR(ht, new_data); memcpy(ht->arData, old_buckets, sizeof(Bucket) * ht->nNumUsed);