From 40f4c4c9676b7aed1a390b96abf0f03b91a040a1 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Wed, 28 Feb 2024 14:56:42 +0100 Subject: [PATCH 1/4] Introduce new zend_module_entry hook: child_startup_func Starting threads in `request_startup_func` is unsafe because SAPIs may fork after this hook. Yet, some modules may start threads as part of their initialisation and have no easy way to safely do so. For example, the `curl` module calls `curl_global_init()` in `request_startup_func` to initialize `libcurl`, but this function may start threads at least on MacOS. This change introduces a new hook: `child_startup_func`. This hook is executed once per request-handling process, which are assumed to be non-forking processes. Modules should use this hook for their thread-creating initialization. --- UPGRADING.INTERNALS | 10 +++ Zend/zend_API.c | 117 +++++++++++++++++++++++++++++ Zend/zend_API.h | 7 ++ Zend/zend_modules.h | 8 +- ext/curl/interface.c | 16 +++- main/SAPI.h | 7 ++ main/main.c | 24 ++++++ main/php.h | 30 +++++--- main/php_main.h | 2 + sapi/apache2handler/sapi_apache2.c | 8 ++ sapi/cgi/cgi_main.c | 10 ++- sapi/cli/php_cli.c | 11 ++- sapi/cli/php_cli_server.c | 9 ++- sapi/embed/php_embed.c | 11 ++- sapi/fpm/fpm/fpm_children.c | 3 + sapi/fpm/fpm/fpm_main.c | 13 ++++ sapi/fuzzer/fuzzer-sapi.c | 10 ++- sapi/phpdbg/phpdbg.c | 11 ++- 18 files changed, 285 insertions(+), 22 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 7463182e083d8..6b2ff771f8851 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -40,6 +40,12 @@ PHP 8.4 INTERNALS UPGRADE NOTES * The inet_aton() and win32/inet.h on Windows have been removed. Use Windows native inet_pton() from ws2tcpip.h. +* It is recommended that extensions initializing external libraries or starting + threads during module startup (MINIT) do so in the child startup hook (CHINIT) + instead. A child startup hook can be declared by using the + PHP_MODULE_SET_CHINIT_FUNC() macro during module startup. See + ext/curl/interface.c for an example. + ======================== 2. Build system changes ======================== @@ -147,3 +153,7 @@ PHP 8.4 INTERNALS UPGRADE NOTES ======================== 5. SAPI changes ======================== + +* SAPIs must call php_module_child_startup() once per request-handling process, + any time before handling the first request of the process, and must not fork + (without exec()) after that. diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 2a16f24250ebe..f9ba92ba8c730 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -34,6 +34,16 @@ #include "zend_observer.h" #include +#if ZEND_DEBUG +# if __linux__ +# include +# include +# elif __APPLE__ +# include +# include +# include +# endif /* __APPLE__ */ +#endif /* these variables are true statics/globals, and have to be mutex'ed on every access */ ZEND_API HashTable module_registry; @@ -2292,6 +2302,64 @@ ZEND_API void add_property_zval_ex(zval *arg, const char *key, size_t key_len, z } /* }}} */ +#if ZEND_DEBUG +# if __linux__ +static int zend_thread_count(void) +{ + pid_t pid = getpid(); + char path[64]; + int len = snprintf(path, sizeof(path), "/proc/%d/task", (int)pid); + if (len == -1 || len >= sizeof(path)) { + return -1; + } + + DIR *d = opendir(path); + if (d == NULL) { + return -1; + } + + struct dirent *entry; + int nthreads = 0; + while ((entry = readdir(d))){ + nthreads++; + } + + closedir(d); + + return nthreads; +} +# elif __APPLE__ +static int zend_thread_count(void) +{ + pid_t pid = getpid(); + mach_port_t me = mach_task_self(); + mach_port_t task; + kern_return_t res; + thread_array_t threads; + mach_msg_type_number_t nthreads; + + res = task_for_pid(me, pid, &task); + if (res != KERN_SUCCESS) { + return -1; + } + + res = task_threads(task, &threads, &nthreads); + if (res != KERN_SUCCESS) { + return -1; + } + + vm_deallocate(me, (vm_address_t)threads, nthreads * sizeof(*threads)); + + return (int) nthreads; +} +# else +static int zend_thread_count(void) +{ + return 0; +} +# endif /* __APPLE__ */ +#endif + ZEND_API zend_result zend_startup_module_ex(zend_module_entry *module) /* {{{ */ { size_t name_len; @@ -2338,6 +2406,9 @@ ZEND_API zend_result zend_startup_module_ex(zend_module_entry *module) /* {{{ */ #endif } if (module->module_startup_func) { +#if ZEND_DEBUG + int thread_count = zend_thread_count(); +#endif EG(current_module) = module; if (module->module_startup_func(module->type, module->module_number)==FAILURE) { zend_error_noreturn(E_CORE_ERROR,"Unable to start %s module", module->name); @@ -2345,7 +2416,19 @@ ZEND_API zend_result zend_startup_module_ex(zend_module_entry *module) /* {{{ */ return FAILURE; } EG(current_module) = NULL; +#if ZEND_DEBUG + if (zend_thread_count() > thread_count) { + zend_error_noreturn(E_CORE_ERROR, + "Thread count increased during the initialization of %s" + " module. This is unsafe as the process may be forked after" + " initialization. Modules whishing to start threads should" + " do so in child_startup_func.", + module->name); + abort(); + } +#endif } + return SUCCESS; } /* }}} */ @@ -2491,6 +2574,40 @@ ZEND_API void zend_destroy_modules(void) /* {{{ */ } /* }}} */ +ZEND_API void zend_child_startup_modules(void) /* {{{ */ +{ + zend_module_entry *module; + + ZEND_HASH_MAP_FOREACH_PTR(&module_registry, module) { + if (module->child_startup_func) { + if (module->child_startup_func(module->type, module->module_number)==FAILURE) { + zend_error(E_CORE_ERROR, "child_startup() for %s module failed", module->name); + exit(1); + } + } + } ZEND_HASH_FOREACH_END(); +} +/* }}} */ + +ZEND_API void zend_module_set_child_startup_func(INIT_FUNC_ARGS, zend_result (*child_startup_func)(INIT_FUNC_ARGS)) /* {{{ */ +{ + if (!EG(current_module)) { + zend_error(E_CORE_ERROR, "Can not set child startup func after startup"); + abort(); + } + + if (EG(current_module)->module_number != module_number) { + zend_error(E_CORE_ERROR, + "Invalid module_number passed to zend_set_child_startup_func" + " during statup of %s module", + EG(current_module)->name); + abort(); + } + + EG(current_module)->child_startup_func = child_startup_func; +} +/* }}} */ + ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module, int module_type) /* {{{ */ { size_t name_len; diff --git a/Zend/zend_API.h b/Zend/zend_API.h index aa085c70d9bb1..557ddab878a57 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -221,6 +221,7 @@ typedef struct _zend_fcall_info_cache { /* Name macros */ #define ZEND_MODULE_STARTUP_N(module) zm_startup_##module #define ZEND_MODULE_SHUTDOWN_N(module) zm_shutdown_##module +#define ZEND_MODULE_CHILD_STARTUP_N(module) zm_child_startup_##module #define ZEND_MODULE_ACTIVATE_N(module) zm_activate_##module #define ZEND_MODULE_DEACTIVATE_N(module) zm_deactivate_##module #define ZEND_MODULE_POST_ZEND_DEACTIVATE_N(module) zm_post_zend_deactivate_##module @@ -231,6 +232,7 @@ typedef struct _zend_fcall_info_cache { /* Declaration macros */ #define ZEND_MODULE_STARTUP_D(module) zend_result ZEND_MODULE_STARTUP_N(module)(INIT_FUNC_ARGS) #define ZEND_MODULE_SHUTDOWN_D(module) zend_result ZEND_MODULE_SHUTDOWN_N(module)(SHUTDOWN_FUNC_ARGS) +#define ZEND_MODULE_CHILD_STARTUP_D(module) zend_result ZEND_MODULE_CHILD_STARTUP_N(module)(INIT_FUNC_ARGS) #define ZEND_MODULE_ACTIVATE_D(module) zend_result ZEND_MODULE_ACTIVATE_N(module)(INIT_FUNC_ARGS) #define ZEND_MODULE_DEACTIVATE_D(module) zend_result ZEND_MODULE_DEACTIVATE_N(module)(SHUTDOWN_FUNC_ARGS) #define ZEND_MODULE_POST_ZEND_DEACTIVATE_D(module) zend_result ZEND_MODULE_POST_ZEND_DEACTIVATE_N(module)(void) @@ -243,6 +245,9 @@ typedef struct _zend_fcall_info_cache { ZEND_DLEXPORT zend_module_entry *get_module(void) { return &name##_module_entry; }\ END_EXTERN_C() +#define ZEND_MODULE_SET_CHILD_STARTUP_FUNC(func) \ + zend_module_set_child_startup_func(INIT_FUNC_ARGS_PASSTHRU, func) + #define ZEND_BEGIN_MODULE_GLOBALS(module_name) \ typedef struct _zend_##module_name##_globals { #define ZEND_END_MODULE_GLOBALS(module_name) \ @@ -384,6 +389,8 @@ ZEND_API zend_result zend_startup_module_ex(zend_module_entry *module); ZEND_API void zend_startup_modules(void); ZEND_API void zend_collect_module_handlers(void); ZEND_API void zend_destroy_modules(void); +ZEND_API void zend_child_startup_modules(void); +ZEND_API void zend_module_set_child_startup_func(INIT_FUNC_ARGS, zend_result (*child_startup_func)(INIT_FUNC_ARGS)); ZEND_API void zend_check_magic_method_implementation( const zend_class_entry *ce, const zend_function *fptr, zend_string *lcname, int error_type); ZEND_API void zend_add_magic_method(zend_class_entry *ce, zend_function *fptr, zend_string *lcname); diff --git a/Zend/zend_modules.h b/Zend/zend_modules.h index f3f6ca826817d..ef8101df40b58 100644 --- a/Zend/zend_modules.h +++ b/Zend/zend_modules.h @@ -46,7 +46,7 @@ #define ZEND_MODULE_BUILD_ID "API" ZEND_TOSTR(ZEND_MODULE_API_NO) ZEND_BUILD_TS ZEND_BUILD_DEBUG ZEND_BUILD_SYSTEM ZEND_BUILD_EXTRA -#define STANDARD_MODULE_PROPERTIES_EX 0, 0, NULL, 0, ZEND_MODULE_BUILD_ID +#define STANDARD_MODULE_PROPERTIES_EX NULL, 0, 0, NULL, 0, ZEND_MODULE_BUILD_ID #define NO_MODULE_GLOBALS 0, NULL, NULL, NULL @@ -77,8 +77,12 @@ struct _zend_module_entry { const struct _zend_module_dep *deps; const char *name; const struct _zend_function_entry *functions; + /* Called once per process, excluding forked processes. This function + * must not start threads. Calling external libraries from this function is + * not recommended. */ zend_result (*module_startup_func)(INIT_FUNC_ARGS); zend_result (*module_shutdown_func)(SHUTDOWN_FUNC_ARGS); + /* Called once per request */ zend_result (*request_startup_func)(INIT_FUNC_ARGS); zend_result (*request_shutdown_func)(SHUTDOWN_FUNC_ARGS); void (*info_func)(ZEND_MODULE_INFO_FUNC_ARGS); @@ -92,6 +96,8 @@ struct _zend_module_entry { void (*globals_ctor)(void *global); void (*globals_dtor)(void *global); zend_result (*post_deactivate_func)(void); + /* Called once per request-handling process */ + zend_result (*child_startup_func)(INIT_FUNC_ARGS); int module_started; unsigned char type; void *handle; diff --git a/ext/curl/interface.c b/ext/curl/interface.c index 60481947dddea..be9c9c0a2757d 100644 --- a/ext/curl/interface.c +++ b/ext/curl/interface.c @@ -365,10 +365,22 @@ PHP_MINFO_FUNCTION(curl) } /* }}} */ +/* {{{ PHP_CHINIT_FUNCTION */ +PHP_CHINIT_FUNCTION(curl) +{ + if (curl_global_init(CURL_GLOBAL_DEFAULT) != CURLE_OK) { + return FAILURE; + } + + return SUCCESS; +} +/* }}} */ + /* {{{ PHP_MINIT_FUNCTION */ PHP_MINIT_FUNCTION(curl) { REGISTER_INI_ENTRIES(); + PHP_MODULE_SET_CHINIT_FUNC(PHP_CHINIT(curl)); register_curl_symbols(module_number); @@ -390,10 +402,6 @@ PHP_MINIT_FUNCTION(curl) } #endif - if (curl_global_init(CURL_GLOBAL_DEFAULT) != CURLE_OK) { - return FAILURE; - } - curl_ce = register_class_CurlHandle(); curl_ce->create_object = curl_create_object; curl_ce->default_object_handlers = &curl_object_handlers; diff --git a/main/SAPI.h b/main/SAPI.h index 284f4cb96f1fa..728f45ec39354 100644 --- a/main/SAPI.h +++ b/main/SAPI.h @@ -238,9 +238,16 @@ struct _sapi_module_struct { char *name; char *pretty_name; + /* Must be called by the SAPI once per process, excluding forked processes + */ int (*startup)(struct _sapi_module_struct *sapi_module); int (*shutdown)(struct _sapi_module_struct *sapi_module); + /* Must be called by the SAPI once per request-handling process. The process + * must not fork (without exec) after that. */ + int (*child_startup)(struct _sapi_module_struct *sapi_module); + + /* Must be called by the SAPI once per request */ int (*activate)(void); int (*deactivate)(void); diff --git a/main/main.c b/main/main.c index 29d8642b8f713..ac0a9382f63f2 100644 --- a/main/main.c +++ b/main/main.c @@ -96,6 +96,10 @@ PHPAPI size_t core_globals_offset; #define SAFE_FILENAME(f) ((f)?(f):"-") +#if ZEND_DEBUG +static pid_t php_child_started = 0; +#endif + PHPAPI const char *php_version(void) { return PHP_VERSION; @@ -1736,6 +1740,11 @@ static void sigchld_handler(int apar) /* {{{ php_request_startup */ zend_result php_request_startup(void) { +#if ZEND_DEBUG + ZEND_ASSERT(php_child_started == getpid() + && "SAPI must call php_module_child_startup() once per request-handling process"); +#endif + zend_result retval = SUCCESS; zend_interned_strings_activate(); @@ -2431,6 +2440,21 @@ void php_module_shutdown(void) } /* }}} */ +/* {{{ php_module_child_startup */ +zend_result php_module_child_startup(sapi_module_struct *sf) +{ +#if ZEND_DEBUG + ZEND_ASSERT(php_child_started == 0 + && "php_module_child_startup() already called for this process or a parent process"); + php_child_started = getpid(); +#endif + + zend_child_startup_modules(); + + return SUCCESS; +} +/* }}} */ + PHPAPI bool php_execute_script_ex(zend_file_handle *primary_file, zval *retval) { zend_file_handle *prepend_file_p = NULL, *append_file_p = NULL; diff --git a/main/php.h b/main/php.h index b10ded010d129..40372ee68963c 100644 --- a/main/php.h +++ b/main/php.h @@ -382,21 +382,26 @@ END_EXTERN_C() #define PHP_ME_MAPPING ZEND_ME_MAPPING #define PHP_FE_END ZEND_FE_END -#define PHP_MODULE_STARTUP_N ZEND_MODULE_STARTUP_N -#define PHP_MODULE_SHUTDOWN_N ZEND_MODULE_SHUTDOWN_N -#define PHP_MODULE_ACTIVATE_N ZEND_MODULE_ACTIVATE_N -#define PHP_MODULE_DEACTIVATE_N ZEND_MODULE_DEACTIVATE_N -#define PHP_MODULE_INFO_N ZEND_MODULE_INFO_N - -#define PHP_MODULE_STARTUP_D ZEND_MODULE_STARTUP_D -#define PHP_MODULE_SHUTDOWN_D ZEND_MODULE_SHUTDOWN_D -#define PHP_MODULE_ACTIVATE_D ZEND_MODULE_ACTIVATE_D -#define PHP_MODULE_DEACTIVATE_D ZEND_MODULE_DEACTIVATE_D -#define PHP_MODULE_INFO_D ZEND_MODULE_INFO_D +#define PHP_MODULE_STARTUP_N ZEND_MODULE_STARTUP_N +#define PHP_MODULE_SHUTDOWN_N ZEND_MODULE_SHUTDOWN_N +#define PHP_MODULE_CHILD_STARTUP_N ZEND_MODULE_CHILD_STARTUP_N +#define PHP_MODULE_ACTIVATE_N ZEND_MODULE_ACTIVATE_N +#define PHP_MODULE_DEACTIVATE_N ZEND_MODULE_DEACTIVATE_N +#define PHP_MODULE_INFO_N ZEND_MODULE_INFO_N + +#define PHP_MODULE_STARTUP_D ZEND_MODULE_STARTUP_D +#define PHP_MODULE_SHUTDOWN_D ZEND_MODULE_SHUTDOWN_D +#define PHP_MODULE_CHILD_STARTUP_D ZEND_MODULE_CHILD_STARTUP_D +#define PHP_MODULE_ACTIVATE_D ZEND_MODULE_ACTIVATE_D +#define PHP_MODULE_DEACTIVATE_D ZEND_MODULE_DEACTIVATE_D +#define PHP_MODULE_INFO_D ZEND_MODULE_INFO_D + +#define PHP_MODULE_SET_CHILD_STARTUP_FUNC ZEND_MODULE_SET_CHILD_STARTUP_FUNC /* Compatibility macros */ #define PHP_MINIT ZEND_MODULE_STARTUP_N #define PHP_MSHUTDOWN ZEND_MODULE_SHUTDOWN_N +#define PHP_CHINIT ZEND_MODULE_CHILD_STARTUP_N #define PHP_RINIT ZEND_MODULE_ACTIVATE_N #define PHP_RSHUTDOWN ZEND_MODULE_DEACTIVATE_N #define PHP_MINFO ZEND_MODULE_INFO_N @@ -405,12 +410,15 @@ END_EXTERN_C() #define PHP_MINIT_FUNCTION ZEND_MODULE_STARTUP_D #define PHP_MSHUTDOWN_FUNCTION ZEND_MODULE_SHUTDOWN_D +#define PHP_CHINIT_FUNCTION ZEND_MODULE_CHILD_STARTUP_D #define PHP_RINIT_FUNCTION ZEND_MODULE_ACTIVATE_D #define PHP_RSHUTDOWN_FUNCTION ZEND_MODULE_DEACTIVATE_D #define PHP_MINFO_FUNCTION ZEND_MODULE_INFO_D #define PHP_GINIT_FUNCTION ZEND_GINIT_FUNCTION #define PHP_GSHUTDOWN_FUNCTION ZEND_GSHUTDOWN_FUNCTION +#define PHP_MODULE_SET_CHINIT_FUNC PHP_MODULE_SET_CHILD_STARTUP_FUNC + #define PHP_MODULE_GLOBALS ZEND_MODULE_GLOBALS diff --git a/main/php_main.h b/main/php_main.h index 1fb460f07f267..a8a75c12a6891 100644 --- a/main/php_main.h +++ b/main/php_main.h @@ -21,6 +21,7 @@ #include "zend_globals.h" #include "php_globals.h" #include "SAPI.h" +#include "zend_modules.h" BEGIN_EXTERN_C() @@ -40,6 +41,7 @@ PHPAPI zend_result php_request_startup(void); PHPAPI void php_request_shutdown(void *dummy); PHPAPI zend_result php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_module); PHPAPI void php_module_shutdown(void); +PHPAPI zend_result php_module_child_startup(sapi_module_struct *sf); PHPAPI int php_module_shutdown_wrapper(sapi_module_struct *sapi_globals); PHPAPI zend_result php_register_extensions(zend_module_entry * const * ptr, int count); diff --git a/sapi/apache2handler/sapi_apache2.c b/sapi/apache2handler/sapi_apache2.c index a5d4e8984a5a8..e0ff3e246251d 100644 --- a/sapi/apache2handler/sapi_apache2.c +++ b/sapi/apache2handler/sapi_apache2.c @@ -386,6 +386,11 @@ static int php_apache2_startup(sapi_module_struct *sapi_module) return php_module_startup(sapi_module, &php_apache_module); } +static int php_apache2_child_startup(sapi_module_struct *sapi_module) +{ + return php_module_child_startup(sapi_module); +} + static sapi_module_struct apache2_sapi_module = { "apache2handler", "Apache 2.0 Handler", @@ -393,6 +398,8 @@ static sapi_module_struct apache2_sapi_module = { php_apache2_startup, /* startup */ php_module_shutdown_wrapper, /* shutdown */ + php_apache2_child_startup, /* child_startup */ + NULL, /* activate */ NULL, /* deactivate */ @@ -755,6 +762,7 @@ zend_first_try { static void php_apache_child_init(apr_pool_t *pchild, server_rec *s) { apr_pool_cleanup_register(pchild, NULL, php_apache_child_shutdown, apr_pool_cleanup_null); + apache2_sapi_module.child_startup(&apache2_sapi_module); } #ifdef ZEND_SIGNALS diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c index 9d7e66ce2b5bc..6475439c7bec2 100644 --- a/sapi/cgi/cgi_main.c +++ b/sapi/cgi/cgi_main.c @@ -970,6 +970,11 @@ static int php_cgi_startup(sapi_module_struct *sapi_module) return php_module_startup(sapi_module, &cgi_module_entry); } +static int php_cgi_child_startup(sapi_module_struct *sapi_module) +{ + return php_module_child_startup(sapi_module); +} + /* {{{ sapi_module_struct cgi_sapi_module */ static sapi_module_struct cgi_sapi_module = { "cgi-fcgi", /* name */ @@ -978,6 +983,8 @@ static sapi_module_struct cgi_sapi_module = { php_cgi_startup, /* startup */ php_module_shutdown_wrapper, /* shutdown */ + php_cgi_child_startup, /* child_startup */ + sapi_cgi_activate, /* activate */ sapi_cgi_deactivate, /* deactivate */ @@ -1864,7 +1871,8 @@ int main(int argc, char *argv[]) } /* startup after we get the above ini override se we get things right */ - if (cgi_sapi_module.startup(&cgi_sapi_module) == FAILURE) { + if (cgi_sapi_module.startup(&cgi_sapi_module) == FAILURE + || cgi_sapi_module.child_startup(&cgi_sapi_module) == FAILURE) { #ifdef ZTS tsrm_shutdown(); #endif diff --git a/sapi/cli/php_cli.c b/sapi/cli/php_cli.c index e107667828caa..364ffc4dbb325 100644 --- a/sapi/cli/php_cli.c +++ b/sapi/cli/php_cli.c @@ -411,6 +411,12 @@ static int php_cli_startup(sapi_module_struct *sapi_module) /* {{{ */ } /* }}} */ +static int php_cli_child_startup(sapi_module_struct *sapi_module) /* {{{ */ +{ + return php_module_child_startup(sapi_module); +} +/* }}} */ + /* {{{ sapi_cli_ini_defaults */ /* overwritable ini defaults must be set in sapi_cli_ini_defaults() */ @@ -433,6 +439,8 @@ static sapi_module_struct cli_sapi_module = { php_cli_startup, /* startup */ php_module_shutdown_wrapper, /* shutdown */ + php_cli_child_startup, /* child_startup */ + NULL, /* activate */ sapi_cli_deactivate, /* deactivate */ @@ -1304,7 +1312,8 @@ int main(int argc, char *argv[]) sapi_module->ini_entries = php_ini_builder_finish(&ini_builder); /* startup after we get the above ini override se we get things right */ - if (sapi_module->startup(sapi_module) == FAILURE) { + if (sapi_module->startup(sapi_module) == FAILURE + || sapi_module->child_startup(sapi_module) == FAILURE) { /* there is no way to see if we must call zend_ini_deactivate() * since we cannot check if EG(ini_directives) has been initialized * because the executor's constructor does not set initialize it. diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index e84defb3da469..bf670c217b4af 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -515,6 +515,11 @@ static int sapi_cli_server_startup(sapi_module_struct *sapi_module) /* {{{ */ return php_module_startup(sapi_module, &cli_server_module_entry); } /* }}} */ +static int sapi_cli_server_child_startup(sapi_module_struct *sapi_module) /* {{{ */ +{ + return php_module_child_startup(sapi_module); +} /* }}} */ + static size_t sapi_cli_server_ub_write(const char *str, size_t str_length) /* {{{ */ { php_cli_server_client *client = SG(server_context); @@ -804,9 +809,11 @@ sapi_module_struct cli_server_sapi_module = { "cli-server", /* name */ "Built-in HTTP server", /* pretty name */ - sapi_cli_server_startup, /* startup */ + sapi_cli_server_startup, /* startup */ php_module_shutdown_wrapper, /* shutdown */ + sapi_cli_server_child_startup, /* child_startup */ + NULL, /* activate */ NULL, /* deactivate */ diff --git a/sapi/embed/php_embed.c b/sapi/embed/php_embed.c index 4626451f9f68a..db945e922729d 100644 --- a/sapi/embed/php_embed.c +++ b/sapi/embed/php_embed.c @@ -122,6 +122,12 @@ static int php_embed_startup(sapi_module_struct *sapi_module) return php_module_startup(sapi_module, NULL); } +/* Request-handling process initialization (CHINIT) */ +static int php_embed_child_startup(sapi_module_struct *sapi_module) +{ + return php_module_child_startup(sapi_module); +} + EMBED_SAPI_API sapi_module_struct php_embed_module = { "embed", /* name */ "PHP Embedded Library", /* pretty name */ @@ -129,6 +135,8 @@ EMBED_SAPI_API sapi_module_struct php_embed_module = { php_embed_startup, /* startup */ php_module_shutdown_wrapper, /* shutdown */ + php_embed_child_startup, /* child_startup */ + NULL, /* activate */ php_embed_deactivate, /* deactivate */ @@ -224,7 +232,8 @@ EMBED_SAPI_API int php_embed_init(int argc, char **argv) } /* Module initialization (MINIT) */ - if (php_embed_module.startup(&php_embed_module) == FAILURE) { + if (php_embed_module.startup(&php_embed_module) == FAILURE + || php_embed_module.child_startup(&php_embed_module) == FAILURE) { return FAILURE; } diff --git a/sapi/fpm/fpm/fpm_children.c b/sapi/fpm/fpm/fpm_children.c index 285df91a9b690..c90899bcf53d6 100644 --- a/sapi/fpm/fpm/fpm_children.c +++ b/sapi/fpm/fpm/fpm_children.c @@ -9,6 +9,7 @@ #include #include +#include "SAPI.h" #include "fpm.h" #include "fpm_children.h" #include "fpm_signals.h" @@ -202,6 +203,8 @@ static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */ zlog(ZLOG_ERROR, "[pool %s] child failed to initialize", wp->config->name); exit(FPM_EXIT_SOFTWARE); } + + sapi_module.child_startup(&sapi_module); } /* }}} */ diff --git a/sapi/fpm/fpm/fpm_main.c b/sapi/fpm/fpm/fpm_main.c index 9526a04c4187c..c99cb062be5cd 100644 --- a/sapi/fpm/fpm/fpm_main.c +++ b/sapi/fpm/fpm/fpm_main.c @@ -793,6 +793,12 @@ static int php_cgi_startup(sapi_module_struct *sapi_module) /* {{{ */ } /* }}} */ +static int php_cgi_child_startup(sapi_module_struct *sapi_module) /* {{{ */ +{ + return php_module_child_startup(sapi_module); +} +/* }}} */ + /* {{{ sapi_module_struct cgi_sapi_module */ static sapi_module_struct cgi_sapi_module = { "fpm-fcgi", /* name */ @@ -801,6 +807,8 @@ static sapi_module_struct cgi_sapi_module = { php_cgi_startup, /* startup */ php_module_shutdown_wrapper, /* shutdown */ + php_cgi_child_startup, /* child_startup */ + sapi_cgi_activate, /* activate */ sapi_cgi_deactivate, /* deactivate */ @@ -1651,6 +1659,7 @@ int main(int argc, char *argv[]) case 'm': /* list compiled in modules */ cgi_sapi_module.startup(&cgi_sapi_module); + cgi_sapi_module.child_startup(&cgi_sapi_module); php_output_activate(); SG(headers_sent) = 1; php_printf("[PHP Modules]\n"); @@ -1689,6 +1698,7 @@ int main(int argc, char *argv[]) case '?': case PHP_GETOPT_INVALID_ARG: cgi_sapi_module.startup(&cgi_sapi_module); + cgi_sapi_module.child_startup(&cgi_sapi_module); php_output_activate(); SG(headers_sent) = 1; php_cgi_usage(argv[0]); @@ -1700,6 +1710,7 @@ int main(int argc, char *argv[]) case 'v': /* show php version & quit */ cgi_sapi_module.startup(&cgi_sapi_module); + cgi_sapi_module.child_startup(&cgi_sapi_module); if (php_request_startup() == FAILURE) { SG(server_context) = NULL; php_module_shutdown(); @@ -1725,6 +1736,7 @@ int main(int argc, char *argv[]) if (php_information) { cgi_sapi_module.phpinfo_as_text = 1; cgi_sapi_module.startup(&cgi_sapi_module); + cgi_sapi_module.child_startup(&cgi_sapi_module); if (php_request_startup() == FAILURE) { SG(server_context) = NULL; php_module_shutdown(); @@ -1742,6 +1754,7 @@ int main(int argc, char *argv[]) /* No other args are permitted here as there is no interactive mode */ if (argc != php_optind) { cgi_sapi_module.startup(&cgi_sapi_module); + cgi_sapi_module.child_startup(&cgi_sapi_module); php_output_activate(); SG(headers_sent) = 1; php_cgi_usage(argv[0]); diff --git a/sapi/fuzzer/fuzzer-sapi.c b/sapi/fuzzer/fuzzer-sapi.c index a018ce23de307..091c417d60356 100644 --- a/sapi/fuzzer/fuzzer-sapi.c +++ b/sapi/fuzzer/fuzzer-sapi.c @@ -65,6 +65,11 @@ static int startup(sapi_module_struct *sapi_module) return php_module_startup(sapi_module, NULL); } +static int child_startup(sapi_module_struct *sapi_module) +{ + return php_module_child_startup(sapi_module); +} + static size_t ub_write(const char *str, size_t str_length) { /* quiet */ @@ -103,6 +108,8 @@ static sapi_module_struct fuzzer_module = { startup, /* startup */ php_module_shutdown_wrapper, /* shutdown */ + child_startup, /* child_startup */ + NULL, /* activate */ NULL, /* deactivate */ @@ -159,7 +166,8 @@ int fuzzer_init_php(const char *extra_ini) */ putenv("USE_ZEND_ALLOC=0"); - if (fuzzer_module.startup(&fuzzer_module)==FAILURE) { + if (fuzzer_module.startup(&fuzzer_module)==FAILURE + || fuzzer_module.child_startup(&fuzzer_module)) { return FAILURE; } diff --git a/sapi/phpdbg/phpdbg.c b/sapi/phpdbg/phpdbg.c index a69ac171cface..b3c879026f4d9 100644 --- a/sapi/phpdbg/phpdbg.c +++ b/sapi/phpdbg/phpdbg.c @@ -28,6 +28,7 @@ #include "phpdbg_print.h" #include "phpdbg_help.h" #include "phpdbg_arginfo.h" +#include "zend_types.h" #include "zend_vm.h" #include "php_ini_builder.h" @@ -711,6 +712,11 @@ static inline int php_sapi_phpdbg_module_startup(sapi_module_struct *module) /* return SUCCESS; } /* }}} */ +static inline int php_sapi_phpdbg_module_child_startup(sapi_module_struct *module) /* {{{ */ +{ + return php_module_child_startup(module); +} /* }}} */ + static char* php_sapi_phpdbg_read_cookies(void) /* {{{ */ { return NULL; @@ -938,6 +944,8 @@ static sapi_module_struct phpdbg_sapi_module = { php_sapi_phpdbg_module_startup, /* startup */ php_module_shutdown_wrapper, /* shutdown */ + php_sapi_phpdbg_module_child_startup, /* child_startup */ + php_sapi_phpdbg_activate, /* activate */ php_sapi_phpdbg_deactivate, /* deactivate */ @@ -1355,7 +1363,8 @@ int main(int argc, char **argv) /* {{{ */ /* set flags from command line */ PHPDBG_G(flags) = flags; - if (phpdbg->startup(phpdbg) == SUCCESS) { + if (phpdbg->startup(phpdbg) == SUCCESS + || phpdbg->child_startup(phpdbg) == FAILURE) { zend_mm_heap *mm_heap; #ifdef _WIN32 EXCEPTION_POINTERS *xp; From 65c4947b36946cc00398c9e03cf1f74c3e856d04 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Wed, 28 Feb 2024 17:58:07 +0100 Subject: [PATCH 2/4] fix tests --- ext/opcache/ZendAccelerator.c | 77 ++++++++++++++++------------------- main/main.c | 1 + sapi/phpdbg/phpdbg.c | 2 +- 3 files changed, 36 insertions(+), 44 deletions(-) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index b9149fa1fa12a..fc1ca828d51ef 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -4667,43 +4667,33 @@ static zend_result accel_finish_startup_preload(bool in_child) static zend_result accel_finish_startup_preload_subprocess(pid_t *pid) { + struct passwd *pw = NULL; uid_t euid = geteuid(); if (euid != 0) { if (ZCG(accel_directives).preload_user && *ZCG(accel_directives).preload_user) { zend_accel_error(ACCEL_LOG_WARNING, "\"opcache.preload_user\" is ignored because the current user is not \"root\""); } + } else { + if (!ZCG(accel_directives).preload_user + || !*ZCG(accel_directives).preload_user) { - *pid = -1; - return SUCCESS; - } - - if (!ZCG(accel_directives).preload_user - || !*ZCG(accel_directives).preload_user) { - - bool sapi_requires_preload_user = !(strcmp(sapi_module.name, "cli") == 0 - || strcmp(sapi_module.name, "phpdbg") == 0); + bool sapi_requires_preload_user = !(strcmp(sapi_module.name, "cli") == 0 + || strcmp(sapi_module.name, "phpdbg") == 0); - if (!sapi_requires_preload_user) { - *pid = -1; - return SUCCESS; + if (sapi_requires_preload_user) { + zend_shared_alloc_unlock(); + zend_accel_error_noreturn(ACCEL_LOG_FATAL, "\"opcache.preload\" requires \"opcache.preload_user\" when running under uid 0"); + return FAILURE; + } + } else { + pw = getpwnam(ZCG(accel_directives).preload_user); + if (pw == NULL) { + zend_shared_alloc_unlock(); + zend_accel_error_noreturn(ACCEL_LOG_FATAL, "Preloading failed to getpwnam(\"%s\")", ZCG(accel_directives).preload_user); + return FAILURE; + } } - - zend_shared_alloc_unlock(); - zend_accel_error_noreturn(ACCEL_LOG_FATAL, "\"opcache.preload\" requires \"opcache.preload_user\" when running under uid 0"); - return FAILURE; - } - - struct passwd *pw = getpwnam(ZCG(accel_directives).preload_user); - if (pw == NULL) { - zend_shared_alloc_unlock(); - zend_accel_error_noreturn(ACCEL_LOG_FATAL, "Preloading failed to getpwnam(\"%s\")", ZCG(accel_directives).preload_user); - return FAILURE; - } - - if (pw->pw_uid == euid) { - *pid = -1; - return SUCCESS; } *pid = fork(); @@ -4714,17 +4704,19 @@ static zend_result accel_finish_startup_preload_subprocess(pid_t *pid) } if (*pid == 0) { /* children */ - if (setgid(pw->pw_gid) < 0) { - zend_accel_error(ACCEL_LOG_WARNING, "Preloading failed to setgid(%d)", pw->pw_gid); - exit(1); - } - if (initgroups(pw->pw_name, pw->pw_gid) < 0) { - zend_accel_error(ACCEL_LOG_WARNING, "Preloading failed to initgroups(\"%s\", %d)", pw->pw_name, pw->pw_uid); - exit(1); - } - if (setuid(pw->pw_uid) < 0) { - zend_accel_error(ACCEL_LOG_WARNING, "Preloading failed to setuid(%d)", pw->pw_uid); - exit(1); + if (pw != NULL && pw->pw_uid != euid) { + if (setgid(pw->pw_gid) < 0) { + zend_accel_error(ACCEL_LOG_WARNING, "Preloading failed to setgid(%d)", pw->pw_gid); + exit(1); + } + if (initgroups(pw->pw_name, pw->pw_gid) < 0) { + zend_accel_error(ACCEL_LOG_WARNING, "Preloading failed to initgroups(\"%s\", %d)", pw->pw_name, pw->pw_uid); + exit(1); + } + if (setuid(pw->pw_uid) < 0) { + zend_accel_error(ACCEL_LOG_WARNING, "Preloading failed to setuid(%d)", pw->pw_uid); + exit(1); + } } } @@ -4769,10 +4761,9 @@ static zend_result accel_finish_startup(void) return FAILURE; } - if (pid == -1) { /* no subprocess was needed */ - /* The called function unlocks the shared alloc lock */ - return accel_finish_startup_preload(false); - } else if (pid == 0) { /* subprocess */ + if (pid == 0) { /* subprocess */ + ZEND_ASSERT(sapi_module.child_startup); + sapi_module.child_startup(&sapi_module); const zend_result ret = accel_finish_startup_preload(true); exit(ret == SUCCESS ? 0 : 1); diff --git a/main/main.c b/main/main.c index ac0a9382f63f2..aa7e79fc75e29 100644 --- a/main/main.c +++ b/main/main.c @@ -2367,6 +2367,7 @@ void php_module_shutdown(void) int module_number=0; module_shutdown = true; + php_child_started = 0; if (!module_initialized) { return; diff --git a/sapi/phpdbg/phpdbg.c b/sapi/phpdbg/phpdbg.c index b3c879026f4d9..962bf914a5b6f 100644 --- a/sapi/phpdbg/phpdbg.c +++ b/sapi/phpdbg/phpdbg.c @@ -1364,7 +1364,7 @@ int main(int argc, char **argv) /* {{{ */ PHPDBG_G(flags) = flags; if (phpdbg->startup(phpdbg) == SUCCESS - || phpdbg->child_startup(phpdbg) == FAILURE) { + && phpdbg->child_startup(phpdbg) == SUCCESS) { zend_mm_heap *mm_heap; #ifdef _WIN32 EXCEPTION_POINTERS *xp; From fd02aa45c39eb0869b5a2685f60aee33a9175008 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Wed, 28 Feb 2024 18:04:21 +0100 Subject: [PATCH 3/4] fix build --- main/main.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/main/main.c b/main/main.c index aa7e79fc75e29..b3f83bfebc1c6 100644 --- a/main/main.c +++ b/main/main.c @@ -2367,7 +2367,9 @@ void php_module_shutdown(void) int module_number=0; module_shutdown = true; +#if ZEND_DEBUG php_child_started = 0; +#endif if (!module_initialized) { return; From 9557a8d5fcc67b5752f28226dc6723da2559a4b5 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Wed, 28 Feb 2024 18:27:47 +0100 Subject: [PATCH 4/4] FFI::load() can not be used during preloading --- UPGRADING | 4 ++++ ext/ffi/ffi.c | 2 +- ext/ffi/tests/300.phpt | 25 ------------------------- ext/ffi/tests/bug78761.phpt | 9 +-------- ext/ffi/tests/bug78761_preload.php | 3 --- 5 files changed, 6 insertions(+), 37 deletions(-) delete mode 100644 ext/ffi/tests/300.phpt delete mode 100644 ext/ffi/tests/bug78761_preload.php diff --git a/UPGRADING b/UPGRADING index 9c0909963aeb8..aa3e80f447ffd 100644 --- a/UPGRADING +++ b/UPGRADING @@ -45,6 +45,10 @@ PHP 8.4 UPGRADE NOTES object. This is no longer possible, and cloning a DOMXPath object now throws an error. +- FFI: + . Using FFI::load() during preloading ("opcache.preload") is not supported + anymore. Use "ffi.preload" instead. + - MBString: . mb_encode_numericentity() and mb_decode_numericentity() now check that the $map is only composed of integers, if not a ValueError is thrown. diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index f1ae201f79745..fc97f076530f0 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -3537,7 +3537,7 @@ ZEND_METHOD(FFI, load) /* {{{ */ ZEND_PARSE_PARAMETERS_END(); if (CG(compiler_options) & ZEND_COMPILE_PRELOAD_IN_CHILD) { - zend_throw_error(zend_ffi_exception_ce, "FFI::load() doesn't work in conjunction with \"opcache.preload_user\". Use \"ffi.preload\" instead."); + zend_throw_error(zend_ffi_exception_ce, "FFI::load() doesn't work in conjunction with \"opcache.preload\". Use \"ffi.preload\" instead."); RETURN_THROWS(); } diff --git a/ext/ffi/tests/300.phpt b/ext/ffi/tests/300.phpt deleted file mode 100644 index fbadfb9829d5e..0000000000000 --- a/ext/ffi/tests/300.phpt +++ /dev/null @@ -1,25 +0,0 @@ ---TEST-- -FFI 300: FFI preloading ---EXTENSIONS-- -ffi -opcache -posix ---SKIPIF-- - ---INI-- -ffi.enable=1 -opcache.enable=1 -opcache.enable_cli=1 -opcache.optimization_level=-1 -opcache.preload={PWD}/preload.inc -opcache.file_cache_only=0 ---FILE-- -printf("Hello World from %s!\n", "PHP"); -?> ---EXPECT-- -Hello World from PHP! diff --git a/ext/ffi/tests/bug78761.phpt b/ext/ffi/tests/bug78761.phpt index fc1c661291142..c72b035f13283 100644 --- a/ext/ffi/tests/bug78761.phpt +++ b/ext/ffi/tests/bug78761.phpt @@ -2,15 +2,8 @@ Bug #78761 (Zend memory heap corruption with preload and casting) --EXTENSIONS-- ffi -posix ---SKIPIF-- - --INI-- -opcache.enable_cli=1 -opcache.preload={PWD}/bug78761_preload.php +ffi.preload={PWD}/bug78761_preload.h --FILE--