From 48a89743f3767878230974f79fdc139e69d1eac6 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Sat, 7 Oct 2023 13:59:40 +0100 Subject: [PATCH 1/4] Fix GH-9921: Loading ext in FPM config does not register module handlers Closes GH-12377 --- Zend/zend_API.c | 7 ++- Zend/zend_API.h | 1 + ext/dl_test/dl_test.c | 25 ++++++++- sapi/fpm/fpm/fpm_php.c | 19 +++++-- .../gh9921-php-value-ext-mod-handlers.phpt | 54 +++++++++++++++++++ 5 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 sapi/fpm/tests/gh9921-php-value-ext-mod-handlers.phpt diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 5e89c9182868..540b71495169 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2326,10 +2326,15 @@ ZEND_API void zend_startup_modules(void) /* {{{ */ } /* }}} */ -ZEND_API void zend_destroy_modules(void) /* {{{ */ +ZEND_API void zend_destroy_module_handlers(void) { free(class_cleanup_handlers); free(module_request_startup_handlers); +} + +ZEND_API void zend_destroy_modules(void) /* {{{ */ +{ + zend_destroy_module_handlers(); zend_hash_graceful_reverse_destroy(&module_registry); } /* }}} */ diff --git a/Zend/zend_API.h b/Zend/zend_API.h index 9ed1a3fbabd2..524c1ec2c2c7 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -361,6 +361,7 @@ ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module); 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_module_handlers(void); ZEND_API void zend_destroy_modules(void); ZEND_API void zend_check_magic_method_implementation( const zend_class_entry *ce, const zend_function *fptr, zend_string *lcname, int error_type); diff --git a/ext/dl_test/dl_test.c b/ext/dl_test/dl_test.c index 88a298b0ec88..fed881b2e994 100644 --- a/ext/dl_test/dl_test.c +++ b/ext/dl_test/dl_test.c @@ -69,6 +69,10 @@ PHP_MINIT_FUNCTION(dl_test) REGISTER_INI_ENTRIES(); } + if (getenv("PHP_DL_TEST_MODULE_DEBUG")) { + fprintf(stderr, "DL TEST MINIT\n"); + } + return SUCCESS; } /* }}} */ @@ -83,6 +87,10 @@ static PHP_MSHUTDOWN_FUNCTION(dl_test) UNREGISTER_INI_ENTRIES(); } + if (getenv("PHP_DL_TEST_MODULE_DEBUG")) { + fprintf(stderr, "DL TEST MSHUTDOWN\n"); + } + return SUCCESS; } /* }}} */ @@ -94,6 +102,21 @@ PHP_RINIT_FUNCTION(dl_test) ZEND_TSRMLS_CACHE_UPDATE(); #endif + if (getenv("PHP_DL_TEST_MODULE_DEBUG")) { + fprintf(stderr, "DL TEST RINIT\n"); + } + + return SUCCESS; +} +/* }}} */ + +/* {{{ PHP_RSHUTDOWN_FUNCTION */ +PHP_RSHUTDOWN_FUNCTION(dl_test) +{ + if (getenv("PHP_DL_TEST_MODULE_DEBUG")) { + fprintf(stderr, "DL TEST RSHUTDOWN\n"); + } + return SUCCESS; } /* }}} */ @@ -127,7 +150,7 @@ zend_module_entry dl_test_module_entry = { PHP_MINIT(dl_test), PHP_MSHUTDOWN(dl_test), PHP_RINIT(dl_test), - NULL, + PHP_RSHUTDOWN(dl_test), PHP_MINFO(dl_test), PHP_DL_TEST_VERSION, PHP_MODULE_GLOBALS(dl_test), diff --git a/sapi/fpm/fpm/fpm_php.c b/sapi/fpm/fpm/fpm_php.c index a9ad0433ba4d..06564fe7c217 100644 --- a/sapi/fpm/fpm/fpm_php.c +++ b/sapi/fpm/fpm/fpm_php.c @@ -90,7 +90,7 @@ int fpm_php_apply_defines_ex(struct key_value_s *kv, int mode) /* {{{ */ zend_interned_strings_switch_storage(0); php_dl(value, MODULE_PERSISTENT, &zv, 1); zend_interned_strings_switch_storage(1); - return Z_TYPE(zv) == IS_TRUE; + return Z_TYPE(zv) == IS_TRUE ? 2 : 0; } if (fpm_php_zend_ini_alter_master(name, name_len, value, value_len, mode, PHP_INI_STAGE_ACTIVATE) == FAILURE) { @@ -116,19 +116,32 @@ int fpm_php_apply_defines_ex(struct key_value_s *kv, int mode) /* {{{ */ static int fpm_php_apply_defines(struct fpm_worker_pool_s *wp) /* {{{ */ { struct key_value_s *kv; + int apply_result; + bool extension_loaded = false; for (kv = wp->config->php_values; kv; kv = kv->next) { - if (fpm_php_apply_defines_ex(kv, ZEND_INI_USER) == -1) { + apply_result = fpm_php_apply_defines_ex(kv, ZEND_INI_USER); + if (apply_result == -1) { zlog(ZLOG_ERROR, "Unable to set php_value '%s'", kv->key); + } else if (apply_result == 2) { + extension_loaded = true; } } for (kv = wp->config->php_admin_values; kv; kv = kv->next) { - if (fpm_php_apply_defines_ex(kv, ZEND_INI_SYSTEM) == -1) { + apply_result = fpm_php_apply_defines_ex(kv, ZEND_INI_SYSTEM); + if (apply_result == -1) { zlog(ZLOG_ERROR, "Unable to set php_admin_value '%s'", kv->key); + } else if (apply_result == 2) { + extension_loaded = true; } } + if (extension_loaded) { + zend_destroy_module_handlers(); + zend_collect_module_handlers(); + } + return 0; } /* }}} */ diff --git a/sapi/fpm/tests/gh9921-php-value-ext-mod-handlers.phpt b/sapi/fpm/tests/gh9921-php-value-ext-mod-handlers.phpt new file mode 100644 index 000000000000..a7057373fdf9 --- /dev/null +++ b/sapi/fpm/tests/gh9921-php-value-ext-mod-handlers.phpt @@ -0,0 +1,54 @@ +--TEST-- +FPM: GH-9921 - loading shared ext in FPM config does not register module handlers +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +$tester->request()->expectBody('bool(true)'); +$tester->expectLogPattern('/DL TEST MINIT/'); +$tester->expectLogPattern('/DL TEST RINIT/'); +$tester->expectLogPattern('/DL TEST RSHUTDOWN/'); +$tester->request()->expectBody('bool(true)'); +$tester->expectLogPattern('/DL TEST RINIT/'); +$tester->expectLogPattern('/DL TEST RSHUTDOWN/'); +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- + + Date: Thu, 2 Nov 2023 14:14:10 +0000 Subject: [PATCH 2/4] Modify zend_collect_module_handlers to handle existing allocations --- Zend/zend_API.c | 13 +++++-------- Zend/zend_API.h | 1 - 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 540b71495169..876c93dcceba 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2271,7 +2271,8 @@ ZEND_API void zend_collect_module_handlers(void) /* {{{ */ post_deactivate_count++; } } ZEND_HASH_FOREACH_END(); - module_request_startup_handlers = (zend_module_entry**)malloc( + module_request_startup_handlers = (zend_module_entry**)realloc( + module_request_startup_handlers, sizeof(zend_module_entry*) * (startup_count + 1 + shutdown_count + 1 + @@ -2303,7 +2304,8 @@ ZEND_API void zend_collect_module_handlers(void) /* {{{ */ } } ZEND_HASH_FOREACH_END(); - class_cleanup_handlers = (zend_class_entry**)malloc( + class_cleanup_handlers = (zend_class_entry**)realloc( + class_cleanup_handlers, sizeof(zend_class_entry*) * (class_count + 1)); class_cleanup_handlers[class_count] = NULL; @@ -2326,15 +2328,10 @@ ZEND_API void zend_startup_modules(void) /* {{{ */ } /* }}} */ -ZEND_API void zend_destroy_module_handlers(void) +ZEND_API void zend_destroy_modules(void) /* {{{ */ { free(class_cleanup_handlers); free(module_request_startup_handlers); -} - -ZEND_API void zend_destroy_modules(void) /* {{{ */ -{ - zend_destroy_module_handlers(); zend_hash_graceful_reverse_destroy(&module_registry); } /* }}} */ diff --git a/Zend/zend_API.h b/Zend/zend_API.h index 524c1ec2c2c7..9ed1a3fbabd2 100644 --- a/Zend/zend_API.h +++ b/Zend/zend_API.h @@ -361,7 +361,6 @@ ZEND_API zend_module_entry* zend_register_module_ex(zend_module_entry *module); 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_module_handlers(void); ZEND_API void zend_destroy_modules(void); ZEND_API void zend_check_magic_method_implementation( const zend_class_entry *ce, const zend_function *fptr, zend_string *lcname, int error_type); From f6f80183c8b2c1f19f052a5ca22830d5c7e6c0c3 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 2 Nov 2023 14:34:13 +0000 Subject: [PATCH 3/4] Use self describing constants for fpm_php_apply_defines_ex result --- sapi/fpm/fpm/fpm_php.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/sapi/fpm/fpm/fpm_php.c b/sapi/fpm/fpm/fpm_php.c index 06564fe7c217..dae98bdfc08f 100644 --- a/sapi/fpm/fpm/fpm_php.c +++ b/sapi/fpm/fpm/fpm_php.c @@ -77,6 +77,11 @@ static void fpm_php_disable(char *value, int (*zend_disable)(const char *, size_ } /* }}} */ +#define FPM_PHP_INI_ALTERING_ERROR -1 +#define FPM_PHP_INI_APPLIED 1 +#define FPM_PHP_INI_EXTENSION_FAILED 0 +#define FPM_PHP_INI_EXTENSION_LOADED 2 + int fpm_php_apply_defines_ex(struct key_value_s *kv, int mode) /* {{{ */ { @@ -90,26 +95,26 @@ int fpm_php_apply_defines_ex(struct key_value_s *kv, int mode) /* {{{ */ zend_interned_strings_switch_storage(0); php_dl(value, MODULE_PERSISTENT, &zv, 1); zend_interned_strings_switch_storage(1); - return Z_TYPE(zv) == IS_TRUE ? 2 : 0; + return Z_TYPE(zv) == IS_TRUE ? FPM_PHP_INI_EXTENSION_LOADED : FPM_PHP_INI_EXTENSION_FAILED; } if (fpm_php_zend_ini_alter_master(name, name_len, value, value_len, mode, PHP_INI_STAGE_ACTIVATE) == FAILURE) { - return -1; + return FPM_PHP_INI_ALTERING_ERROR; } if (!strcmp(name, "disable_functions") && *value) { zend_disable_functions(value); - return 1; + return FPM_PHP_INI_APPLIED; } if (!strcmp(name, "disable_classes") && *value) { char *v = strdup(value); PG(disable_classes) = v; fpm_php_disable(v, zend_disable_class); - return 1; + return FPM_PHP_INI_APPLIED; } - return 1; + return FPM_PHP_INI_APPLIED; } /* }}} */ @@ -121,24 +126,23 @@ static int fpm_php_apply_defines(struct fpm_worker_pool_s *wp) /* {{{ */ for (kv = wp->config->php_values; kv; kv = kv->next) { apply_result = fpm_php_apply_defines_ex(kv, ZEND_INI_USER); - if (apply_result == -1) { + if (apply_result == FPM_PHP_INI_ALTERING_ERROR) { zlog(ZLOG_ERROR, "Unable to set php_value '%s'", kv->key); - } else if (apply_result == 2) { + } else if (apply_result == FPM_PHP_INI_EXTENSION_LOADED) { extension_loaded = true; } } for (kv = wp->config->php_admin_values; kv; kv = kv->next) { apply_result = fpm_php_apply_defines_ex(kv, ZEND_INI_SYSTEM); - if (apply_result == -1) { + if (apply_result == FPM_PHP_INI_ALTERING_ERROR) { zlog(ZLOG_ERROR, "Unable to set php_admin_value '%s'", kv->key); - } else if (apply_result == 2) { + } else if (apply_result == FPM_PHP_INI_EXTENSION_LOADED) { extension_loaded = true; } } if (extension_loaded) { - zend_destroy_module_handlers(); zend_collect_module_handlers(); } From b347e5dcb53a1165bbe3408a56ac4149e905cf0a Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 2 Nov 2023 15:25:41 +0000 Subject: [PATCH 4/4] Reset class_cleanup_handlers and module_request_startup_handlers --- Zend/zend_API.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Zend/zend_API.c b/Zend/zend_API.c index 876c93dcceba..5fd38a25c8b7 100644 --- a/Zend/zend_API.c +++ b/Zend/zend_API.c @@ -2331,7 +2331,9 @@ ZEND_API void zend_startup_modules(void) /* {{{ */ ZEND_API void zend_destroy_modules(void) /* {{{ */ { free(class_cleanup_handlers); + class_cleanup_handlers = NULL; free(module_request_startup_handlers); + module_request_startup_handlers = NULL; zend_hash_graceful_reverse_destroy(&module_registry); } /* }}} */