From c561f7da8587c9d26d5c214406f48349a643f400 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 12 Nov 2024 18:07:41 +0100 Subject: [PATCH] Refresh zend_mm shadow key on fork The shadow key is refreshed when resetting the memory manager between two requests. But in forking SAPIs the first request of a child process inherits the shadow key of the parent. As a result, a leak of the shadow key during the first request of one process gives away the shadow key used during the first request of other processes. This makes the key refresh mechanism less useful. Here I ensure that we refresh the shadow key after a fork. We can not reset the manager as there may be active allocations. Instead, we have to recompute shadow pointers with the new key. Closes GH-16765 --- UPGRADING.INTERNALS | 6 +++ Zend/zend_alloc.c | 63 +++++++++++++++++++++++++----- Zend/zend_alloc.h | 3 ++ ext/pcntl/pcntl.c | 7 ++-- main/main.c | 6 +++ main/php_main.h | 1 + sapi/apache2handler/sapi_apache2.c | 1 + sapi/cgi/cgi_main.c | 2 + sapi/cli/php_cli_server.c | 1 + sapi/fpm/fpm/fpm_php.c | 3 ++ sapi/litespeed/lsapi_main.c | 2 + sapi/litespeed/lscriu.c | 3 ++ 12 files changed, 85 insertions(+), 13 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 3fd2f27f252..a0e69bd083a 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -73,6 +73,8 @@ PHP 8.5 INTERNALS UPGRADE NOTES . EG(fake_scope) now is a _const_ zend_class_entry*. . zend_begin_record_errors() or EG(record_errors)=true cause errors to be delayed. Before, errors would be recorded but not delayed. + . zend_mm_refresh_key_child() must be called on any zend_mm_heap inherited + from the parent process after a fork(). ======================== 2. Build system changes @@ -148,3 +150,7 @@ PHP 8.5 INTERNALS UPGRADE NOTES ======================== 5. SAPI changes ======================== + +- SAPIs must now call php_child_init() after a fork. If php-src code was + executed in other threads than the one initiating the fork, + refresh_memory_manager() must be called in every such thread. diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index f2f801db633..edadf024df2 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -317,7 +317,9 @@ struct _zend_mm_heap { } debug; }; #endif +#if ZEND_DEBUG pid_t pid; +#endif zend_random_bytes_insecure_state rand_state; }; @@ -1310,15 +1312,20 @@ static zend_always_inline zend_mm_free_slot* zend_mm_encode_free_slot(const zend #endif } -static zend_always_inline zend_mm_free_slot* zend_mm_decode_free_slot(zend_mm_heap *heap, zend_mm_free_slot *slot) +static zend_always_inline zend_mm_free_slot* zend_mm_decode_free_slot_key(uintptr_t shadow_key, zend_mm_free_slot *slot) { #ifdef WORDS_BIGENDIAN - return (zend_mm_free_slot*)((uintptr_t)slot ^ heap->shadow_key); + return (zend_mm_free_slot*)((uintptr_t)slot ^ shadow_key); #else - return (zend_mm_free_slot*)(BSWAPPTR((uintptr_t)slot ^ heap->shadow_key)); + return (zend_mm_free_slot*)(BSWAPPTR((uintptr_t)slot ^ shadow_key)); #endif } +static zend_always_inline zend_mm_free_slot* zend_mm_decode_free_slot(zend_mm_heap *heap, zend_mm_free_slot *slot) +{ + return zend_mm_decode_free_slot_key(heap->shadow_key, slot); +} + static zend_always_inline void zend_mm_set_next_free_slot(zend_mm_heap *heap, uint32_t bin_num, zend_mm_free_slot *slot, zend_mm_free_slot *next) { ZEND_ASSERT(bin_data_size[bin_num] >= ZEND_MM_MIN_USEABLE_BIN_SIZE); @@ -2027,6 +2034,34 @@ static void zend_mm_init_key(zend_mm_heap *heap) zend_mm_refresh_key(heap); } +ZEND_API void zend_mm_refresh_key_child(zend_mm_heap *heap) +{ + uintptr_t old_key = heap->shadow_key; + + zend_mm_init_key(heap); + + /* Update shadow pointers with new key */ + for (int i = 0; i < ZEND_MM_BINS; i++) { + zend_mm_free_slot *slot = heap->free_slot[i]; + if (!slot) { + continue; + } + zend_mm_free_slot *next; + while ((next = slot->next_free_slot)) { + zend_mm_free_slot *shadow = ZEND_MM_FREE_SLOT_PTR_SHADOW(slot, i); + if (UNEXPECTED(next != zend_mm_decode_free_slot_key(old_key, shadow))) { + zend_mm_panic("zend_mm_heap corrupted"); + } + zend_mm_set_next_free_slot(heap, i, slot, next); + slot = next; + } + } + +#if ZEND_DEBUG + heap->pid = getpid(); +#endif +} + static zend_mm_heap *zend_mm_init(void) { zend_mm_chunk *chunk = (zend_mm_chunk*)zend_mm_chunk_alloc_int(ZEND_MM_CHUNK_SIZE, ZEND_MM_CHUNK_SIZE); @@ -2075,7 +2110,9 @@ static zend_mm_heap *zend_mm_init(void) heap->storage = NULL; #endif heap->huge_list = NULL; +#if ZEND_DEBUG heap->pid = getpid(); +#endif return heap; } @@ -2535,13 +2572,12 @@ ZEND_API void zend_mm_shutdown(zend_mm_heap *heap, bool full, bool silent) p->free_map[0] = (1L << ZEND_MM_FIRST_PAGE) - 1; p->map[0] = ZEND_MM_LRUN(ZEND_MM_FIRST_PAGE); - pid_t pid = getpid(); - if (heap->pid != pid) { - zend_mm_init_key(heap); - heap->pid = pid; - } else { - zend_mm_refresh_key(heap); - } +#if ZEND_DEBUG + ZEND_ASSERT(getpid() == heap->pid + && "heap was re-used without calling zend_mm_refresh_key_child() after a fork"); +#endif + + zend_mm_refresh_key(heap); } } @@ -2949,6 +2985,11 @@ ZEND_API void shutdown_memory_manager(bool silent, bool full_shutdown) zend_mm_shutdown(AG(mm_heap), full_shutdown, silent); } +ZEND_API void refresh_memory_manager(void) +{ + zend_mm_refresh_key_child(AG(mm_heap)); +} + static ZEND_COLD ZEND_NORETURN void zend_out_of_memory(void) { fprintf(stderr, "Out of memory\n"); @@ -3506,7 +3547,9 @@ ZEND_API zend_mm_heap *zend_mm_startup_ex(const zend_mm_handlers *handlers, void memcpy(storage->data, data, data_size); } heap->storage = storage; +#if ZEND_DEBUG heap->pid = getpid(); +#endif return heap; #else return NULL; diff --git a/Zend/zend_alloc.h b/Zend/zend_alloc.h index 541989a2a13..264e13848d1 100644 --- a/Zend/zend_alloc.h +++ b/Zend/zend_alloc.h @@ -220,6 +220,7 @@ ZEND_API bool zend_alloc_in_memory_limit_error_reporting(void); ZEND_API void start_memory_manager(void); ZEND_API void shutdown_memory_manager(bool silent, bool full_shutdown); +ZEND_API void refresh_memory_manager(void); ZEND_API bool is_zend_mm(void); ZEND_API bool is_zend_ptr(const void *ptr); @@ -316,6 +317,8 @@ struct _zend_mm_storage { ZEND_API zend_mm_storage *zend_mm_get_storage(zend_mm_heap *heap); ZEND_API zend_mm_heap *zend_mm_startup_ex(const zend_mm_handlers *handlers, void *data, size_t data_size); +ZEND_API void zend_mm_refresh_key_child(zend_mm_heap *heap); + /* // The following example shows how to use zend_mm_heap API with custom storage diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index e11de304b1b..4d1b5dff2e3 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -32,6 +32,7 @@ #include "php_signal.h" #include "php_ticks.h" #include "zend_fibers.h" +#include "main/php_main.h" #if defined(HAVE_GETPRIORITY) || defined(HAVE_SETPRIORITY) || defined(HAVE_WAIT3) #include @@ -297,7 +298,7 @@ PHP_FUNCTION(pcntl_fork) } } else if (id == 0) { - zend_max_execution_timer_init(); + php_child_init(); } RETURN_LONG((zend_long) id); @@ -1560,7 +1561,7 @@ PHP_FUNCTION(pcntl_rfork) php_error_docref(NULL, E_WARNING, "Error %d", errno); } } else if (pid == 0) { - zend_max_execution_timer_init(); + php_child_init(); } RETURN_LONG((zend_long) pid); @@ -1605,7 +1606,7 @@ PHP_FUNCTION(pcntl_forkx) php_error_docref(NULL, E_WARNING, "Error %d", errno); } } else if (pid == 0) { - zend_max_execution_timer_init(); + php_child_init(); } RETURN_LONG((zend_long) pid); diff --git a/main/main.c b/main/main.c index 3518e4137ec..e2973f17c24 100644 --- a/main/main.c +++ b/main/main.c @@ -1816,6 +1816,12 @@ static void sigchld_handler(int apar) /* }}} */ #endif +PHPAPI void php_child_init(void) +{ + refresh_memory_manager(); + zend_max_execution_timer_init(); +} + /* {{{ php_request_startup */ zend_result php_request_startup(void) { diff --git a/main/php_main.h b/main/php_main.h index a5b049487db..bd28a0dee1d 100644 --- a/main/php_main.h +++ b/main/php_main.h @@ -49,6 +49,7 @@ ZEND_ATTRIBUTE_CONST PHPAPI const char *php_build_provider(void); PHPAPI char *php_get_version(sapi_module_struct *sapi_module); PHPAPI void php_print_version(sapi_module_struct *sapi_module); +PHPAPI void php_child_init(void); 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); diff --git a/sapi/apache2handler/sapi_apache2.c b/sapi/apache2handler/sapi_apache2.c index e87223b055e..88fc584a8bc 100644 --- a/sapi/apache2handler/sapi_apache2.c +++ b/sapi/apache2handler/sapi_apache2.c @@ -752,6 +752,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); + php_child_init(); } #ifdef ZEND_SIGNALS diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c index e3cd6f49b9f..6db96a43ac9 100644 --- a/sapi/cgi/cgi_main.c +++ b/sapi/cgi/cgi_main.c @@ -2041,6 +2041,8 @@ consult the installation file that came with this distribution, or visit \n\ */ parent = 0; + php_child_init(); + /* don't catch our signals */ sigaction(SIGTERM, &old_term, 0); sigaction(SIGQUIT, &old_quit, 0); diff --git a/sapi/cli/php_cli_server.c b/sapi/cli/php_cli_server.c index b7e3e5fa1be..92c6ecbdfb5 100644 --- a/sapi/cli/php_cli_server.c +++ b/sapi/cli/php_cli_server.c @@ -2530,6 +2530,7 @@ static void php_cli_server_startup_workers(void) { #if defined(HAVE_PRCTL) || defined(HAVE_PROCCTL) php_cli_server_worker_install_pdeathsig(); #endif + php_child_init(); return; } else { php_cli_server_workers[php_cli_server_worker] = pid; diff --git a/sapi/fpm/fpm/fpm_php.c b/sapi/fpm/fpm/fpm_php.c index fb02a3e1917..55b0377c1ad 100644 --- a/sapi/fpm/fpm/fpm_php.c +++ b/sapi/fpm/fpm/fpm_php.c @@ -253,6 +253,9 @@ int fpm_php_init_child(struct fpm_worker_pool_s *wp) /* {{{ */ limit_extensions = wp->limit_extensions; wp->limit_extensions = NULL; } + + php_child_init(); + return 0; } /* }}} */ diff --git a/sapi/litespeed/lsapi_main.c b/sapi/litespeed/lsapi_main.c index 432c0338c46..d095f9ae90e 100644 --- a/sapi/litespeed/lsapi_main.c +++ b/sapi/litespeed/lsapi_main.c @@ -1395,6 +1395,8 @@ void start_children( int children ) switch( pid ) { case 0: /* children process */ + php_child_init(); + /* don't catch our signals */ sigaction( SIGTERM, &old_term, 0 ); sigaction( SIGQUIT, &old_quit, 0 ); diff --git a/sapi/litespeed/lscriu.c b/sapi/litespeed/lscriu.c index 09ad53e233c..042f5fb7a2b 100644 --- a/sapi/litespeed/lscriu.c +++ b/sapi/litespeed/lscriu.c @@ -85,6 +85,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "lscriu.h" #include +#include "main/php_main.h" #define LSCRIU_PATH 256 @@ -459,6 +460,7 @@ static void LSCRIU_CloudLinux_Checkpoint(void) else { s_restored = 1; LSAPI_reset_server_state(); + php_child_init(); /* Here we have restored the php process, so we should to tell (via semaphore) mod_lsapi that we are started and ready to receive data. @@ -532,6 +534,7 @@ static void LSCRIU_try_checkpoint(int *forked_pid) LSCRIU_Wait_Dump_Finish_Or_Restored(iPidParent); LSCRIU_Restored_Error(0, "Restored!"); LSAPI_reset_server_state(); + php_child_init(); s_restored = 1; } else {