1
0
mirror of https://github.com/php/php-src.git synced 2026-03-24 00:02:20 +01:00

Split the live-ranges of loop variables again (#20865)

* Fix use-after-free in FE_FREE with GC interaction

When FE_FREE with ZEND_FREE_ON_RETURN frees the loop variable during
an early return from a foreach loop, the live range for the loop
variable was incorrectly extending past the FE_FREE to the normal
loop end. This caused GC to access the already-freed loop variable
when it ran after the RETURN opcode, resulting in use-after-free.

Fix by splitting the ZEND_LIVE_LOOP range when an FE_FREE with
ZEND_FREE_ON_RETURN is encountered:
- One range covers the early return path up to the FE_FREE
- A separate range covers the normal loop end FE_FREE
- Multiple early returns create multiple separate ranges

* Split the live-ranges of loop variables again

b0af9ac733 removed the live-range splitting of foreach variables, however it only added handling to ZEND_HANDLE_EXCEPTION.
This was sort-of elegant, until it was realized in 8258b7731b that it would leak the return variable, requiring some more special handling.
At some point we added live tmpvar rooting in 52cf7ab8a2, but this did not take into account already freed loop variables, which also might happen during ZEND_RETURN, which cannot be trivially accounted for, without even more complicated handling in zend_gc_*_tmpvars() functions.

This commit also proposes a simpler way of tracking the loop end in loopvar freeing ops: handle it directly during live range computation rather than during compilation, eliminating the need for opcache to handle it specifically.
Further, opcache was using live_ranges in its basic block computation in the past, which it no longer does. Thus this complication is no longer necessary and this approach should be actually simpler now.

Closes #20766.

Signed-off-by: Bob Weinand <bobwei9@hotmail.com>

---------

Signed-off-by: Bob Weinand <bobwei9@hotmail.com>
Co-authored-by: Gustavo Lopes <mail@geleia.net>
This commit is contained in:
Bob Weinand
2026-01-15 16:13:43 +01:00
committed by GitHub
parent 2c112e3696
commit 27ed48c0be
14 changed files with 194 additions and 83 deletions

1
NEWS
View File

@@ -9,6 +9,7 @@ PHP NEWS
. Fix OSS-Fuzz #472563272 (Borked block_pass JMP[N]Z optimization). (ilutov)
. Fixed bug GH-GH-20914 (Internal enums can be cloned and compared). (Arnaud)
. Fix OSS-Fuzz #474613951 (Leaked parent property default value). (ilutov)
. Fixed bug GH-20766 (Use-after-free in FE_FREE with GC interaction). (Bob)
- Date:
. Update timelib to 2022.16. (Derick)

View File

@@ -122,6 +122,10 @@ static void zend_dump_unused_op(const zend_op *opline, znode_op op, uint32_t fla
if (op.num != (uint32_t)-1) {
fprintf(stderr, " try-catch(%u)", op.num);
}
} else if (ZEND_VM_OP_LOOP_END == (flags & ZEND_VM_OP_MASK)) {
if (opline->extended_value & ZEND_FREE_ON_RETURN) {
fprintf(stderr, " loop-end(+%u)", op.num);
}
} else if (ZEND_VM_OP_THIS == (flags & ZEND_VM_OP_MASK)) {
fprintf(stderr, " THIS");
} else if (ZEND_VM_OP_NEXT == (flags & ZEND_VM_OP_MASK)) {

View File

@@ -1,37 +1,40 @@
--TEST--
GC 050: Destructor are never called twice
GC 050: Try/finally in foreach should create separate live ranges
--FILE--
<?php
class G
{
public static $v;
}
class WithDestructor
{
public function __destruct()
{
echo "d\n";
G::$v = $this;
function f(int $n): object {
try {
foreach ((array) $n as $v) {
if ($n === 1) {
try {
$a = new stdClass;
return $a;
} finally {
return $ret = $a;
}
}
if ($n === 2) {
$b = new stdClass;
return $ret = $b;
}
}
} finally {
$ret->v = 1;
}
return new stdClass;
}
$o = new WithDestructor();
$weakO = \WeakReference::create($o);
echo "---\n";
unset($o);
echo "---\n";
var_dump($weakO->get() !== null); // verify if kept allocated
G::$v = null;
echo "---\n";
var_dump($weakO->get() !== null); // verify if released
for ($i = 0; $i < 100000; $i++) {
// Create cyclic garbage to trigger GC
$a = new stdClass;
$b = new stdClass;
$a->r = $b;
$b->r = $a;
$r = f($i % 2 + 1);
}
echo "OK\n";
?>
--EXPECT--
---
d
---
bool(true)
---
bool(false)
OK

29
Zend/tests/gc_051.phpt Normal file
View File

@@ -0,0 +1,29 @@
--TEST--
GC 048: FE_FREE should mark variable as UNDEF to prevent use-after-free during GC
--FILE--
<?php
// FE_FREE frees the iterator but doesn't set zval to UNDEF
// When GC runs during RETURN, zend_gc_remove_root_tmpvars() may access freed memory
function test_foreach_early_return(string $s): object {
foreach ((array) $s as $v) {
$obj = new stdClass;
// in the early return, the VAR for the cast result is still live
return $obj; // the return may trigger GC
}
}
for ($i = 0; $i < 100000; $i++) {
// create cyclic garbage to fill GC buffer
$a = new stdClass;
$b = new stdClass;
$a->ref = $b;
$b->ref = $a;
$result = test_foreach_early_return("x");
}
echo "OK\n";
?>
--EXPECT--
OK

36
Zend/tests/gc_052.phpt Normal file
View File

@@ -0,0 +1,36 @@
--TEST--
GC 049: Multiple early returns from foreach should create separate live ranges
--FILE--
<?php
function f(int $n): object {
foreach ((array) $n as $v) {
if ($n === 1) {
$a = new stdClass;
return $a;
}
if ($n === 2) {
$b = new stdClass;
return $b;
}
if ($n === 3) {
$c = new stdClass;
return $c;
}
}
return new stdClass;
}
for ($i = 0; $i < 100000; $i++) {
// Create cyclic garbage to trigger GC
$a = new stdClass;
$b = new stdClass;
$a->r = $b;
$b->r = $a;
$r = f($i % 3 + 1);
}
echo "OK\n";
?>
--EXPECT--
OK

37
Zend/tests/gc_053.phpt Normal file
View File

@@ -0,0 +1,37 @@
--TEST--
GC 050: Destructor are never called twice
--FILE--
<?php
class G
{
public static $v;
}
class WithDestructor
{
public function __destruct()
{
echo "d\n";
G::$v = $this;
}
}
$o = new WithDestructor();
$weakO = \WeakReference::create($o);
echo "---\n";
unset($o);
echo "---\n";
var_dump($weakO->get() !== null); // verify if kept allocated
G::$v = null;
echo "---\n";
var_dump($weakO->get() !== null); // verify if released
?>
--EXPECT--
---
d
---
bool(true)
---
bool(false)

View File

@@ -4673,20 +4673,6 @@ static void cleanup_unfinished_calls(zend_execute_data *execute_data, uint32_t o
}
/* }}} */
static const zend_live_range *find_live_range(const zend_op_array *op_array, uint32_t op_num, uint32_t var_num) /* {{{ */
{
int i;
for (i = 0; i < op_array->last_live_range; i++) {
const zend_live_range *range = &op_array->live_range[i];
if (op_num >= range->start && op_num < range->end
&& var_num == (range->var & ~ZEND_LIVE_MASK)) {
return range;
}
}
return NULL;
}
/* }}} */
static void cleanup_live_vars(zend_execute_data *execute_data, uint32_t op_num, uint32_t catch_op_num) /* {{{ */
{
int i;
@@ -4702,6 +4688,16 @@ static void cleanup_live_vars(zend_execute_data *execute_data, uint32_t op_num,
uint32_t var_num = range->var & ~ZEND_LIVE_MASK;
zval *var = EX_VAR(var_num);
/* Handle the split range for loop vars */
if (catch_op_num) {
zend_op *final_op = EX(func)->op_array.opcodes + range->end;
if (final_op->extended_value & ZEND_FREE_ON_RETURN && (final_op->opcode == ZEND_FE_FREE || final_op->opcode == ZEND_FREE)) {
if (catch_op_num < range->end + final_op->op2.num) {
continue;
}
}
}
if (kind == ZEND_LIVE_TMPVAR) {
zval_ptr_dtor_nogc(var);
} else if (kind == ZEND_LIVE_NEW) {

View File

@@ -981,6 +981,35 @@ static void zend_calc_live_ranges(
/* OP_DATA is really part of the previous opcode. */
last_use[var_num] = opnum - (opline->opcode == ZEND_OP_DATA);
}
} else if ((opline->opcode == ZEND_FREE || opline->opcode == ZEND_FE_FREE) && opline->extended_value & ZEND_FREE_ON_RETURN) {
int jump_offset = 1;
while (((opline + jump_offset)->opcode == ZEND_FREE || (opline + jump_offset)->opcode == ZEND_FE_FREE)
&& (opline + jump_offset)->extended_value & ZEND_FREE_ON_RETURN) {
++jump_offset;
}
// loop var frees directly precede the jump (or return) operand, except that ZEND_VERIFY_RETURN_TYPE may happen first.
if ((opline + jump_offset)->opcode == ZEND_VERIFY_RETURN_TYPE) {
++jump_offset;
}
/* FREE with ZEND_FREE_ON_RETURN immediately followed by RETURN frees
* the loop variable on early return. We need to split the live range
* so GC doesn't access the freed variable after this FREE. */
uint32_t opnum_last_use = last_use[var_num];
zend_op *opline_last_use = op_array->opcodes + opnum_last_use;
ZEND_ASSERT(opline_last_use->opcode == opline->opcode); // any ZEND_FREE_ON_RETURN must be followed by a FREE without
if (opnum + jump_offset + 1 != opnum_last_use) {
emit_live_range_raw(op_array, var_num, opline->opcode == ZEND_FE_FREE ? ZEND_LIVE_LOOP : ZEND_LIVE_TMPVAR,
opnum + jump_offset + 1, opnum_last_use);
}
/* Update last_use so next range includes this FREE */
last_use[var_num] = opnum;
/* Store opline offset to loop end */
opline->op2.opline_num = opnum_last_use - opnum;
if (opline_last_use->extended_value & ZEND_FREE_ON_RETURN) {
opline->op2.opline_num += opline_last_use->op2.opline_num;
}
}
}
if (opline->op2_type & (IS_TMP_VAR|IS_VAR)) {

View File

@@ -3193,7 +3193,7 @@ ZEND_VM_COLD_CONST_HANDLER(47, ZEND_JMPNZ_EX, CONST|TMPVAR|CV, JMP_ADDR)
ZEND_VM_JMP(opline);
}
ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, ANY)
ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, LOOP_END)
{
USE_OPLINE
@@ -3202,7 +3202,7 @@ ZEND_VM_HANDLER(70, ZEND_FREE, TMPVAR, ANY)
ZEND_VM_NEXT_OPCODE_CHECK_EXCEPTION();
}
ZEND_VM_HOT_HANDLER(127, ZEND_FE_FREE, TMPVAR, ANY)
ZEND_VM_HOT_HANDLER(127, ZEND_FE_FREE, TMPVAR, LOOP_END)
{
zval *var;
USE_OPLINE
@@ -8140,24 +8140,11 @@ ZEND_VM_HANDLER(149, ZEND_HANDLE_EXCEPTION, ANY, ANY)
&& throw_op->extended_value & ZEND_FREE_ON_RETURN) {
/* exceptions thrown because of loop var destruction on return/break/...
* are logically thrown at the end of the foreach loop, so adjust the
* throw_op_num.
* throw_op_num to the final loop variable FREE.
*/
const zend_live_range *range = find_live_range(
&EX(func)->op_array, throw_op_num, throw_op->op1.var);
/* free op1 of the corresponding RETURN */
for (i = throw_op_num; i < range->end; i++) {
if (EX(func)->op_array.opcodes[i].opcode == ZEND_FREE
|| EX(func)->op_array.opcodes[i].opcode == ZEND_FE_FREE) {
/* pass */
} else {
if (EX(func)->op_array.opcodes[i].opcode == ZEND_RETURN
&& (EX(func)->op_array.opcodes[i].op1_type & (IS_VAR|IS_TMP_VAR))) {
zval_ptr_dtor(EX_VAR(EX(func)->op_array.opcodes[i].op1.var));
}
break;
}
}
throw_op_num = range->end;
uint32_t new_throw_op_num = throw_op_num + throw_op->op2.opline_num;
cleanup_live_vars(execute_data, throw_op_num, new_throw_op_num);
throw_op_num = new_throw_op_num;
}
/* Find the innermost try/catch/finally the exception was thrown in */

21
Zend/zend_vm_execute.h generated
View File

@@ -3265,24 +3265,11 @@ static ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL ZEND_HANDLE_EXCEPTION_SPEC_HANDLER(
&& throw_op->extended_value & ZEND_FREE_ON_RETURN) {
/* exceptions thrown because of loop var destruction on return/break/...
* are logically thrown at the end of the foreach loop, so adjust the
* throw_op_num.
* throw_op_num to the final loop variable FREE.
*/
const zend_live_range *range = find_live_range(
&EX(func)->op_array, throw_op_num, throw_op->op1.var);
/* free op1 of the corresponding RETURN */
for (i = throw_op_num; i < range->end; i++) {
if (EX(func)->op_array.opcodes[i].opcode == ZEND_FREE
|| EX(func)->op_array.opcodes[i].opcode == ZEND_FE_FREE) {
/* pass */
} else {
if (EX(func)->op_array.opcodes[i].opcode == ZEND_RETURN
&& (EX(func)->op_array.opcodes[i].op1_type & (IS_VAR|IS_TMP_VAR))) {
zval_ptr_dtor(EX_VAR(EX(func)->op_array.opcodes[i].op1.var));
}
break;
}
}
throw_op_num = range->end;
uint32_t new_throw_op_num = throw_op_num + throw_op->op2.opline_num;
cleanup_live_vars(execute_data, throw_op_num, new_throw_op_num);
throw_op_num = new_throw_op_num;
}
/* Find the innermost try/catch/finally the exception was thrown in */

View File

@@ -63,7 +63,7 @@ $vm_op_flags = array(
"ZEND_VM_OP_NUM" => 0x10,
"ZEND_VM_OP_JMP_ADDR" => 0x20,
"ZEND_VM_OP_TRY_CATCH" => 0x30,
// unused 0x40
"ZEND_VM_OP_LOOP_END" => 0x40,
"ZEND_VM_OP_THIS" => 0x50,
"ZEND_VM_OP_NEXT" => 0x60,
"ZEND_VM_OP_CLASS_FETCH" => 0x70,
@@ -111,6 +111,7 @@ $vm_op_decode = array(
"NUM" => ZEND_VM_OP_NUM,
"JMP_ADDR" => ZEND_VM_OP_JMP_ADDR,
"TRY_CATCH" => ZEND_VM_OP_TRY_CATCH,
"LOOP_END" => ZEND_VM_OP_LOOP_END,
"THIS" => ZEND_VM_OP_THIS,
"NEXT" => ZEND_VM_OP_NEXT,
"CLASS_FETCH" => ZEND_VM_OP_CLASS_FETCH,

View File

@@ -306,7 +306,7 @@ static uint32_t zend_vm_opcodes_flags[210] = {
0x00001301,
0x0100a173,
0x01040300,
0x00000005,
0x00004005,
0x00186703,
0x00106703,
0x08000007,
@@ -363,7 +363,7 @@ static uint32_t zend_vm_opcodes_flags[210] = {
0x0000a103,
0x00002003,
0x03000001,
0x00000005,
0x00004005,
0x01000700,
0x00000000,
0x00000000,

View File

@@ -48,6 +48,7 @@
#define ZEND_VM_OP_NUM 0x00000010
#define ZEND_VM_OP_JMP_ADDR 0x00000020
#define ZEND_VM_OP_TRY_CATCH 0x00000030
#define ZEND_VM_OP_LOOP_END 0x00000040
#define ZEND_VM_OP_THIS 0x00000050
#define ZEND_VM_OP_NEXT 0x00000060
#define ZEND_VM_OP_CLASS_FETCH 0x00000070

View File

@@ -28,9 +28,9 @@ $_main:
0000 T1 = PRE_INC_STATIC_PROP string("prop") string("X")
0001 T2 = ISSET_ISEMPTY_CV (empty) CV0($xx)
0002 JMPZ T2 0005
0003 FREE T1
0003 FREE T1 loop-end(+2)
0004 RETURN null
0005 FREE T1
0006 RETURN int(1)
LIVE RANGES:
1: 0001 - 0005 (tmp/var)
1: 0001 - 0003 (tmp/var)