Send a blank email to [email protected] to get a copy of this message
Author: Niels Dossche (nielsdos)
Date: 2025-05-05T19:13:39+02:00
Commit: https://github.com/php/php-src/commit/fb3536fd60d8264a7226db765ea5d8153a8b0864
Raw diff: https://github.com/php/php-src/commit/fb3536fd60d8264a7226db765ea5d8153a8b0864.diff
Fix leak+crash with sapi_windows_set_ctrl_handler()
The ctrl_handler is never destroyed. We have to destroy it at request
end so we avoid leaking it and also avoid keeping a reference to
previous request memory in a next request. The latter can result in a
crash and can be demonstrated with this script and --repeat 2:
```php
class Test {
public function set() {
sapi_windows_set_ctrl_handler(self::cb(...));
}
public function cb() {
}
}
$test = new Test;
$test->set();
sleep(3);
```
When you hit CTRL+C in the second request you can crash.
This patch resolves both the leak and crash by destroying the
ctrl_handler after a request.
Closes GH-18231.
Changed paths:
A sapi/cli/tests/sapi_windows_set_ctrl_handler_leak.phpt
M NEWS
M win32/globals.c
M win32/signal.c
M win32/signal.h
Diff:
diff --git a/NEWS b/NEWS
index ed6cd4e2e3a57..66301a388ce28 100644
--- a/NEWS
+++ b/NEWS
@@ -32,6 +32,9 @@ PHP NEWS
- Standard:
. Fixed bug GH-17403 (Potential deadlock when putenv fails). (nielsdos)
+- Windows:
+ . Fix leak+crash with sapi_windows_set_ctrl_handler(). (nielsdos)
+
- Zip:
. Fixed bug GH-18431 (Registering ZIP progress callback twice doesn't work).
(nielsdos)
diff --git a/sapi/cli/tests/sapi_windows_set_ctrl_handler_leak.phpt
b/sapi/cli/tests/sapi_windows_set_ctrl_handler_leak.phpt
new file mode 100644
index 0000000000000..e91b6c63c344f
--- /dev/null
+++ b/sapi/cli/tests/sapi_windows_set_ctrl_handler_leak.phpt
@@ -0,0 +1,28 @@
+--TEST--
+sapi_windows_set_ctrl_handler() leak bug
+--SKIPIF--
+<?php
+include "skipif.inc";
+
+if (strtoupper(substr(PHP_OS, 0, 3)) !== 'WIN')
+ die("skip this test is for Windows platforms only");
+?>
+--FILE--
+<?php
+
+class Test {
+ public function set() {
+ sapi_windows_set_ctrl_handler(self::cb(...));
+ }
+ public function cb() {
+ }
+}
+
+$test = new Test;
+$test->set();
+
+echo "Done\n";
+
+?>
+--EXPECT--
+Done
diff --git a/win32/globals.c b/win32/globals.c
index b2889c77e9cef..3f35249057328 100644
--- a/win32/globals.c
+++ b/win32/globals.c
@@ -63,5 +63,7 @@ PHP_RSHUTDOWN_FUNCTION(win32_core_globals)
{/*{{{*/
closelog();
+ php_win32_signal_ctrl_handler_request_shutdown();
+
return SUCCESS;
}/*}}}*/
diff --git a/win32/signal.c b/win32/signal.c
index 1207fe1fc092f..21b855f02ea2a 100644
--- a/win32/signal.c
+++ b/win32/signal.c
@@ -68,9 +68,28 @@ PHP_WINUTIL_API void php_win32_signal_ctrl_handler_shutdown(void)
zend_interrupt_function = orig_interrupt_function;
orig_interrupt_function = NULL;
vm_interrupt_flag = NULL;
- ZVAL_UNDEF(&ctrl_handler);
}/*}}}*/
+PHP_WINUTIL_API void php_win32_signal_ctrl_handler_request_shutdown(void)
+{
+ /* Must be initialized and in main thread */
+ if (!vm_interrupt_flag) {
+ return;
+ }
+#ifdef ZTS
+ if (!tsrm_is_main_thread()) {
+ return;
+ }
+#endif
+
+ /* The ctrl_handler must be cleared between requests, otherwise we can crash
+ * due to accessing a previous request's memory. */
+ if (!Z_ISUNDEF(ctrl_handler)) {
+ zval_ptr_dtor(&ctrl_handler);
+ ZVAL_UNDEF(&ctrl_handler);
+ }
+}
+
static BOOL WINAPI php_win32_signal_system_ctrl_handler(DWORD evt)
{/*{{{*/
if (CTRL_C_EVENT != evt && CTRL_BREAK_EVENT != evt) {
@@ -125,7 +144,7 @@ PHP_FUNCTION(sapi_windows_set_ctrl_handler)
RETURN_FALSE;
}
- zval_ptr_dtor_nogc(&ctrl_handler);
+ zval_ptr_dtor(&ctrl_handler);
ZVAL_COPY(&ctrl_handler, &fci.function_name);
RETURN_TRUE;
diff --git a/win32/signal.h b/win32/signal.h
index 2058dd873b075..d7048d96a0a6c 100644
--- a/win32/signal.h
+++ b/win32/signal.h
@@ -10,6 +10,7 @@
#define SIGPROF 27 /* profiling time alarm */
PHP_WINUTIL_API void php_win32_signal_ctrl_handler_init(void);
+PHP_WINUTIL_API void php_win32_signal_ctrl_handler_request_shutdown(void);
PHP_WINUTIL_API void php_win32_signal_ctrl_handler_shutdown(void);
#endif /* PHP_WIN32_SIGNAL_H */