Fix use-after-free in FE_FREE with GC interaction

[cataphract]

function f(string $s) {
    foreach ((array) $s as $v) {
        $o = new stdClass;
        return $o;
    }
}

compiles to:

Live ranges:
  V4: 0003 - 0010 (loop)
  V5: 0005 - 0006 (new)

Opcodes:
 0  2  CV0($s) = ZEND_RECV
 1  3  T3 = ZEND_CAST CV0($s)
 2  3  V4 = ZEND_FE_RESET_R T3, ->10
 3  3  ZEND_FE_FETCH_R V4, CV1($v)
 4  4  V5 = ZEND_NEW string("stdClass")
 5  4  ZEND_DO_FCALL
 6  4  ZEND_ASSIGN CV2($o), V5
 7  5  ZEND_FE_FREE V4
 8  5  ZEND_RETURN CV2($o)
 9  3  ZEND_JMP ->3
10  3  ZEND_FE_FREE V4
11  7  ZEND_RETURN null

Since we're returning early, in instruction 8, V4 has been freed. However, ZEND_RETURN may start GC:

if (Z_OPT_REFCOUNTED_P(retval_ptr)) {
if (EXPECTED(!Z_OPT_ISREF_P(retval_ptr))) {
    if (EXPECTED(!(EX_CALL_INFO() & (ZEND_CALL_CODE|ZEND_CALL_OBSERVED)))) {
	zend_refcounted *ref = Z_COUNTED_P(retval_ptr);
	ZVAL_COPY_VALUE(return_value, retval_ptr);
	if (GC_MAY_LEAK(ref)) {
	    SAVE_OPLINE();
	    gc_possible_root(ref);

Eventually, zend_gc_collect_cycles calls zend_gc_remove_root_tmpvars. This function is slighly misnamed, because it also removes from the GC buffer VARs that are loop variables, like V4. And instruction 8 is within the live range of V4. Therefore, it attempts to remove V4 from the buffer. But at this point, the zend_refcount behind V4 has already been destroyed. Hence, we have a use-after free:

$ USE_ZEND_ALLOC=0 valgrind ~/php/8.4.15-release/bin/php test_gc_final.php
==3125745== Memcheck, a memory error detector
==3125745== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==3125745== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==3125745== Command: /home/glopes/php/8.4.15-release/bin/php test_gc_final.php
==3125745==
==3125745== Invalid read of size 4
==3125745==    at 0x9E9A5C: zend_gc_remove_root_tmpvars (zend_gc.c:2209)
==3125745==    by 0x9E8983: zend_gc_collect_cycles (zend_gc.c:1921)
==3125745==    by 0x9E3C0E: gc_possible_root_when_full (zend_gc.c:664)
==3125745==    by 0x9E3F0C: gc_possible_root (zend_gc.c:714)
==3125745==    by 0x9D6CE4: execute_ex (zend_vm_execute.h:62997)
==3125745==    by 0x9DC630: zend_execute (zend_vm_execute.h:64319)
==3125745==    by 0xAB4C0B: zend_execute_script (zend.c:1934)
==3125745==    by 0x6F34DB: php_execute_script_ex (main.c:2577)
==3125745==    by 0x6F3659: php_execute_script (main.c:2617)
==3125745==    by 0xAB6D61: do_cli (php_cli.c:935)
==3125745==    by 0xAB7F38: main (php_cli.c:1310)
==3125745==  Address 0x6405c94 is 4 bytes inside a block of size 56 free'd
==3125745==    at 0x484B27F: free (vg_replace_malloc.c:872)
==3125745==    by 0x7F2049: __zend_free (zend_alloc.c:3322)
==3125745==    by 0x7EACFE: _efree_56 (zend_alloc.c:2710)
==3125745==    by 0x9FCF9C: zend_array_destroy (zend_hash.c:1873)
==3125745==    by 0xAA4E5B: rc_dtor_func (zend_variables.c:57)
==3125745==    by 0x9CC9F3: ZEND_FE_FREE_SPEC_TMPVAR_HANDLER (zend_vm_execute.h:15189)
==3125745==    by 0x9CC9F3: execute_ex (zend_vm_execute.h:60761)
==3125745==    by 0x9DC630: zend_execute (zend_vm_execute.h:64319)
==3125745==    by 0xAB4C0B: zend_execute_script (zend.c:1934)
==3125745==    by 0x6F34DB: php_execute_script_ex (main.c:2577)
==3125745==    by 0x6F3659: php_execute_script (main.c:2617)
==3125745==    by 0xAB6D61: do_cli (php_cli.c:935)
==3125745==    by 0xAB7F38: main (php_cli.c:1310)
==3125745==  Block was alloc'd at
==3125745==    at 0x4848899: malloc (vg_replace_malloc.c:381)
==3125745==    by 0x7F1E73: __zend_malloc (zend_alloc.c:3294)
==3125745==    by 0x7E77A6: _emalloc_56 (zend_alloc.c:2662)
==3125745==    by 0x9EF869: _zend_new_array_0 (zend_hash.c:284)
==3125745==    by 0x978D19: ZEND_CAST_SPEC_CV_HANDLER (zend_vm_execute.h:41066)
==3125745==    by 0x9D6E9C: execute_ex (zend_vm_execute.h:63068)
==3125745==    by 0x9DC630: zend_execute (zend_vm_execute.h:64319)
==3125745==    by 0xAB4C0B: zend_execute_script (zend.c:1934)
==3125745==    by 0x6F34DB: php_execute_script_ex (main.c:2577)
==3125745==    by 0x6F3659: php_execute_script (main.c:2617)
==3125745==    by 0xAB6D61: do_cli (php_cli.c:935)
==3125745==    by 0xAB7F38: main (php_cli.c:1310)

The solution adopted was to mark the loop variable zval as UNDEF.

[bwoebi]

This really is a bug in live_range computation :-(
The whole point of live ranges is to not have to zero freed temporaries.

Essentially a proper fix would be creating gaps in live ranges from any freeing op having ZEND_FREE_ON_RETURN as extended_value until the actual ZEND_RETURN; then marking the ZEND_RETURN as starting op for the new live range.

[cataphract]

@bwoebi OK, I've updated the live ranges calculation. Now I have:

Live ranges:
  V4: [0003, 0008) (loop)
  V5: [0005, 0006) (new)
  V4: [0010, 0011) (loop)

Opcodes:
 0   5  CV0($s) = ZEND_RECV
 1   6  T3 = ZEND_CAST CV0($s)  ; to array
 2   6  V4 = ZEND_FE_RESET_R T3, ->11
 3   6  ZEND_FE_FETCH_R V4, CV1($v)
 4   7  V5 = ZEND_NEW string("stdClass")
 5   7  ZEND_DO_FCALL
 6   7  ZEND_ASSIGN CV2($obj), V5
 7   9  ZEND_VERIFY_RETURN_TYPE CV2($obj)
 8   9  ZEND_FE_FREE V4
 9   9  ZEND_RETURN CV2($obj)
10   6  ZEND_JMP ->3
11   6  ZEND_FE_FREE V4
12  11  ZEND_VERIFY_RETURN_TYPE
13  11  ZEND_RETURN null

I need to look at the failures, but is this the direction?