From bcd4be7d50b3632e6191b880bd9ab458cc615b08 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 30 Sep 2025 00:54:41 +0200 Subject: [PATCH] Fix double-free of EG(errors)/persistent_script->warnings on persist of already persisted file Both processes race to compile warning_replay.inc. Whichever is first will get to persist the script. The loser will use the script that is already persisted, and the script that was just compiled is freed. However, EG(errors) and persistent_script->warnings still refer to the same allocation, and EG(errors) becomes a dangling pointer. To solve this, we simply don't free warnings from free_persistent_script() anymore to maintain exclusive ownership for EG(errors). Furthermore, we need to adjust a call to zend_emit_recorded_errors() that would previously use EG(errors), even when persistent_script has been swapped out. Fixes GH-19984 Closes GH-19995 --- NEWS | 2 ++ ext/opcache/ZendAccelerator.c | 9 ++++++++- ext/opcache/tests/gh19984.phpt | 21 +++++++++++++++++++++ ext/opcache/zend_accelerator_util_funcs.c | 10 ---------- 4 files changed, 31 insertions(+), 11 deletions(-) create mode 100644 ext/opcache/tests/gh19984.phpt diff --git a/NEWS b/NEWS index 740755ecd40..575fcdbbad9 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,8 @@ PHP NEWS - Opcache: . Fixed segfault in function JIT due to NAN to bool warning. (Girgias) + . Fixed bug GH-19984 (Double-free of EG(errors)/persistent_script->warnings on + persist of already persisted file). (ilutov, Arnaud) - SOAP: . Fixed bug GH-19773 (SIGSEGV due to uninitialized soap_globals->lang_en). diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index f0d33795345..0456017dae0 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -2209,7 +2209,14 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) SHM_PROTECT(); HANDLE_UNBLOCK_INTERRUPTIONS(); - zend_emit_recorded_errors(); + /* We may have switched to an existing persistent script that was persisted in + * the meantime. Make sure to use its warnings if available. */ + if (ZCG(accel_directives).record_warnings) { + EG(record_errors) = false; + zend_emit_recorded_errors_ex(persistent_script->num_warnings, persistent_script->warnings); + } else { + zend_emit_recorded_errors(); + } zend_free_recorded_errors(); } else { diff --git a/ext/opcache/tests/gh19984.phpt b/ext/opcache/tests/gh19984.phpt new file mode 100644 index 00000000000..4584fa6494c --- /dev/null +++ b/ext/opcache/tests/gh19984.phpt @@ -0,0 +1,21 @@ +--TEST-- +GH-19984: Double-free of EG(errors)/persistent_script->warnings on persist of already persisted file +--EXTENSIONS-- +opcache +pcntl +--INI-- +opcache.enable_cli=1 +opcache.record_warnings=1 +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: Unsupported declare 'unknown' in %s on line %d + +Warning: Unsupported declare 'unknown' in %s on line %d diff --git a/ext/opcache/zend_accelerator_util_funcs.c b/ext/opcache/zend_accelerator_util_funcs.c index 21f056901fd..99523ca7227 100644 --- a/ext/opcache/zend_accelerator_util_funcs.c +++ b/ext/opcache/zend_accelerator_util_funcs.c @@ -65,16 +65,6 @@ void free_persistent_script(zend_persistent_script *persistent_script, int destr zend_string_release_ex(persistent_script->script.filename, 0); } - if (persistent_script->warnings) { - for (uint32_t i = 0; i < persistent_script->num_warnings; i++) { - zend_error_info *info = persistent_script->warnings[i]; - zend_string_release(info->filename); - zend_string_release(info->message); - efree(info); - } - efree(persistent_script->warnings); - } - zend_accel_free_delayed_early_binding_list(persistent_script); efree(persistent_script);