Skip to content

Commit f084460

Browse files
committed
Fix phpGH-23898: Keep user_filter_map alive until streams are closed
BG(user_filter_map) was destroyed in the user_filters RSHUTDOWN, while the matching factories in FG(stream_filters) survive until php_shutdown_stream_hashes(). Streams are only closed later in zend_deactivate(), so a filter() callback running at that point could call stream_filter_register() and stream_filter_append() with the two tables out of sync, recreating a leaked map and hitting ZEND_ASSERT(ce) or a NULL class entry dereference. Free the map in a post-deactivate hook of the basic module instead, which runs after the executor is gone and no PHP callback can run anymore. This also covers phpGH-22449 and phpGH-22818, whose tests now expect the shutdown registration and append to succeed.
1 parent 3913e03 commit f084460

7 files changed

Lines changed: 79 additions & 15 deletions

File tree

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ PHP NEWS
6767

6868
- Opcache:
6969
. Fix zend_analyze_calls() call_stack buffer overrun. (Mrmaxmeier)
70+
71+
- Standard:
72+
. Fixed bug GH-23898 (stream_filter_register() during shutdown leads to
73+
assertion failure or crash in user_filter_factory_create()). (Jakub Zelenka)
7074
. Fixed bug GH-23679 (Tracing JIT writes a parent private property into a
7175
child's shadowing public property). (Ilia Alshanetsky)
7276
. Fix multiple incorrect DCE due to unsound escape analysis. (ndossche,

‎ext/standard/basic_functions.c‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ zend_module_entry basic_functions_module = { /* {{{ */
156156
PHP_MINFO(basic), /* extension info */
157157
PHP_STANDARD_VERSION, /* extension version */
158158
NO_MODULE_GLOBALS,
159-
ZEND_MODULE_POST_ZEND_DEACTIVATE_N(streams),
159+
ZEND_MODULE_POST_ZEND_DEACTIVATE_N(basic),
160160
STANDARD_MODULE_PROPERTIES_EX
161161
};
162162
/* }}} */
@@ -475,7 +475,6 @@ PHP_RSHUTDOWN_FUNCTION(basic) /* {{{ */
475475
BG(user_tick_functions) = NULL;
476476
}
477477

478-
BASIC_RSHUTDOWN_SUBMODULE(user_filters)
479478
BASIC_RSHUTDOWN_SUBMODULE(browscap)
480479

481480
/* Free last http headers */
@@ -487,6 +486,14 @@ PHP_RSHUTDOWN_FUNCTION(basic) /* {{{ */
487486
}
488487
/* }}} */
489488

489+
ZEND_MODULE_POST_ZEND_DEACTIVATE_D(basic) /* {{{ */
490+
{
491+
ZEND_MODULE_POST_ZEND_DEACTIVATE_N(user_filters)();
492+
493+
return ZEND_MODULE_POST_ZEND_DEACTIVATE_N(streams)();
494+
}
495+
/* }}} */
496+
490497
PHP_MINFO_FUNCTION(basic) /* {{{ */
491498
{
492499
php_info_print_table_start();

‎ext/standard/basic_functions.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,14 @@ PHP_MINIT_FUNCTION(basic);
3838
PHP_MSHUTDOWN_FUNCTION(basic);
3939
PHP_RINIT_FUNCTION(basic);
4040
PHP_RSHUTDOWN_FUNCTION(basic);
41+
ZEND_MODULE_POST_ZEND_DEACTIVATE_D(basic);
4142
PHP_MINFO_FUNCTION(basic);
4243

4344
ZEND_API void php_get_highlight_struct(zend_syntax_highlighter_ini *syntax_highlighter_ini);
4445

4546
PHP_MINIT_FUNCTION(poll);
4647
PHP_MINIT_FUNCTION(user_filters);
47-
PHP_RSHUTDOWN_FUNCTION(user_filters);
48+
ZEND_MODULE_POST_ZEND_DEACTIVATE_D(user_filters);
4849
PHP_RSHUTDOWN_FUNCTION(browscap);
4950

5051
PHPAPI zend_result _php_error_log(int opt_err, const zend_string *message, const zend_string *opt, const zend_string *headers);

‎ext/standard/tests/gh22449.phpt‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,17 @@ Bug GH-22449: user_filter_factory_create NULL dereference during shutdown
55

66
class rotate_filter_nw extends php_user_filter
77
{
8+
private static bool $done = false;
9+
810
public function filter($in, $out, &$consumed, $closing): int
911
{
12+
if (self::$done) {
13+
return PSFS_PASS_ON;
14+
}
15+
self::$done = true;
16+
1017
$stream = fopen('php://memory', 'w+');
11-
stream_filter_append($stream, "rotator_notWorking");
18+
var_dump(stream_filter_append($stream, "rotator_notWorking") !== false);
1219

1320
return PSFS_PASS_ON;
1421
}
@@ -21,7 +28,6 @@ stream_filter_append($stream, "rotator_notWorking");
2128

2229
echo "done\n";
2330
?>
24-
--EXPECTF--
31+
--EXPECT--
2532
done
26-
27-
Warning: stream_filter_append(): Unable to create or locate filter "rotator_notWorking" in %s on line %d
33+
bool(true)

‎ext/standard/tests/gh22818.phpt‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,18 @@ Bug GH-22818: user_filter_factory_create assertion failure on shutdown re-regist
55

66
class rotate_filter_nw extends php_user_filter
77
{
8+
private static bool $done = false;
9+
810
public function filter($in, $out, &$consumed, $closing): int
911
{
12+
if (self::$done) {
13+
return PSFS_PASS_ON;
14+
}
15+
self::$done = true;
16+
1017
$stream = fopen('php://memory', 'w+');
11-
stream_filter_register("rotator_notWorking", rotate_filter_nw::class);
12-
stream_filter_append($stream, "rotator_notWorking");
18+
var_dump(stream_filter_register("rotator_notWorking", rotate_filter_nw::class));
19+
var_dump(stream_filter_append($stream, "rotator_notWorking") !== false);
1320

1421
return PSFS_PASS_ON;
1522
}
@@ -22,7 +29,7 @@ stream_filter_append($stream, "rotator_notWorking");
2229

2330
echo "done\n";
2431
?>
25-
--EXPECTF--
32+
--EXPECT--
2633
done
27-
28-
Warning: stream_filter_append(): Unable to create or locate filter "rotator_notWorking" in %s on line %d
34+
bool(false)
35+
bool(true)

‎ext/standard/tests/gh23898.phpt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
Bug GH-23898: user_filter_factory_create assertion failure on shutdown registration of a new filter
3+
--FILE--
4+
<?php
5+
6+
class shutdown_filter extends php_user_filter
7+
{
8+
private static bool $done = false;
9+
10+
public function filter($in, $out, &$consumed, $closing): int
11+
{
12+
if (self::$done) {
13+
return PSFS_PASS_ON;
14+
}
15+
self::$done = true;
16+
17+
$stream = fopen('php://memory', 'w+');
18+
var_dump(stream_filter_register("shutdown_filter_other", shutdown_filter::class));
19+
var_dump(stream_filter_append($stream, "shutdown_filter") !== false);
20+
var_dump(stream_filter_append($stream, "shutdown_filter_other") !== false);
21+
22+
return PSFS_PASS_ON;
23+
}
24+
}
25+
26+
stream_filter_register("shutdown_filter", shutdown_filter::class);
27+
28+
$stream = fopen('php://memory', 'w+');
29+
stream_filter_append($stream, "shutdown_filter");
30+
31+
echo "done\n";
32+
?>
33+
--EXPECT--
34+
done
35+
bool(true)
36+
bool(true)
37+
bool(true)

‎ext/standard/user_filters.c‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ PHP_MINIT_FUNCTION(user_filters)
9595
return SUCCESS;
9696
}
9797

98-
PHP_RSHUTDOWN_FUNCTION(user_filters)
98+
/* Freed post-deactivate rather than in RSHUTDOWN, as streams closed during
99+
* executor shutdown may still create user filters from their callbacks. */
100+
ZEND_MODULE_POST_ZEND_DEACTIVATE_D(user_filters)
99101
{
100102
if (BG(user_filter_map)) {
101103
zend_hash_destroy(BG(user_filter_map));
@@ -605,8 +607,8 @@ PHP_FUNCTION(stream_filter_register)
605607
RETURN_THROWS();
606608
}
607609

608-
/* Register the factory first; if that fails, don't (re)create the map,
609-
* which would leak during shutdown re-registration. */
610+
/* Register the factory first so that a duplicate name is rejected before
611+
* the map is touched; the map and FG(stream_filters) must stay in sync. */
610612
if (php_stream_filter_register_factory_volatile(filtername, &user_filter_factory) == FAILURE) {
611613
RETURN_FALSE;
612614
}

0 commit comments

Comments
 (0)