Skip to content

Commit 7b7d998

Browse files
committed
Fix symtable cache being used while cleaning symtable
We need to first clean the symtable and then check whether a cache slot is available for it. Otherwise, it may happen that a destructor runs while cleaning the table and uses up all the remaining slots in the cache. This is particularly insidious because once we overflow the cache, the first pointer we modify is symtable_cache_ptr, making it hard to understand what happened after the fact. Fixes oss-fuzz #30815.
1 parent 3646604 commit 7b7d998

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
--TEST--
2+
Symtable cache slots may be acquired while cleaning symtable
3+
--FILE--
4+
<?php
5+
class A {
6+
// Must be larger than the symtable cache.
7+
static $max = 40;
8+
function __destruct() {
9+
if (self::$max-- < 0) return;
10+
$x = 'y';
11+
$$x = new a;
12+
}
13+
}
14+
new A;
15+
16+
?>
17+
===DONE===
18+
--EXPECT--
19+
===DONE===

Zend/zend_execute.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3437,12 +3437,13 @@ ZEND_API void execute_internal(zend_execute_data *execute_data, zval *return_val
34373437

34383438
ZEND_API void zend_clean_and_cache_symbol_table(zend_array *symbol_table) /* {{{ */
34393439
{
3440+
/* Clean before putting into the cache, since clean could call dtors,
3441+
* which could use the cached hash. Also do this before the check for
3442+
* available cache slots, as those may be used by a dtor as well. */
3443+
zend_symtable_clean(symbol_table);
34403444
if (EG(symtable_cache_ptr) >= EG(symtable_cache_limit)) {
34413445
zend_array_destroy(symbol_table);
34423446
} else {
3443-
/* clean before putting into the cache, since clean
3444-
could call dtors, which could use cached hash */
3445-
zend_symtable_clean(symbol_table);
34463447
*(EG(symtable_cache_ptr)++) = symbol_table;
34473448
}
34483449
}

0 commit comments

Comments
 (0)