Skip to content

Commit e9a7447

Browse files
committed
Fixed GH-16233: Observer segfault when calling user function in internal function via trampoline
In the test, I have an internal `__call` function for `_ZendTestMagicCallForward` that calls the global function with name `$name` via `call_user_function`. Note that observer writes the pointer to the previously observed frame in the last temporary of the new call frame (`*prev_observed_frame`). The following happens: First, we call `$test->callee`, this will be handled via a trampoline with T=2 for the two arguments. The call frame is allocated at this point. This call frame is not observed because it has `ZEND_ACC_CALL_VIA_TRAMPOLINE` set. Next we use ZEND_CALL_TRAMPOLINE to call the trampoline, this reuses the stack frame allocated earlier with T=2, but this time it is observed. The pointer to the previous frame is written outside of the call frame because `T` is too small (should be 3). We are now in the internal function `_ZendTestMagicCallForward::__call` where we call the global function `callee`. This will push a new call frame which will overlap `*prev_observed_frame`. This value gets overwritten by `zend_init_func_execute_data` when `EX(opline)` is set because `*prev_observed_frame` overlaps with `EX(opline)`. From now on, `*prev_observed_frame` is corrupted. When `zend_observer_fcall_end` is called this will result in reading wrong value `*prev_observed_frame` into `current_observed_frame`. This causes issues in `zend_observer_fcall_end_all` leading to the segfault we observe. Despite function with `ZEND_ACC_CALL_VIA_TRAMPOLINE` not being observed, the reuse of call frames makes problems when `T` is not large enough. To fix this, we make sure to add 1 to `T` if `ZEND_OBSERVER_ENABLED` is true.
1 parent f453d1a commit e9a7447

File tree

5 files changed

+79
-2
lines changed

5 files changed

+79
-2
lines changed

Zend/zend_object_handlers.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "zend_closures.h"
3131
#include "zend_compile.h"
3232
#include "zend_hash.h"
33+
#include "zend_observer.h"
3334

3435
#define DEBUG_OBJECT_HANDLERS 0
3536

@@ -1294,7 +1295,8 @@ ZEND_API zend_function *zend_get_call_trampoline_func(zend_class_entry *ce, zend
12941295
* value so that it doesn't contain garbage when the engine allocates space for the next stack
12951296
* frame. This didn't cause any issues until now due to "lucky" structure layout. */
12961297
func->last_var = 0;
1297-
func->T = (fbc->type == ZEND_USER_FUNCTION)? MAX(fbc->op_array.last_var + fbc->op_array.T, 2) : 2;
1298+
size_t min_T = 2 + ZEND_OBSERVER_ENABLED;
1299+
func->T = (fbc->type == ZEND_USER_FUNCTION)? MAX(fbc->op_array.last_var + fbc->op_array.T, min_T) : min_T;
12981300
func->filename = (fbc->type == ZEND_USER_FUNCTION)? fbc->op_array.filename : ZSTR_EMPTY_ALLOC();
12991301
func->line_start = (fbc->type == ZEND_USER_FUNCTION)? fbc->op_array.line_start : 0;
13001302
func->line_end = (fbc->type == ZEND_USER_FUNCTION)? fbc->op_array.line_end : 0;

ext/zend_test/test.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,23 @@ static ZEND_METHOD(_ZendTestMagicCall, __call)
879879
RETURN_ARR(zend_new_pair(&name_zv, arguments));
880880
}
881881

882+
static ZEND_METHOD(_ZendTestMagicCallForward, __call)
883+
{
884+
zend_string *name;
885+
zval *arguments;
886+
887+
ZEND_PARSE_PARAMETERS_START(2, 2)
888+
Z_PARAM_STR(name)
889+
Z_PARAM_ARRAY(arguments)
890+
ZEND_PARSE_PARAMETERS_END();
891+
892+
ZEND_IGNORE_VALUE(arguments);
893+
894+
zval func;
895+
ZVAL_STR(&func, name);
896+
call_user_function(NULL, NULL, &func, return_value, 0, NULL);
897+
}
898+
882899
PHP_INI_BEGIN()
883900
STD_PHP_INI_BOOLEAN("zend_test.replace_zend_execute_ex", "0", PHP_INI_SYSTEM, OnUpdateBool, replace_zend_execute_ex, zend_zend_test_globals, zend_test_globals)
884901
STD_PHP_INI_BOOLEAN("zend_test.register_passes", "0", PHP_INI_SYSTEM, OnUpdateBool, register_passes, zend_zend_test_globals, zend_test_globals)
@@ -993,6 +1010,8 @@ PHP_MINIT_FUNCTION(zend_test)
9931010

9941011
zend_test_magic_call = register_class__ZendTestMagicCall();
9951012

1013+
register_class__ZendTestMagicCallForward();
1014+
9961015
zend_register_functions(NULL, ext_function_legacy, NULL, EG(current_module)->type);
9971016

9981017
// Loading via dl() not supported with the observer API

ext/zend_test/test.stub.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ class _ZendTestMagicCall
5252
public function __call(string $name, array $args): mixed {}
5353
}
5454

55+
class _ZendTestMagicCallForward
56+
{
57+
public function __call(string $name, array $args): mixed {}
58+
}
59+
5560
class _ZendTestChildClass extends _ZendTestClass
5661
{
5762
public function returnsThrowable(): Exception {}

ext/zend_test/test_arginfo.h

Lines changed: 20 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/zend_test/tests/gh16233.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
GH-16233 (Observer segfault when calling user function in internal function via trampoline)
3+
--EXTENSIONS--
4+
zend_test
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.show_output=1
8+
zend_test.observer.observe_all=1
9+
--FILE--
10+
<?php
11+
12+
function callee() {
13+
echo "in callee\n";
14+
}
15+
16+
$test = new _ZendTestMagicCallForward;
17+
$test->callee();
18+
echo "done\n";
19+
20+
?>
21+
--EXPECTF--
22+
<!-- init '%sgh16233.php' -->
23+
<file '%sgh16233.php'>
24+
<!-- init _ZendTestMagicCallForward::__call() -->
25+
<_ZendTestMagicCallForward::__call>
26+
<!-- init callee() -->
27+
<callee>
28+
in callee
29+
</callee>
30+
</_ZendTestMagicCallForward::__call>
31+
done
32+
</file '%sgh16233.php'>

0 commit comments

Comments
 (0)