Skip to content

Commit 3dab585

Browse files
authored
Fix GH-16811: Crash in zend_test observer on runtime observe_function_names change (GH-21635)
OnUpdateCommaList called zend_observer_remove/add_begin_handler without checking whether observer data was initialized. This null-dereferenced when the function had never been called (no runtime cache), and hit ZEND_UNREACHABLE() when observe_all had already installed the same handler. Guard both the remove and add blocks with runtime cache checks. Remove existing handlers before re-adding to prevent slot overflow from duplicates. Fixes GH-16811
1 parent cbd71cb commit 3dab585

File tree

3 files changed

+42
-2
lines changed

3 files changed

+42
-2
lines changed

ext/zend_test/observer.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,8 @@ static ZEND_INI_MH(zend_test_observer_OnUpdateCommaList)
334334
}
335335
if (stage != PHP_INI_STAGE_STARTUP && stage != PHP_INI_STAGE_ACTIVATE && stage != PHP_INI_STAGE_DEACTIVATE && stage != PHP_INI_STAGE_SHUTDOWN) {
336336
ZEND_HASH_FOREACH_STR_KEY(*p, funcname) {
337-
if ((func = zend_hash_find_ptr(EG(function_table), funcname))) {
337+
if ((func = zend_hash_find_ptr(EG(function_table), funcname))
338+
&& RUN_TIME_CACHE(&func->common)) {
338339
void *old_handler;
339340
zend_observer_remove_begin_handler(func, observer_begin, (zend_observer_fcall_begin_handler *)&old_handler);
340341
zend_observer_remove_end_handler(func, observer_end, (zend_observer_fcall_end_handler *)&old_handler);
@@ -357,7 +358,11 @@ static ZEND_INI_MH(zend_test_observer_OnUpdateCommaList)
357358
zend_string_release(str);
358359
if (stage != PHP_INI_STAGE_STARTUP && stage != PHP_INI_STAGE_ACTIVATE && stage != PHP_INI_STAGE_DEACTIVATE && stage != PHP_INI_STAGE_SHUTDOWN) {
359360
ZEND_HASH_FOREACH_STR_KEY(*p, funcname) {
360-
if ((func = zend_hash_find_ptr(EG(function_table), funcname))) {
361+
if ((func = zend_hash_find_ptr(EG(function_table), funcname))
362+
&& RUN_TIME_CACHE(&func->common) && *ZEND_OBSERVER_DATA(func)) {
363+
void *old_handler;
364+
zend_observer_remove_begin_handler(func, observer_begin, (zend_observer_fcall_begin_handler *)&old_handler);
365+
zend_observer_remove_end_handler(func, observer_end, (zend_observer_fcall_end_handler *)&old_handler);
361366
zend_observer_add_begin_handler(func, observer_begin);
362367
zend_observer_add_end_handler(func, observer_end);
363368
}

ext/zend_test/tests/gh16811.phpt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
GH-16811 (Segmentation fault in zend observer)
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_function_names=a,d
9+
--FILE--
10+
<?php
11+
var_dump(ini_set("zend_test.observer.observe_function_names", "bar"));
12+
function d() {}
13+
?>
14+
--EXPECTF--
15+
<!-- init '%s' -->
16+
<!-- init ini_set() -->
17+
<!-- init var_dump() -->
18+
string(3) "a,d"
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
GH-16811 (Assertion failure adding duplicate observer handler)
3+
--EXTENSIONS--
4+
zend_test
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.observe_all=1
8+
zend_test.observer.show_output=0
9+
--FILE--
10+
<?php
11+
function foo() {}
12+
foo();
13+
ini_set("zend_test.observer.observe_function_names", "foo");
14+
echo "Done\n";
15+
?>
16+
--EXPECT--
17+
Done

0 commit comments

Comments
 (0)