Skip to content

Commit 52cf7ab

Browse files
committed
Fix bug #80072: Root live tmpvars after GC
TMPVAR operands are destroyed using zval_ptr_dtor_nogc(), because they usually cannot contain cycles. However, there are some rare exceptions where this is possible, e.g. unserialize() return value. In such cases we rely on the producing code to root the value. If a GC run occurs between the rooting and consumption of the value, we would end up leaking it. To avoid this, root all live TMPVAR values after a GC run. Closes GH-7210.
1 parent 083d7f5 commit 52cf7ab

File tree

3 files changed

+56
-2
lines changed

3 files changed

+56
-2
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ PHP NEWS
44

55
- Core:
66
. Fixed bug #81202 (powerpc64 build fails on fibers). (krakjoe)
7+
. Fixed bug #80072 (Cyclic unserialize in TMPVAR operand may leak). (Nikita)
78

89
- Curl:
910
. Fixed bug #81085 (Support CURLOPT_SSLCERT_BLOB for cert strings).

Zend/tests/bug80072.phpt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
Bug #80072: Cyclic unserialize in TMPVAR operand may leak
3+
--FILE--
4+
<?php
5+
6+
try {
7+
$s = 'O:8:"stdClass":1:{s:1:"x";r:1;}';
8+
unserialize($s) % gc_collect_cycles();
9+
} catch (Error $e) {
10+
echo $e->getMessage(), "\n";
11+
}
12+
13+
$a[]=&$a == $a=&$b > gc_collect_cycles();
14+
15+
?>
16+
--EXPECT--
17+
Unsupported operand types: stdClass % int

Zend/zend_gc.c

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1427,6 +1427,7 @@ static int gc_remove_nested_data_from_buffer(zend_refcounted *ref, gc_root_buffe
14271427
}
14281428

14291429
static void zend_get_gc_buffer_release(void);
1430+
static void zend_gc_root_tmpvars(void);
14301431

14311432
ZEND_API int zend_gc_collect_cycles(void)
14321433
{
@@ -1465,9 +1466,8 @@ ZEND_API int zend_gc_collect_cycles(void)
14651466
/* nothing to free */
14661467
GC_TRACE("Nothing to free");
14671468
gc_stack_free(&stack);
1468-
zend_get_gc_buffer_release();
14691469
GC_G(gc_active) = 0;
1470-
return 0;
1470+
goto finish;
14711471
}
14721472

14731473
zend_fiber_switch_block();
@@ -1627,7 +1627,9 @@ ZEND_API int zend_gc_collect_cycles(void)
16271627
goto rerun_gc;
16281628
}
16291629

1630+
finish:
16301631
zend_get_gc_buffer_release();
1632+
zend_gc_root_tmpvars();
16311633
return count;
16321634
}
16331635

@@ -1661,6 +1663,40 @@ static void zend_get_gc_buffer_release() {
16611663
gc_buffer->start = gc_buffer->end = gc_buffer->cur = NULL;
16621664
}
16631665

1666+
/* TMPVAR operands are destroyed using zval_ptr_dtor_nogc(), because they usually cannot contain
1667+
* cycles. However, there are some rare exceptions where this is possible, in which case we rely
1668+
* on the producing code to root the value. If a GC run occurs between the rooting and consumption
1669+
* of the value, we would end up leaking it. To avoid this, root all live TMPVAR values here. */
1670+
static void zend_gc_root_tmpvars(void) {
1671+
zend_execute_data *ex = EG(current_execute_data);
1672+
for (; ex; ex = ex->prev_execute_data) {
1673+
zend_function *func = ex->func;
1674+
if (!func || !ZEND_USER_CODE(func->type)) {
1675+
continue;
1676+
}
1677+
1678+
uint32_t op_num = ex->opline - ex->func->op_array.opcodes;
1679+
for (uint32_t i = 0; i < func->op_array.last_live_range; i++) {
1680+
const zend_live_range *range = &func->op_array.live_range[i];
1681+
if (range->start > op_num) {
1682+
break;
1683+
}
1684+
if (range->end <= op_num) {
1685+
continue;
1686+
}
1687+
1688+
uint32_t kind = range->var & ZEND_LIVE_MASK;
1689+
if (kind == ZEND_LIVE_TMPVAR) {
1690+
uint32_t var_num = range->var & ~ZEND_LIVE_MASK;
1691+
zval *var = ZEND_CALL_VAR(ex, var_num);
1692+
if (Z_REFCOUNTED_P(var)) {
1693+
gc_check_possible_root(Z_COUNTED_P(var));
1694+
}
1695+
}
1696+
}
1697+
}
1698+
}
1699+
16641700
#ifdef ZTS
16651701
size_t zend_gc_globals_size(void)
16661702
{

0 commit comments

Comments
 (0)