diff --git a/NEWS b/NEWS index 131c02d4c240..01556b62951d 100644 --- a/NEWS +++ b/NEWS @@ -40,7 +40,7 @@ PHP NEWS - Curl: . Fix use-after-free of CURLOPT_STDERR stream on cloned curl handle. (lacatoire) - + - Date: . Fixed bug GH-20503 (Assertion failure with ext/date DateInterval property hash construction). (heathdutton) @@ -135,6 +135,8 @@ PHP NEWS order, resulting in wrong functions possibly called in different configurations). (ndossche) . Fixed bug GH-24438 (Preserve indirect types in JIT side traces). (aasul) + . Fixed bug GH-24577 (CE-cache flag on interned class name visible before + map_ptr offset). (prateekbhujel) - PDO: . Fixed invalid ATTR_ORACLE_NULLS values being accepted and truncated. diff --git a/Zend/zend.c b/Zend/zend.c index 32028c9c5a0e..1b1dd9e12e05 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -2101,6 +2101,7 @@ 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); + ZEND_ATOMIC_FENCE_RELEASE(); + GC_ADD_FLAGS(type_name, IS_STR_CLASS_NAME_MAP_PTR); } 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 b2e40e2a2c88..5d4f87490731 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,7 +863,13 @@ 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) +{ + 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) #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) 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/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 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();