From bab796e7cbb591f1ec7cef2f4336c9702a39afe6 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 30 Sep 2025 00:54:41 +0200 Subject: [PATCH 1/2] 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 --- ext/opcache/ZendAccelerator.c | 9 ++++++++- ext/opcache/tests/gh19984.phpt | 21 +++++++++++++++++++++ ext/opcache/zend_accelerator_util_funcs.c | 10 ---------- 3 files changed, 29 insertions(+), 11 deletions(-) create mode 100644 ext/opcache/tests/gh19984.phpt diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index f0d3379534591..a533ee3f54a84 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 (persistent_script->num_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 0000000000000..4584fa6494c18 --- /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 21f056901fd1b..99523ca72279d 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); From a9a9ec4ff63c3a7f658a456a481bb31440a98e09 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 30 Sep 2025 14:01:42 +0200 Subject: [PATCH 2/2] Improve accuracy of check --- ext/opcache/ZendAccelerator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index a533ee3f54a84..0456017dae055 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -2211,7 +2211,7 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type) /* We may have switched to an existing persistent script that was persisted in * the meantime. Make sure to use its warnings if available. */ - if (persistent_script->num_warnings) { + if (ZCG(accel_directives).record_warnings) { EG(record_errors) = false; zend_emit_recorded_errors_ex(persistent_script->num_warnings, persistent_script->warnings); } else {