PHP crashes when execute_ex is overridden and a __call trampoline is used from internal code

[derickr]

Description

This is reproducible by first installing this extension, which pretty much only sets up a dummy overload of zend_execute_ex.

Then run the code below (by @Girgias), with the following command:

USE_ZEND_ALLOC=0 gdb --args php -n -dextension=skeleton trampoline.php

<?php

class DummyStreamWrapper
{
    /** @var resource|null */
    public $context;

    /** @var resource|null */
    public $handle;


    public function stream_cast(int $castAs)
    {
        return $this->handle;
    }


    public function stream_close(): void
    {
    }

    public function stream_open(string $path, string $mode, int $options = 0, ?string &$openedPath = null): bool
    {
        return true;
    }


    public function stream_read(int $count)
    {
        return 0;
    }


    public function stream_seek(int $offset, int $whence = SEEK_SET): bool
    {
        return true;
    }


    public function stream_set_option(int $option, int $arg1, ?int $arg2): bool
    {
        return false;
    }


    public function stream_stat()
    {
        return [];
    }


    public function stream_tell()
    {
        return [];
    }


    public function stream_truncate(int $newSize): bool
    {
        return true;
    }


    public function stream_write(string $data)
    {
    }


    public function unlink(string $path): bool
    {
        return false;
    }
}

class TrampolineTest {
    /** @var resource|null */
    public $context;

    /** @var object|null */
    private $wrapper;

    public function __call(string $name, array $arguments) {
        if (!$this->wrapper) {
            $this->wrapper = new DummyStreamWrapper();
        }
        echo 'Trampoline for ', $name, PHP_EOL;
        return $this->wrapper->$name(...$arguments);
	}

}

stream_wrapper_register('custom', TrampolineTest::class);


$fp = fopen("custom://myvar", "r+");
?>

This crashes with the following back trace:

Program received signal SIGSEGV, Segmentation fault.
0x0000555555c9d51a in ZEND_CALL_TRAMPOLINE_SPEC_OBSERVER_HANDLER () at /home/derick/dev/php/php-src.git/Zend/zend_vm_execute.h:3459
3459				LOAD_OPLINE();
(gdb) bt
#0  0x0000555555c9d51a in ZEND_CALL_TRAMPOLINE_SPEC_OBSERVER_HANDLER () at /home/derick/dev/php/php-src.git/Zend/zend_vm_execute.h:3459
#1  0x0000555555d09234 in execute_ex (ex=0x555556d01de0) at /home/derick/dev/php/php-src.git/Zend/zend_vm_execute.h:55911
#2  0x00007ffff36521df in skeleton_execute_ex (execute_data=0x555556d01de0) at /home/derick/dev/php/extension-skeleton/skeleton.c:48
#3  0x0000555555c43c5c in zend_call_function (fci=0x7fffffffb9f0, fci_cache=0x7fffffffb860) at /home/derick/dev/php/php-src.git/Zend/zend_execute_API.c:912
#4  0x0000555555c43168 in _call_user_function_impl (object=0x555556d5ea48, function_name=0x7fffffffba80, retval_ptr=0x7fffffffba70, param_count=0, params=0x0, named_params=0x0)
    at /home/derick/dev/php/php-src.git/Zend/zend_execute_API.c:712
#5  0x0000555555be95ca in php_userstreamop_close (stream=0x555556d5c850, close_handle=1) at /home/derick/dev/php/php-src.git/main/streams/userspace.c:708
#6  0x0000555555bdb464 in _php_stream_free (stream=0x555556d5c850, close_options=11) at /home/derick/dev/php/php-src.git/main/streams/streams.c:475
#7  0x0000555555bde057 in stream_resource_regular_dtor (rsrc=0x7fffffffbb40) at /home/derick/dev/php/php-src.git/main/streams/streams.c:1666
#8  0x0000555555c77cb6 in zend_resource_dtor (res=0x555556d5be60) at /home/derick/dev/php/php-src.git/Zend/zend_list.c:73
#9  0x0000555555c781ac in zend_close_rsrc_list (ht=0x555556a2fd30 <executor_globals+560>) at /home/derick/dev/php/php-src.git/Zend/zend_list.c:224
#10 0x0000555555c41b03 in zend_shutdown_executor_values (fast_shutdown=false) at /home/derick/dev/php/php-src.git/Zend/zend_execute_API.c:270
#11 0x0000555555c42405 in shutdown_executor () at /home/derick/dev/php/php-src.git/Zend/zend_execute_API.c:403
#12 0x0000555555c5a12c in zend_deactivate () at /home/derick/dev/php/php-src.git/Zend/zend.c:1271
#13 0x0000555555bbe61f in php_request_shutdown (dummy=0x0) at /home/derick/dev/php/php-src.git/main/main.c:1847
#14 0x0000555555dc5be7 in do_cli (argc=4, argv=0x555556a639c0) at /home/derick/dev/php/php-src.git/sapi/cli/php_cli.c:1135
#15 0x0000555555dc631f in main (argc=4, argv=0x555556a639c0) at /home/derick/dev/php/php-src.git/sapi/cli/php_cli.c:1367

What seems to happen here is that the LOAD_OPLINE tries to access opline from prev_execute_data (line 3458 sets execute_data to prev_execute_data, which is NULL in this case. The code in zend_vm_def.h is:

 3450         if (EXPECTED(zend_execute_ex == execute_ex)) {
 3451             LOAD_OPLINE_EX();
 3452             SAVE_OPLINE();
 3453             zend_observer_fcall_begin(execute_data);
 3454             ZEND_VM_ENTER_EX();
 3455         } else {
 3456             SAVE_OPLINE_EX();
 3457             zend_observer_fcall_begin(execute_data);
 3458             execute_data = EX(prev_execute_data);
 3459             LOAD_OPLINE();
 3460             ZEND_ADD_CALL_FLAG(call, ZEND_CALL_TOP);
 3461             zend_execute_ex(call);
 3462         }

I guess that there is no prev_execute_data, as this is called when cleaning up the stream resource, which isn't run from user land.

If you change line 3450 (and regenerate the executor with Zend/zend_vm_gen.php) to:

 3450         if (EXPECTED(zend_execute_ex == execute_ex) || !EX(prev_execute_data)) {
 3451             LOAD_OPLINE_EX();
 3452             SAVE_OPLINE();
 3453             zend_observer_fcall_begin(execute_data);
 3454             ZEND_VM_ENTER_EX();
 3455         } else {

Then the crash is gone, but the overloaded zend_execute_ex is not run. In Xdebug, that results in the __call line for stream_close to be missing in a function trace. You can see that in this sample, where the __call is there for stream_open, but not for stream_close.

TRACE START [2022-12-09 11:25:02.275874]
    0.0005          0   -> {main}() /tmp/issue2139/mine/trampoline.php:0
    0.0005          0     -> stream_wrapper_register($protocol = 'custom', $class = 'TrampolineTest') /tmp/issue2139/mine/trampoline.php:99
    0.0006          0     -> fopen($filename = 'custom://myvar', $mode = 'r+') /tmp/issue2139/mine/trampoline.php:102
    0.0006          0       -> TrampolineTest->stream_open('custom://myvar', 'r+', 0, NULL) /tmp/issue2139/mine/trampoline.php:102
    0.0006          0         -> TrampolineTest->__call($name = 'stream_open', $arguments = [0 => 'custom://myvar', 1 => 'r+', 2 => 0, 3 => NULL]) /tmp/issue2139/mine/trampoline.php:102
    0.0007          0           -> DummyStreamWrapper->stream_open($path = 'custom://myvar', $mode = 'r+', $options = 0, $openedPath = NULL) /tmp/issue2139/mine/trampoline.php:87
    0.0009          0   -> TrampolineTest->stream_close() /tmp/issue2139/mine/trampoline.php:0
    0.0009          0     -> DummyStreamWrapper->stream_close() /tmp/issue2139/mine/trampoline.php:87
    0.0010          0
TRACE END   [2022-12-09 11:25:02.276503]

So although the one line patch fixes the crash, it does not fix the issue.

I haven't figured out what the correct fix is.

PHP Version

PHP 8.1.15-dev, but also reproducible with PHP 8.2.1-dev

Operating System

Debian unstable, but it is not relevant

[derickr]

@iluuu1994, @bwoebi, would you have an idea what this could be?

[arnaud-lb]

I guess that there is no prev_execute_data, as this is called when cleaning up the stream resource, which isn't run from user land.

This seems right. The issue can be reproduced by calling a trampoline from any native code, as long as execute_ex is overridden:

array_map([new TrampolineTest, 'test'], [1]); // crash

I think that the effect of the first two first statements in

 3458             execute_data = EX(prev_execute_data);
 3459             LOAD_OPLINE(); // opline = execute_data->opline;
 3460             ZEND_ADD_CALL_FLAG(call, ZEND_CALL_TOP);
 3461             zend_execute_ex(call);

is never used:

• execute_ex does not use the value of opline / execute_data appart from saving it on entry and restoring it on return, due to the ZEND_CALL_TOP flag
• ZEND_CALL_TRAMPOLINE halts the VM immediately after zend_execute_ex(call)

A custom execute_ex may access opline / execute_data, but this seems unlikely.

There is a similar sequence in ZEND_DO_FCALL, but it makes more sense there because it doesn't halt the VM (and EX(prev_execute_data) is never null).

We could remove these two lines, or skip LOAD_OPLINE() if execute_data is null.

[derickr]

A custom execute_ex may access opline / execute_data, but this seems unlikely.

Xdebug definitely uses execute_data, for example, to get to a return value, but that would be the original execute_data, and not the one that is reset in line 3458.

I'll make a PR to not call LOAD_OPLINE() if execute_data is NULL.