From 4f811bd1841ff213ce5b5ccfdc78c7a039959f4d Mon Sep 17 00:00:00 2001 From: Pratik Bhujel Date: Sat, 24 Oct 2026 09:32:03 +0545 Subject: [PATCH 1/3] Fix GH-24577: Reject uncommitted refcount in interned class CE-cache When OPcache persists a script into shared memory, zend_accel_pending_ce_cache_commit() (and zend_alloc_ce_cache() in Zend/zend.c) writes the allocated map_ptr offset into type_name->gc.refcount and sets IS_STR_CLASS_NAME_MAP_PTR on the shared interned string. Because these are plain non-atomic stores to adjacent struct fields, the compiler or weakly ordered CPU can make IS_STR_CLASS_NAME_MAP_PTR visible to concurrent worker processes before the map_ptr offset overwrites the initial SHM interned string refcount of 2. Previously, ZSTR_VALID_CE_CACHE() only checked (GC_REFCOUNT(s) - 1) / sizeof(void *) < CG(map_ptr_last), which accepted refcount 2 ((2 - 1) / 8 == 0 < CG(map_ptr_last)) and dereferenced an unaligned address at map_ptr_base + 2. Require GC_REFCOUNT(s) > 2 in ZSTR_VALID_CE_CACHE() and ZSTR_SET_CE_CACHE_EX(), and order GC_SET_REFCOUNT before GC_ADD_FLAGS in zend_alloc_ce_cache(). --- Zend/zend.c | 2 +- Zend/zend_types.h | 7 +- ext/opcache/tests/gh24577.phpt | 150 +++++++++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 ext/opcache/tests/gh24577.phpt diff --git a/Zend/zend.c b/Zend/zend.c index 32028c9c5a0e..f44f1913ee09 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -2101,6 +2101,6 @@ ZEND_API void zend_alloc_ce_cache(zend_string *type_name) do { ret = ZEND_MAP_PTR_NEW_OFFSET(); } while (ret <= 2); - GC_ADD_FLAGS(type_name, IS_STR_CLASS_NAME_MAP_PTR); GC_SET_REFCOUNT(type_name, ret); + GC_ADD_FLAGS(type_name, IS_STR_CLASS_NAME_MAP_PTR); } diff --git a/Zend/zend_types.h b/Zend/zend_types.h index b2e40e2a2c88..57cb3ba180f6 100644 --- a/Zend/zend_types.h +++ b/Zend/zend_types.h @@ -866,13 +866,16 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) { #define ZSTR_GET_CE_CACHE(s) ZSTR_GET_CE_CACHE_EX(s, 1) #define ZSTR_SET_CE_CACHE(s, ce) ZSTR_SET_CE_CACHE_EX(s, ce, 1) -#define ZSTR_VALID_CE_CACHE(s) EXPECTED((GC_REFCOUNT(s)-1)/sizeof(void *) < CG(map_ptr_last)) +/* Valid map_ptr offsets are always odd and strictly > 2 ((k * sizeof(void*)) + 1). + * Interned strings in SHM initialize with refcount 2. A refcount <= 2 indicates + * an unallocated or not-yet-committed cache slot and must not be dereferenced. */ +#define ZSTR_VALID_CE_CACHE(s) EXPECTED(GC_REFCOUNT(s) > 2 && (GC_REFCOUNT(s)-1)/sizeof(void *) < CG(map_ptr_last)) #define ZSTR_GET_CE_CACHE_EX(s, validate) \ ((!(validate) || ZSTR_VALID_CE_CACHE(s)) ? GET_CE_CACHE(GC_REFCOUNT(s)) : NULL) #define ZSTR_SET_CE_CACHE_EX(s, ce, validate) do { \ - if (!(validate) || ZSTR_VALID_CE_CACHE(s)) { \ + if ((!(validate) || ZSTR_VALID_CE_CACHE(s)) && EXPECTED(GC_REFCOUNT(s) > 2)) { \ ZEND_ASSERT((validate) || ZSTR_VALID_CE_CACHE(s)); \ SET_CE_CACHE(GC_REFCOUNT(s), ce); \ } \ diff --git a/ext/opcache/tests/gh24577.phpt b/ext/opcache/tests/gh24577.phpt new file mode 100644 index 000000000000..12374425c859 --- /dev/null +++ b/ext/opcache/tests/gh24577.phpt @@ -0,0 +1,150 @@ +--TEST-- +Bug GH-24577: CE-cache flag on shared interned class name must not crash with concurrent access +--EXTENSIONS-- +opcache +pcntl +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.interned_strings_buffer=8 +opcache.file_update_protection=0 +--FILE-- + $readyPrefix . $r, range(0, $numReaders - 1)); +$expectedReady[] = $readyPrefix . 'writer'; +foreach ($expectedReady as $readyFile) { + while (!file_exists($readyFile)) { + usleep(200); + } +} + +// Release barrier so writer persists $classesFile while readers probe $names +file_put_contents($gateFile, '1'); + +$failed = false; +foreach ($children as $pid) { + pcntl_waitpid($pid, $status); + if (!pcntl_wifexited($status) || pcntl_wexitstatus($status) !== 0) { + $failed = true; + } +} + +foreach ($expectedReady as $readyFile) { + @unlink($readyFile); +} +@unlink($gateFile); +@unlink($consumerFile); +@unlink($classesFile); + +echo $failed ? "Failed\n" : "Done\n"; +?> +--CLEAN-- + +--EXPECT-- +Done From a6b9c1f4333df1cba93b71130f5c6ec8281a1392 Mon Sep 17 00:00:00 2001 From: Pratik Bhujel Date: Sat, 24 Oct 2026 16:06:38 +0545 Subject: [PATCH 2/3] Use release and acquire memory barriers for interned class CE-cache Replace hardening check with explicit release and acquire memory barriers to prevent compiler and CPU reordering between GC_SET_REFCOUNT and GC_ADD_FLAGS(..., IS_STR_CLASS_NAME_MAP_PTR) across all interned class cache writers and readers. --- Zend/zend.c | 1 + Zend/zend_types.h | 19 +++++++++++++------ ext/opcache/ZendAccelerator.c | 4 +++- ext/opcache/zend_persist.c | 1 + 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Zend/zend.c b/Zend/zend.c index f44f1913ee09..1b1dd9e12e05 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -2102,5 +2102,6 @@ ZEND_API void zend_alloc_ce_cache(zend_string *type_name) ret = ZEND_MAP_PTR_NEW_OFFSET(); } while (ret <= 2); GC_SET_REFCOUNT(type_name, ret); + ZEND_ATOMIC_FENCE_RELEASE(); GC_ADD_FLAGS(type_name, IS_STR_CLASS_NAME_MAP_PTR); } diff --git a/Zend/zend_types.h b/Zend/zend_types.h index 57cb3ba180f6..8a6398f0973a 100644 --- a/Zend/zend_types.h +++ b/Zend/zend_types.h @@ -24,6 +24,7 @@ #include "zend_portability.h" #include "zend_long.h" +#include "zend_atomic.h" #include #include @@ -862,20 +863,26 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) { #define OBJ_EXTRA_FLAGS(obj) ((obj)->extra_flags) /* Fast class cache */ -#define ZSTR_HAS_CE_CACHE(s) (GC_FLAGS(s) & IS_STR_CLASS_NAME_MAP_PTR) +static zend_always_inline bool zstr_has_ce_cache(const zend_string *s) +{ + if (GC_FLAGS(s) & IS_STR_CLASS_NAME_MAP_PTR) { + ZEND_ATOMIC_FENCE_ACQUIRE(); + return true; + } + return false; +} + +#define ZSTR_HAS_CE_CACHE(s) zstr_has_ce_cache(s) #define ZSTR_GET_CE_CACHE(s) ZSTR_GET_CE_CACHE_EX(s, 1) #define ZSTR_SET_CE_CACHE(s, ce) ZSTR_SET_CE_CACHE_EX(s, ce, 1) -/* Valid map_ptr offsets are always odd and strictly > 2 ((k * sizeof(void*)) + 1). - * Interned strings in SHM initialize with refcount 2. A refcount <= 2 indicates - * an unallocated or not-yet-committed cache slot and must not be dereferenced. */ -#define ZSTR_VALID_CE_CACHE(s) EXPECTED(GC_REFCOUNT(s) > 2 && (GC_REFCOUNT(s)-1)/sizeof(void *) < CG(map_ptr_last)) +#define ZSTR_VALID_CE_CACHE(s) EXPECTED((GC_REFCOUNT(s)-1)/sizeof(void *) < CG(map_ptr_last)) #define ZSTR_GET_CE_CACHE_EX(s, validate) \ ((!(validate) || ZSTR_VALID_CE_CACHE(s)) ? GET_CE_CACHE(GC_REFCOUNT(s)) : NULL) #define ZSTR_SET_CE_CACHE_EX(s, ce, validate) do { \ - if ((!(validate) || ZSTR_VALID_CE_CACHE(s)) && EXPECTED(GC_REFCOUNT(s) > 2)) { \ + if (!(validate) || ZSTR_VALID_CE_CACHE(s)) { \ ZEND_ASSERT((validate) || ZSTR_VALID_CE_CACHE(s)); \ SET_CE_CACHE(GC_REFCOUNT(s), ce); \ } \ diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index da3114ad60bb..b5980f2b3190 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -450,8 +450,9 @@ static void accel_interned_strings_restore_state(void) /* Discard non-global CE_CACHE slots on reset. */ uintptr_t idx = (GC_REFCOUNT(s) - 1) / sizeof(void *); if (idx >= ZCSG(map_ptr_last)) { - GC_SET_REFCOUNT(s, 2); GC_DEL_FLAGS(s, IS_STR_CLASS_NAME_MAP_PTR); + ZEND_ATOMIC_FENCE_RELEASE(); + GC_SET_REFCOUNT(s, 2); } } @@ -560,6 +561,7 @@ zend_string* ZEND_FASTCALL accel_new_interned_string(zend_string *str) if (ZSTR_HAS_CE_CACHE(str) && !ZSTR_HAS_CE_CACHE(s)) { ZEND_ASSERT(GC_FLAGS(str) & IS_STR_PERMANENT); GC_SET_REFCOUNT(s, GC_REFCOUNT(str)); + ZEND_ATOMIC_FENCE_RELEASE(); GC_ADD_FLAGS(s, IS_STR_CLASS_NAME_MAP_PTR); } diff --git a/ext/opcache/zend_persist.c b/ext/opcache/zend_persist.c index 8b8312ff81ef..8e9167ac7d45 100644 --- a/ext/opcache/zend_persist.c +++ b/ext/opcache/zend_persist.c @@ -343,6 +343,7 @@ void zend_accel_pending_ce_cache_commit(void) ZEND_ASSERT(!ZSTR_HAS_CE_CACHE(type_name)); GC_SET_REFCOUNT(type_name, (uint32_t)Z_LVAL_P(zv)); + ZEND_ATOMIC_FENCE_RELEASE(); GC_ADD_FLAGS(type_name, IS_STR_CLASS_NAME_MAP_PTR); } ZEND_HASH_FOREACH_END(); From 4d16d4fb7932cc456eb6f3a1556af65677d63000 Mon Sep 17 00:00:00 2001 From: Pratik Bhujel Date: Mon, 26 Oct 2026 05:50:56 +0545 Subject: [PATCH 3/3] Use ZEND_ATOMIC_LOAD_ACQUIRE_32 in zstr_has_ce_cache() --- Zend/zend_atomic.h | 13 +++++++++++++ Zend/zend_types.h | 7 ++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/Zend/zend_atomic.h b/Zend/zend_atomic.h index 5eeb212ec09e..677d2ed0ccb2 100644 --- a/Zend/zend_atomic.h +++ b/Zend/zend_atomic.h @@ -402,6 +402,19 @@ ZEND_API int zend_atomic_int_load(const zend_atomic_int *obj); # define ZEND_ATOMIC_FENCE_ACQUIRE() ((void)0) #endif +static zend_always_inline uint32_t zend_atomic_load_acquire_32(const uint32_t *ptr) +{ +#if defined(__GNUC__) || defined(__clang__) || defined(HAVE_GNUC_ATOMICS) + return __atomic_load_n(ptr, __ATOMIC_ACQUIRE); +#else + uint32_t val = *(const volatile uint32_t *)ptr; + ZEND_ATOMIC_FENCE_ACQUIRE(); + return val; +#endif +} + +#define ZEND_ATOMIC_LOAD_ACQUIRE_32(ptr) zend_atomic_load_acquire_32(ptr) + END_EXTERN_C() #endif diff --git a/Zend/zend_types.h b/Zend/zend_types.h index 8a6398f0973a..5d4f87490731 100644 --- a/Zend/zend_types.h +++ b/Zend/zend_types.h @@ -865,11 +865,8 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) { /* Fast class cache */ static zend_always_inline bool zstr_has_ce_cache(const zend_string *s) { - if (GC_FLAGS(s) & IS_STR_CLASS_NAME_MAP_PTR) { - ZEND_ATOMIC_FENCE_ACQUIRE(); - return true; - } - return false; + uint32_t type_info = ZEND_ATOMIC_LOAD_ACQUIRE_32(&GC_TYPE_INFO(s)); + return (type_info & (IS_STR_CLASS_NAME_MAP_PTR << GC_FLAGS_SHIFT)) != 0; } #define ZSTR_HAS_CE_CACHE(s) zstr_has_ce_cache(s)