From 53fa98ecd34b55be8495e8fbeec435884bd36b2e Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Mon, 24 Feb 2025 14:35:47 +0100 Subject: [PATCH] Fix GH-17715: Handle preloaded internal function runtime cache (#17835) This solely affects the builtin enum functions currently. Given that these are stored in SHM, we cannot simply hardwire a pointer into the internal function runtime cache on NTS too, but have to use a MAP_PTR (like on ZTS). Now, by design, the runtime cache of internal functions no longer is reset between requests, hence we need to store them explicitly as static runtime cache. On NTS builds we cannot trivially move the pointers into CG(internal_run_time_cache) as they're directly stored on the individual functions (on ZTS we could simply iterate the static map_ptrs). Hence, we have the choice between having opcache managing the internal run_time_cache for its preloaded functions itself or realloc CG(internal_run_time_cache) and iterate through all functions to assign the new address. We choose the latter for simplicity and initial speed. --- NEWS | 4 +- Zend/zend_enum.c | 5 +- Zend/zend_map_ptr.h | 3 + ext/opcache/ZendAccelerator.c | 73 ++++++++++++++++---- ext/opcache/ZendAccelerator.h | 6 +- ext/opcache/tests/preload_enum_observed.phpt | 55 +++++++++++++++ ext/opcache/zend_accelerator_module.c | 2 +- ext/opcache/zend_persist.c | 7 +- 8 files changed, 135 insertions(+), 20 deletions(-) create mode 100644 ext/opcache/tests/preload_enum_observed.phpt diff --git a/NEWS b/NEWS index b8e830ca10e..69c6e954b66 100644 --- a/NEWS +++ b/NEWS @@ -55,6 +55,8 @@ PHP NEWS . Fixed bug GH-17577 (JIT packed type guard crash). (nielsdos, Dmitry) . Fixed bug GH-17747 (Exception on reading property in register-based FETCH_OBJ_R breaks JIT). (Dmitry, nielsdos) + . Fixed bug GH-17715 (Null pointer deref in observer API when calling + cases() method on preloaded enum). (Bob) - PDO_SQLite: . Fixed GH-17837 ()::getColumnMeta() on unexecuted statement segfaults). @@ -78,7 +80,7 @@ PHP NEWS . Fix memory leak on overflow in _php_stream_scandir(). (nielsdos) - Windows: - . Fixed phpize for Windows 11 (24H2). (bwoebi) + . Fixed phpize for Windows 11 (24H2). (Bob) . Fixed GH-17855 (CURL_STATICLIB flag set even if linked with shared lib). (cmb) diff --git a/Zend/zend_enum.c b/Zend/zend_enum.c index 8259441fd42..ccafca48fe9 100644 --- a/Zend/zend_enum.c +++ b/Zend/zend_enum.c @@ -418,7 +418,10 @@ static void zend_enum_register_func(zend_class_entry *ce, zend_known_string_id n zif->module = EG(current_module); zif->scope = ce; zif->T = ZEND_OBSERVER_ENABLED; - if (EG(active)) { // at run-time + if (EG(active)) { // at run-time + if (CG(compiler_options) & ZEND_COMPILE_PRELOAD) { + zif->fn_flags |= ZEND_ACC_PRELOADED; + } ZEND_MAP_PTR_INIT(zif->run_time_cache, zend_arena_calloc(&CG(arena), 1, zend_internal_run_time_cache_reserved_size())); } else { #ifdef ZTS diff --git a/Zend/zend_map_ptr.h b/Zend/zend_map_ptr.h index ebcda89411d..4dfa0e5043e 100644 --- a/Zend/zend_map_ptr.h +++ b/Zend/zend_map_ptr.h @@ -70,6 +70,9 @@ typedef struct _zend_string zend_string; } while (0) # define ZEND_MAP_PTR_BIASED_BASE(real_base) \ ((void*)(((uintptr_t)(real_base)) + zend_map_ptr_static_size * sizeof(void *) - 1)) +/* Note: chunked like: [8192..12287][4096..8191][0..4095] */ +#define ZEND_MAP_PTR_STATIC_NUM_TO_PTR(num) \ + ((void **)CG(map_ptr_real_base) + zend_map_ptr_static_size - ZEND_MM_ALIGNED_SIZE_EX((num) + 1, 4096) + ((num) & 4095)) #else # error "Unknown ZEND_MAP_PTR_KIND" #endif diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 364440639b6..6e53b987d67 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -2614,6 +2614,7 @@ static void zend_reset_cache_vars(void) ZCSG(restart_pending) = false; ZCSG(force_restart_time) = 0; ZCSG(map_ptr_last) = CG(map_ptr_last); + ZCSG(map_ptr_static_last) = zend_map_ptr_static_last; } static void accel_reset_pcre_cache(void) @@ -2629,7 +2630,7 @@ static void accel_reset_pcre_cache(void) } ZEND_HASH_FOREACH_END(); } -zend_result accel_activate(INIT_FUNC_ARGS) +ZEND_RINIT_FUNCTION(zend_accelerator) { if (!ZCG(enabled) || !accel_startup_ok) { ZCG(accelerator_enabled) = false; @@ -2961,12 +2962,15 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals) GC_MAKE_PERSISTENT_LOCAL(accel_globals->key); } -#ifdef ZTS static void accel_globals_dtor(zend_accel_globals *accel_globals) { +#ifdef ZTS zend_string_free(accel_globals->key); -} #endif + if (accel_globals->preloaded_internal_run_time_cache) { + pefree(accel_globals->preloaded_internal_run_time_cache, 1); + } +} #ifdef HAVE_HUGE_CODE_PAGES # ifndef _WIN32 @@ -3407,6 +3411,8 @@ void accel_shutdown(void) if (!ZCG(enabled) || !accel_startup_ok) { #ifdef ZTS ts_free_id(accel_globals_id); +#else + accel_globals_dtor(&accel_globals); #endif return; } @@ -3421,6 +3427,8 @@ void accel_shutdown(void) #ifdef ZTS ts_free_id(accel_globals_id); +#else + accel_globals_dtor(&accel_globals); #endif if (!_file_cache_only) { @@ -4318,7 +4326,7 @@ static zend_persistent_script* preload_script_in_shared_memory(zend_persistent_s return new_persistent_script; } -static void preload_load(void) +static void preload_load(size_t orig_map_ptr_static_last) { /* Load into process tables */ zend_script *script = &ZCSG(preload_script)->script; @@ -4353,14 +4361,42 @@ static void preload_load(void) if (EG(class_table)) { EG(persistent_classes_count) = EG(class_table)->nNumUsed; } - if (CG(map_ptr_last) != ZCSG(map_ptr_last)) { - size_t old_map_ptr_last = CG(map_ptr_last); + + size_t old_map_ptr_last = CG(map_ptr_last); + if (zend_map_ptr_static_last != ZCSG(map_ptr_static_last) || old_map_ptr_last != ZCSG(map_ptr_last)) { CG(map_ptr_last) = ZCSG(map_ptr_last); - CG(map_ptr_size) = ZEND_MM_ALIGNED_SIZE_EX(CG(map_ptr_last) + 1, 4096); - CG(map_ptr_real_base) = perealloc(CG(map_ptr_real_base), CG(map_ptr_size) * sizeof(void*), 1); + CG(map_ptr_size) = ZEND_MM_ALIGNED_SIZE_EX(ZCSG(map_ptr_last) + 1, 4096); + zend_map_ptr_static_last = ZCSG(map_ptr_static_last); + + /* Grow map_ptr table as needed, but allocate once for static + regular map_ptrs */ + size_t new_static_size = ZEND_MM_ALIGNED_SIZE_EX(zend_map_ptr_static_last, 4096); + if (zend_map_ptr_static_size != new_static_size) { + void *new_base = pemalloc((new_static_size + CG(map_ptr_size)) * sizeof(void *), 1); + if (CG(map_ptr_real_base)) { + memcpy((void **) new_base + new_static_size - zend_map_ptr_static_size, CG(map_ptr_real_base), (old_map_ptr_last + zend_map_ptr_static_size) * sizeof(void *)); + pefree(CG(map_ptr_real_base), 1); + } + CG(map_ptr_real_base) = new_base; + zend_map_ptr_static_size = new_static_size; + } else { + CG(map_ptr_real_base) = perealloc(CG(map_ptr_real_base), (zend_map_ptr_static_size + CG(map_ptr_size)) * sizeof(void *), 1); + } + + memset((void **) CG(map_ptr_real_base) + zend_map_ptr_static_size + old_map_ptr_last, 0, (CG(map_ptr_last) - old_map_ptr_last) * sizeof(void *)); CG(map_ptr_base) = ZEND_MAP_PTR_BIASED_BASE(CG(map_ptr_real_base)); - memset((void **) CG(map_ptr_real_base) + old_map_ptr_last, 0, - (CG(map_ptr_last) - old_map_ptr_last) * sizeof(void *)); + } + + if (orig_map_ptr_static_last != zend_map_ptr_static_last) { + /* preloaded static entries currently are all runtime cache pointers, just assign them as such */ + size_t runtime_cache_size = zend_internal_run_time_cache_reserved_size(); + ZCG(preloaded_internal_run_time_cache_size) = (zend_map_ptr_static_last - orig_map_ptr_static_last) * runtime_cache_size; + char *cache = pemalloc(ZCG(preloaded_internal_run_time_cache_size), 1); + ZCG(preloaded_internal_run_time_cache) = cache; + + for (size_t cur_static_map_ptr = orig_map_ptr_static_last; cur_static_map_ptr < zend_map_ptr_static_last; ++cur_static_map_ptr) { + *ZEND_MAP_PTR_STATIC_NUM_TO_PTR(cur_static_map_ptr) = cache; + cache += runtime_cache_size; + } } } @@ -4369,7 +4405,7 @@ static zend_result accel_preload(const char *config, bool in_child) zend_file_handle file_handle; zend_result ret; char *orig_open_basedir; - size_t orig_map_ptr_last; + size_t orig_map_ptr_last, orig_map_ptr_static_last; uint32_t orig_compiler_options; ZCG(enabled) = false; @@ -4380,6 +4416,7 @@ static zend_result accel_preload(const char *config, bool in_child) accelerator_orig_compile_file = preload_compile_file; orig_map_ptr_last = CG(map_ptr_last); + orig_map_ptr_static_last = zend_map_ptr_static_last; /* Compile and execute preloading script */ zend_stream_init_filename(&file_handle, (char *) config); @@ -4559,7 +4596,7 @@ static zend_result accel_preload(const char *config, bool in_child) SHM_PROTECT(); HANDLE_UNBLOCK_INTERRUPTIONS(); - preload_load(); + preload_load(orig_map_ptr_static_last); /* Store individual scripts with unlinked classes */ HANDLE_BLOCK_INTERRUPTIONS(); @@ -4811,7 +4848,7 @@ static zend_result accel_finish_startup(void) if (ZCSG(preload_script)) { /* Preloading was done in another process */ - preload_load(); + preload_load(zend_map_ptr_static_last); zend_shared_alloc_unlock(); return SUCCESS; } @@ -4839,7 +4876,7 @@ static zend_result accel_finish_startup(void) } if (ZCSG(preload_script)) { - preload_load(); + preload_load(zend_map_ptr_static_last); } zend_shared_alloc_unlock(); @@ -4853,6 +4890,12 @@ static zend_result accel_finish_startup(void) #endif /* ZEND_WIN32 */ } +static void accel_activate(void) { + if (ZCG(preloaded_internal_run_time_cache)) { + memset(ZCG(preloaded_internal_run_time_cache), 0, ZCG(preloaded_internal_run_time_cache_size)); + } +} + ZEND_EXT_API zend_extension zend_extension_entry = { ACCELERATOR_PRODUCT_NAME, /* name */ PHP_VERSION, /* version */ @@ -4861,7 +4904,7 @@ ZEND_EXT_API zend_extension zend_extension_entry = { "Copyright (c)", /* copyright */ accel_startup, /* startup */ NULL, /* shutdown */ - NULL, /* per-script activation */ + accel_activate, /* per-script activation */ #ifdef HAVE_JIT accel_deactivate, /* per-script deactivation */ #else diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index 54d55e10e4f..486074ef001 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -50,6 +50,7 @@ #include "zend_extensions.h" #include "zend_compile.h" +#include "zend_API.h" #include "Optimizer/zend_optimizer.h" #include "zend_accelerator_hash.h" @@ -216,6 +217,8 @@ typedef struct _zend_accel_globals { #ifndef ZEND_WIN32 zend_ulong root_hash; #endif + void *preloaded_internal_run_time_cache; + size_t preloaded_internal_run_time_cache_size; /* preallocated shared-memory block to save current script */ void *mem; zend_persistent_script *current_persistent_script; @@ -251,6 +254,7 @@ typedef struct _zend_accel_shared_globals { zend_accel_hash hash; /* hash table for cached scripts */ size_t map_ptr_last; + size_t map_ptr_static_last; /* Directives & Maintenance */ time_t start_time; @@ -310,7 +314,7 @@ extern const char *zps_api_failure_reason; BEGIN_EXTERN_C() void accel_shutdown(void); -zend_result accel_activate(INIT_FUNC_ARGS); +ZEND_RINIT_FUNCTION(zend_accelerator); zend_result accel_post_deactivate(void); void zend_accel_schedule_restart(zend_accel_restart_reason reason); void zend_accel_schedule_restart_if_necessary(zend_accel_restart_reason reason); diff --git a/ext/opcache/tests/preload_enum_observed.phpt b/ext/opcache/tests/preload_enum_observed.phpt new file mode 100644 index 00000000000..7898fa084a5 --- /dev/null +++ b/ext/opcache/tests/preload_enum_observed.phpt @@ -0,0 +1,55 @@ +--TEST-- +Enum preloading with observers +--EXTENSIONS-- +opcache +zend_test +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=-1 +opcache.preload={PWD}/preload_enum.inc +zend_test.observer.enabled=1 +zend_test.observer.show_output=1 +zend_test.observer.observe_all=1 +--SKIPIF-- + +--FILE-- + +--EXPECTF-- + + + + +enum(MyEnum::Bar) + + + + + + + + + + + + +array(2) { + [0]=> + enum(MyEnum::Foo) + [1]=> + enum(MyEnum::Bar) +} + + diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index a59502eb0d5..ffa09aaf9e6 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -572,7 +572,7 @@ static zend_module_entry accel_module_entry = { ext_functions, ZEND_MINIT(zend_accelerator), ZEND_MSHUTDOWN(zend_accelerator), - accel_activate, + ZEND_RINIT(zend_accelerator), NULL, zend_accel_info, PHP_VERSION, diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index c5ddc040b22..1c21e031a19 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -735,7 +735,11 @@ static zend_op_array *zend_persist_class_method(zend_op_array *op_array, zend_cl // Real dynamically created internal functions like enum methods must have their own run_time_cache pointer. They're always on the same scope as their defining class. // However, copies - as caused by inheritance of internal methods - must retain the original run_time_cache pointer, shared with the source function. if (!op_array->scope || (op_array->scope == ce && !(op_array->fn_flags & ZEND_ACC_TRAIT_CLONE))) { - ZEND_MAP_PTR_NEW(op_array->run_time_cache); + if (op_array->fn_flags & ZEND_ACC_PRELOADED) { + ZEND_MAP_PTR_NEW_STATIC(op_array->run_time_cache); + } else { + ZEND_MAP_PTR_NEW(op_array->run_time_cache); + } } } } @@ -1413,6 +1417,7 @@ zend_persistent_script *zend_accel_script_persist(zend_persistent_script *script if (for_shm) { ZCSG(map_ptr_last) = CG(map_ptr_last); + ZCSG(map_ptr_static_last) = zend_map_ptr_static_last; } #ifdef HAVE_JIT