Segfault when using ReflectionFiber

[danog]

Description

The following code: https://paste.daniil.it/segfault_reflectionfiber_v2.tar.xz (all dependencies are exactly the ones specified in composer.lock, with a few manual patches made to vendor/revolt/event-loop/src/EventLoop/Internal/AbstractDriver.php, vendor/danog/madelineproto/src/GarbageCollector.php to enable ReflectionFiber logging; use API ID and API hash 1, 1 to test)

Resulted in a segfault with this gdb backtrace:

(gdb) back
#0  zend_fetch_debug_backtrace (return_value=0x7fffe727d2e0, skip_last=<optimized out>, options=1, limit=0) at /usr/src/debug/php/php-8.2.5/Zend/zend_builtin_functions.c:1748
#1  0x00005555557713d4 in zim_ReflectionFiber_getTrace (execute_data=0x7fffe727d310, return_value=0x7fffe727d2e0) at /usr/src/debug/php/php-8.2.5/ext/reflection/php_reflection.c:7066
#2  0x00005555558f034c in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (execute_data=0x7fffe727d1e0) at /usr/src/debug/php/php-8.2.5/Zend/zend_vm_execute.h:1951
#3  0x0000555555939438 in execute_ex (ex=<optimized out>) at /usr/src/debug/php/php-8.2.5/Zend/zend_vm_execute.h:55823
#4  0x00005555558b57fe in zend_call_function (fci=<optimized out>, fci_cache=<optimized out>) at /usr/src/debug/php/php-8.2.5/Zend/zend_execute_API.c:947
#5  0x000055555596ba5f in zend_fiber_execute (transfer=0x7fffe63fbfb0) at /usr/src/debug/php/php-8.2.5/Zend/zend_fibers.c:504
#6  0x000055555596b7e2 in zend_fiber_trampoline (data=...) at /usr/src/debug/php/php-8.2.5/Zend/zend_fibers.c:299
#7  0x0000555555a05e97 in make_fcontext () at /usr/src/debug/php/php-8.2.5/Zend/asm/make_x86_64_sysv_elf_gas.S:135
#8  0x0000000000000000 in ?? ()

zbacktrace doesn't work.

But I expected this output instead: No segfault

PHP Version

8.2.5

Operating System

Arch linux

[iluuu1994]

Thank you! I can reproduce this. Interestingly, I also get errors in emalloc with ASAN. When disabling ZMM I can also reproduce the issue you've mentioned above. Unfortunately, the issue is hard to understand without a condensed example.

[danog]

Unfortunately, the issue is hard to understand without a condensed example.

Precisely my problem as well, was hoping someone here could make more sense of it :)

[nielsdos]

I tried to debug this for a bit now. Look like fiber->execute_data->func and fiber->execute_data->opline are pointing to garbage or uninitialised values when ZEND_METHOD(ReflectionFiber, getTrace) executes. Didn't really find a root cause for this yet.

[bwoebi]

The root cause is the fiber->execute_data not being updated when a fiber is switched from due to resumption of another fiber.

Essentially, it's pointing to memory in the middle of its VM stack.

This non-crashing example, but having the wrong output illustrates the issue:
Expected would be the top frame of the $f fiber to be Fiber->start(), but it's the position of the last suspension.

root@bobtop-ubuntu:~/php-src-8.2# php -r 'function f() { Fiber::suspend(); } function g() { (new Fiber(function() { global $f; var_dump((new ReflectionFiber($f))->getTrace()); }))->start(); } $f = new Fiber(function() { f(); g(); }); $f->start(); $f->resume();'
array(3) {
  [0]=>
  array(6) {
    ["file"]=>
    string(17) "Command line code"
    ["line"]=>
    int(1)
    ["function"]=>
    string(7) "suspend"
    ["class"]=>
    string(5) "Fiber"
    ["type"]=>
    string(2) "::"
    ["args"]=>
    array(0) {
    }
  }
  [1]=>
  array(4) {
    ["file"]=>
    string(17) "Command line code"
    ["line"]=>
    int(1)
    ["function"]=>
    string(1) "g"
    ["args"]=>
    array(0) {
    }
  }
  [2]=>
  array(2) {
    ["function"]=>
    string(9) "{closure}"
    ["args"]=>
    array(0) {
    }
  }
}

A crashing example (by stack spraying with that max() call):

php -r 'function f() { Fiber::suspend(); } function g() { (new Fiber(function() { global $f; var_dump((new ReflectionFiber($f))->getTrace()); }))->start(); } $f = new Fiber(function() { f(); max(...[1,2,3,4,5,6,7,8,9,10,11,12]); g(); }); $f->start(); $f->resume();'

However, I'm not sure what the proper fix would be:

• Fiber::resume() has control of the fiber, but technically another extension could call zend_fiber_resume(), bypassing the method handler.
• zend_fiber_switch_context() could check whether EG(active_fiber)->kind == zend_ce_fiber and conditionally set fiber->execute_data.
• Moving the zend_fiber_vm_state to the fiber context instead of the stack and exposing it in the header and just accessing the data from there. (Not possible on non-master branch).

I'd prefer the latter for master.

For the current branches, we probably have to just special case zend_ce_fiber in zend_fiber_switch_context:

diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c
index 53f78417f5..6f55455190 100644
--- a/Zend/zend_fibers.c
+++ b/Zend/zend_fibers.c
@@ -395,6 +395,10 @@ ZEND_API void zend_fiber_switch_context(zend_fiber_transfer *transfer)

        zend_observer_fiber_switch_notify(from, to);

+       if (from->kind == zend_ce_fiber) {
+               zend_fiber_from_context(from)->execute_data = EG(current_execute_data);
+       }
+
        zend_fiber_capture_vm_state(&state);

        to->status = ZEND_FIBER_STATUS_RUNNING;

Maybe someone else has a better idea?

[nielsdos]

Imo the plan for stable branches, and the plan (the last one, which is also your preference) for master sound good.

[danog]

Ping, could anyone merge the suggested fix? :)