From 304c37156a78b49d2d4009b4d0f1dc0503cf4653 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 12 Nov 2024 18:07:41 +0100 Subject: [PATCH 1/5] Refresh zend_mm shadow key on fork The memory manager is cleaned up after each request by calling shutdown_memory_manager(). At the same time, this prepares the manager for the next request, and the shadow key is refreshed. In forking SAPIs the first request of every child process inherits the memory manager of the parent process, including the shadow key. 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. The memory manager is not empty at this point (we perform allocations after shutdown_memory_manager()), so we have to recompute any shadow pointers with the new key. We assume that the parent process had only one PHP thread running, as anything else would be unsafe. If a SAPI forks a process running more than one PHP thread, it is its responsibility to call refresh_memory_manager() in each PHP thread of the child process. Closes GH-16765 --- Zend/zend_alloc.c | 53 +++++++++++++++++++++--------- Zend/zend_alloc.h | 1 + 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 ++ 11 files changed, 62 insertions(+), 18 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index f2f801db63396..b9dc29e864838 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -70,8 +70,6 @@ # include # include # include "win32/winutil.h" -# define getpid _getpid -typedef int pid_t; #endif #include @@ -317,7 +315,6 @@ struct _zend_mm_heap { } debug; }; #endif - pid_t pid; zend_random_bytes_insecure_state rand_state; }; @@ -1310,15 +1307,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 +2029,30 @@ static void zend_mm_init_key(zend_mm_heap *heap) zend_mm_refresh_key(heap); } +static 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; + } + } +} + 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 +2101,6 @@ static zend_mm_heap *zend_mm_init(void) heap->storage = NULL; #endif heap->huge_list = NULL; - heap->pid = getpid(); return heap; } @@ -2535,13 +2560,7 @@ 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); - } + zend_mm_refresh_key(heap); } } @@ -2949,6 +2968,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 +3530,6 @@ ZEND_API zend_mm_heap *zend_mm_startup_ex(const zend_mm_handlers *handlers, void memcpy(storage->data, data, data_size); } heap->storage = storage; - heap->pid = getpid(); return heap; #else return NULL; diff --git a/Zend/zend_alloc.h b/Zend/zend_alloc.h index 541989a2a13e0..d45ff01a9bf4f 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); diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index e11de304b1b79..4d1b5dff2e353 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 3518e4137ecef..e2973f17c248c 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 a5b049487db23..bd28a0dee1d7f 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 e87223b055e12..88fc584a8bcbb 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 e3cd6f49b9f0c..6db96a43ac97b 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 b7e3e5fa1be2a..92c6ecbdfb581 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 fb02a3e191775..55b0377c1ad06 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 432c0338c46da..d095f9ae90ec3 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 09ad53e233c62..042f5fb7a2b5c 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 { From 69f89a115d10e84420fd738729fda502753eb111 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 29 Jul 2025 11:00:47 +0200 Subject: [PATCH 2/5] Add debug assertion --- Zend/zend_alloc.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index b9dc29e864838..6cfb6aac4617e 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -70,6 +70,8 @@ # include # include # include "win32/winutil.h" +# define getpid _getpid +typedef int pid_t; #endif #include @@ -314,6 +316,9 @@ struct _zend_mm_heap { bool check_freelists_on_shutdown; } debug; }; +#endif +#if ZEND_DEBUG + pid_t pid; #endif zend_random_bytes_insecure_state rand_state; }; @@ -2051,6 +2056,10 @@ static void zend_mm_refresh_key_child(zend_mm_heap *heap) slot = next; } } + +#if ZEND_DEBUG + heap->pid = getpid(); +#endif } static zend_mm_heap *zend_mm_init(void) @@ -2101,6 +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; } @@ -2560,6 +2572,9 @@ 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); + ZEND_ASSERT(getpid() == heap->pid + && "heap was re-used without calling zend_mm_refresh_key_child() after a fork"); + zend_mm_refresh_key(heap); } } @@ -3530,6 +3545,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; From 2f13ece0e4f2941e3e0f8cb69e3b090eeca8fc7c Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 29 Jul 2025 11:33:03 +0200 Subject: [PATCH 3/5] UPGRADING.INTERNALS --- UPGRADING.INTERNALS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 3fd2f27f25270..0e6528ae15248 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -148,3 +148,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. From 12e9cca1ae8306fbf8ae69b32a9f2d39feb9b33e Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 29 Jul 2025 11:38:21 +0200 Subject: [PATCH 4/5] Expose zend_mm_refresh_key_child() --- Zend/zend_alloc.c | 2 +- Zend/zend_alloc.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index 6cfb6aac4617e..70c4081d96b32 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -2034,7 +2034,7 @@ static void zend_mm_init_key(zend_mm_heap *heap) zend_mm_refresh_key(heap); } -static void zend_mm_refresh_key_child(zend_mm_heap *heap) +ZEND_API void zend_mm_refresh_key_child(zend_mm_heap *heap) { uintptr_t old_key = heap->shadow_key; diff --git a/Zend/zend_alloc.h b/Zend/zend_alloc.h index d45ff01a9bf4f..264e13848d1b7 100644 --- a/Zend/zend_alloc.h +++ b/Zend/zend_alloc.h @@ -317,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 From 86769da006a58e8789ace30f2abe6e7841f0ed51 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Tue, 29 Jul 2025 11:46:58 +0200 Subject: [PATCH 5/5] Fix non-debug build --- Zend/zend_alloc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index 70c4081d96b32..edadf024df24b 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -2572,8 +2572,10 @@ 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); +#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); }