From f3c4b25dc2c6221c96fdc7903edd70e62bcd94d0 Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Tue, 10 Nov 2026 15:16:41 +0100 Subject: [PATCH 1/3] Fix stale PFA cache entries after the declaring site is freed The request PFA cache is keyed on the address of the declaring site. When eval()'d code or a class constant AST was freed, a new PFA could reuse that address and get the cached function for a different call. --- NEWS | 2 + .../cache_freed_declaring_site_001.phpt | 37 +++++++++++++++++++ .../cache_freed_declaring_site_002.phpt | 36 ++++++++++++++++++ .../cache_freed_declaring_site_003.phpt | 37 +++++++++++++++++++ Zend/zend_ast.c | 5 ++- Zend/zend_partial.c | 16 +++++--- Zend/zend_partial.h | 6 ++- Zend/zend_vm_def.h | 2 +- Zend/zend_vm_execute.h | 8 ++-- ext/opcache/ZendAccelerator.c | 29 +++++++++++---- ext/opcache/ZendAccelerator.h | 1 + 11 files changed, 157 insertions(+), 22 deletions(-) create mode 100644 Zend/tests/partial_application/cache_freed_declaring_site_001.phpt create mode 100644 Zend/tests/partial_application/cache_freed_declaring_site_002.phpt create mode 100644 Zend/tests/partial_application/cache_freed_declaring_site_003.phpt diff --git a/NEWS b/NEWS index 85e52683194f..4c3b30f0ba3f 100644 --- a/NEWS +++ b/NEWS @@ -26,6 +26,8 @@ PHP NEWS . Fixed use-after-free when the error handler releases a string while an "Illegal string offset" warning is raised for its offset in a read, ?? or nested isset()/empty(). (Jorg Sowa) + . Fixed partial function applications in eval'd code or class constants + reusing a stale cached function with wrong bound arguments. (Jorg Sowa) - Curl: . Fixed curl_reset() retaining CURLOPT_PRIVATE after reset. (lacatoire) diff --git a/Zend/tests/partial_application/cache_freed_declaring_site_001.phpt b/Zend/tests/partial_application/cache_freed_declaring_site_001.phpt new file mode 100644 index 000000000000..56b4edfc932a --- /dev/null +++ b/Zend/tests/partial_application/cache_freed_declaring_site_001.phpt @@ -0,0 +1,37 @@ +--TEST-- +PFA in class constant: request cache must not outlive the freed declaring AST +--FILE-- + +--EXPECT-- +Done diff --git a/Zend/tests/partial_application/cache_freed_declaring_site_002.phpt b/Zend/tests/partial_application/cache_freed_declaring_site_002.phpt new file mode 100644 index 000000000000..56959d460c35 --- /dev/null +++ b/Zend/tests/partial_application/cache_freed_declaring_site_002.phpt @@ -0,0 +1,36 @@ +--TEST-- +PFA in eval(): request cache must not outlive the freed declaring op_array +--FILE-- + +--EXPECT-- +Done diff --git a/Zend/tests/partial_application/cache_freed_declaring_site_003.phpt b/Zend/tests/partial_application/cache_freed_declaring_site_003.phpt new file mode 100644 index 000000000000..aa85ad6c9480 --- /dev/null +++ b/Zend/tests/partial_application/cache_freed_declaring_site_003.phpt @@ -0,0 +1,37 @@ +--TEST-- +PFA in constant declared in eval(): request cache must not outlive the freed declaring AST +--FILE-- + +--EXPECT-- +Done diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index 051e9e612667..a331155dac46 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -1361,10 +1361,13 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_inner( if (uses_variadic_placeholder) { flags |= ZEND_PARTIAL_USES_VARIADIC_PLACEHOLDER; } + /* The cached PFA keeps a non-interned name alive, unlike a non-persistent AST */ + const uint32_t *cache_key = ZSTR_IS_INTERNED(fcc_ast->name) + ? &ast->lineno : (const uint32_t *) fcc_ast->name; zend_partial_create(result, scope, &frame->This, fptr, ZEND_CALL_NUM_ARGS(frame), ZEND_CALL_ARG(frame, 1), extra_named_params, named_positions, - fcc_ast->filename, &ast->lineno, + fcc_ast->filename, cache_key, /* declaring_op_array */ NULL, (void**)cache_slot, fcc_ast->name, flags, /* const_args */ 0); if (named_positions) { diff --git a/Zend/zend_partial.c b/Zend/zend_partial.c index 1108cff2301a..b227c1f9e257 100644 --- a/Zend/zend_partial.c +++ b/Zend/zend_partial.c @@ -701,7 +701,8 @@ static zend_op_array *zp_compile(zval *this_ptr, zend_function *function, uint32_t argc, zval *argv, zend_array *extra_named_params, const zend_array *named_positions, zend_string *declaring_filename, - const uint32_t *declaring_lineno_ptr, void **cache_slot, + const uint32_t *declaring_lineno_ptr, + const zend_op_array *declaring_op_array, void **cache_slot, zend_string *pfa_name, uint32_t flags, uint32_t const_args) { zend_op_array *op_array = NULL; @@ -1016,7 +1017,8 @@ static zend_op_array *zp_compile(zval *this_ptr, zend_function *function, /* Takes ownership of closure_ast */ op_array = zend_accel_compile_pfa(closure_ast, declaring_filename, - declaring_lineno_ptr, function, pfa_name, flags & ZEND_PARTIAL_CACHEABLE_IN_SHM); + declaring_lineno_ptr, declaring_op_array, function, pfa_name, + flags & ZEND_PARTIAL_CACHEABLE_IN_SHM); clean: zp_names_dtor(var_names, argc); @@ -1040,7 +1042,8 @@ static const zend_op_array *zp_get_op_array(zval *this_ptr, zend_function *funct uint32_t argc, zval *argv, zend_array *extra_named_params, const zend_array *named_positions, zend_string *declaring_filename, - const uint32_t *declaring_lineno_ptr, void **cache_slot, + const uint32_t *declaring_lineno_ptr, + const zend_op_array *declaring_op_array, void **cache_slot, zend_string *pfa_name, uint32_t flags, uint32_t const_args) { if (EXPECTED(function->type == ZEND_INTERNAL_FUNCTION @@ -1056,7 +1059,7 @@ static const zend_op_array *zp_get_op_array(zval *this_ptr, zend_function *funct if (UNEXPECTED(!op_array)) { op_array = zp_compile(this_ptr, function, argc, argv, extra_named_params, named_positions, declaring_filename, declaring_lineno_ptr, - cache_slot, pfa_name, flags, const_args); + declaring_op_array, cache_slot, pfa_name, flags, const_args); } if (EXPECTED(op_array) && !(function->common.fn_flags & ZEND_ACC_NEVER_CACHE)) { @@ -1135,7 +1138,8 @@ void zend_partial_create(zval *result, zend_class_entry *scope, zval *this_ptr, uint32_t argc, zval *argv, zend_array *extra_named_params, const zend_array *named_positions, zend_string *declaring_filename, - const uint32_t *declaring_lineno_ptr, void **cache_slot, + const uint32_t *declaring_lineno_ptr, + const zend_op_array *declaring_op_array, void **cache_slot, zend_string *pfa_name, uint32_t flags, uint32_t const_args) { ZEND_ASSERT(pfa_name); @@ -1143,7 +1147,7 @@ void zend_partial_create(zval *result, zend_class_entry *scope, zval *this_ptr, const zend_op_array *op_array = zp_get_op_array(this_ptr, function, argc, argv, extra_named_params, named_positions, declaring_filename, declaring_lineno_ptr, - cache_slot, pfa_name, flags, const_args); + declaring_op_array, cache_slot, pfa_name, flags, const_args); if (UNEXPECTED(!op_array)) { ZEND_ASSERT(EG(exception)); diff --git a/Zend/zend_partial.h b/Zend/zend_partial.h index 285db1161e78..43b4c8ab5615 100644 --- a/Zend/zend_partial.h +++ b/Zend/zend_partial.h @@ -30,12 +30,14 @@ BEGIN_EXTERN_C() /* Create a partial application of 'function' * 'declaring_lineno_ptr' should be a pointer the zend_op.lineno or * zend_ast.lineno that declares the PFA. The address is used to build a cache - * key. */ + * key, so it must not be reused while cached. 'declaring_op_array' is retained + * for that purpose. */ void zend_partial_create(zval *result, zend_class_entry *scope, zval *this_ptr, zend_function *function, uint32_t argc, zval *argv, zend_array *extra_named_params, const zend_array *named_positions, zend_string *declaring_filename, - const uint32_t *declaring_lineno_ptr, void **cache_slot, + const uint32_t *declaring_lineno_ptr, + const zend_op_array *declaring_op_array, void **cache_slot, zend_string *pfa_name, uint32_t flags, uint32_t const_args); void zend_partial_op_array_dtor(zval *pDest); diff --git a/Zend/zend_vm_def.h b/Zend/zend_vm_def.h index 351c67f2ab9f..0154dd297458 100644 --- a/Zend/zend_vm_def.h +++ b/Zend/zend_vm_def.h @@ -9923,7 +9923,7 @@ ZEND_VM_HANDLER(212, ZEND_CALLABLE_CONVERT_PARTIAL, CONST, CONST|UNUSED, NUM) (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) ? call->extra_named_params : NULL, OP2_TYPE == IS_CONST ? Z_ARRVAL_P(named_positions) : NULL, - EX(func)->op_array.filename, &opline->lineno, cache_slot, + EX(func)->op_array.filename, &opline->lineno, &EX(func)->op_array, cache_slot, pfa_name, opline->extended_value & ZEND_PARTIAL_FLAGS, const_args); if (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h index 96405697644f..ced719c677a7 100644 --- a/Zend/zend_vm_execute.h +++ b/Zend/zend_vm_execute.h @@ -8690,7 +8690,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_FUNC_CCONV ZEND_CALLABLE_CONV (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) ? call->extra_named_params : NULL, IS_CONST == IS_CONST ? Z_ARRVAL_P(named_positions) : NULL, - EX(func)->op_array.filename, &opline->lineno, cache_slot, + EX(func)->op_array.filename, &opline->lineno, &EX(func)->op_array, cache_slot, pfa_name, opline->extended_value & ZEND_PARTIAL_FLAGS, const_args); if (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { @@ -12063,7 +12063,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_FUNC_CCONV ZEND_CALLABLE_CONV (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) ? call->extra_named_params : NULL, IS_UNUSED == IS_CONST ? Z_ARRVAL_P(named_positions) : NULL, - EX(func)->op_array.filename, &opline->lineno, cache_slot, + EX(func)->op_array.filename, &opline->lineno, &EX(func)->op_array, cache_slot, pfa_name, opline->extended_value & ZEND_PARTIAL_FLAGS, const_args); if (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { @@ -61585,7 +61585,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_CCONV ZEND_CALLABLE_CONVERT_P (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) ? call->extra_named_params : NULL, IS_CONST == IS_CONST ? Z_ARRVAL_P(named_positions) : NULL, - EX(func)->op_array.filename, &opline->lineno, cache_slot, + EX(func)->op_array.filename, &opline->lineno, &EX(func)->op_array, cache_slot, pfa_name, opline->extended_value & ZEND_PARTIAL_FLAGS, const_args); if (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { @@ -64856,7 +64856,7 @@ static ZEND_OPCODE_HANDLER_RET ZEND_OPCODE_HANDLER_CCONV ZEND_CALLABLE_CONVERT_P (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) ? call->extra_named_params : NULL, IS_UNUSED == IS_CONST ? Z_ARRVAL_P(named_positions) : NULL, - EX(func)->op_array.filename, &opline->lineno, cache_slot, + EX(func)->op_array.filename, &opline->lineno, &EX(func)->op_array, cache_slot, pfa_name, opline->extended_value & ZEND_PARTIAL_FLAGS, const_args); if (ZEND_CALL_INFO(call) & ZEND_CALL_HAS_EXTRA_NAMED_PARAMS) { diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index 9330a85a6ea2..a5fa14a5fc76 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -2113,6 +2113,7 @@ const zend_op_array *zend_accel_pfa_cache_get( zend_op_array *zend_accel_compile_pfa(zend_ast *ast, zend_string *declaring_filename, const uint32_t *declaring_lineno_ptr, + const zend_op_array *declaring_op_array, const zend_function *called_function, zend_string *pfa_func_name, bool cacheable_in_shm) { @@ -2169,21 +2170,33 @@ zend_op_array *zend_accel_compile_pfa(zend_ast *ast, destroy_op_array(script_op_array); efree(script_op_array); + const zend_op_array *retained[2]; + uint32_t num_retained = 0; if ((called_function->common.fn_flags & ZEND_ACC_CLOSURE) && called_function->type == ZEND_USER_FUNCTION && called_function->op_array.refcount) { /* Extend the lifetime of the called opcodes if * the called function is a closure. * See comment in zend_accel_pfa_key(). */ - zend_op_array *copy = zend_arena_alloc(&CG(arena), sizeof(*copy)); - memcpy(copy, called_function, sizeof(*copy)); - function_add_ref((zend_function *) copy); - /* Reference the copy in op_array->dynamic_func_defs so that it's - * destroyed when op_array is destroyed. */ + retained[num_retained++] = &called_function->op_array; + } + if (declaring_op_array && declaring_op_array->refcount) { + /* The key contains a declaring opline address, which eval() code may free and reuse. */ + retained[num_retained++] = declaring_op_array; + } + if (num_retained) { + /* Copies in dynamic_func_defs are released with op_array. */ ZEND_ASSERT(!op_array->dynamic_func_defs && !op_array->num_dynamic_func_defs); - op_array->dynamic_func_defs = safe_emalloc(1, sizeof(*op_array->dynamic_func_defs), 0); - op_array->dynamic_func_defs[0] = copy; - op_array->num_dynamic_func_defs = 1; + op_array->dynamic_func_defs = safe_emalloc(num_retained, sizeof(*op_array->dynamic_func_defs), 0); + for (uint32_t i = 0; i < num_retained; i++) { + zend_op_array *copy = zend_arena_alloc(&CG(arena), sizeof(*copy)); + memcpy(copy, retained[i], sizeof(*copy)); + /* function_add_ref() ignores ZEND_EVAL_CODE */ + copy->type = ZEND_USER_FUNCTION; + function_add_ref((zend_function *) copy); + op_array->dynamic_func_defs[i] = copy; + } + op_array->num_dynamic_func_defs = num_retained; } zend_hash_add_new_ptr(&EG(partial_function_application_cache), key, op_array); diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index 5fd34441c402..5f8ebdd6f27f 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -353,6 +353,7 @@ const zend_op_array *zend_accel_pfa_cache_get( zend_op_array *zend_accel_compile_pfa(zend_ast *ast, zend_string *declaring_filename, const uint32_t *declaring_lineno_ptr, + const zend_op_array *declaring_op_array, const zend_function *called_function, zend_string *pfa_func_name, bool cacheable_in_shm); From ce6c564fca79d0568eb289f328bce6d4954fb44b Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Tue, 10 Nov 2026 20:40:59 +0100 Subject: [PATCH 2/3] Key const expr PFAs on CACHEABLE_IN_SHM, add tests Interned names don't imply SHM ASTs (e.g. file_cache_only), and names are not unique by content. --- NEWS | 5 ++- .../cache_freed_declaring_site_004.phpt | 16 +++++++ .../cache_freed_declaring_site_005.phpt | 42 +++++++++++++++++++ Zend/zend_ast.c | 4 +- Zend/zend_compile.c | 1 + 5 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 Zend/tests/partial_application/cache_freed_declaring_site_004.phpt create mode 100644 Zend/tests/partial_application/cache_freed_declaring_site_005.phpt diff --git a/NEWS b/NEWS index 4c3b30f0ba3f..5838e39b4f5e 100644 --- a/NEWS +++ b/NEWS @@ -26,8 +26,9 @@ PHP NEWS . Fixed use-after-free when the error handler releases a string while an "Illegal string offset" warning is raised for its offset in a read, ?? or nested isset()/empty(). (Jorg Sowa) - . Fixed partial function applications in eval'd code or class constants - reusing a stale cached function with wrong bound arguments. (Jorg Sowa) + . Fixed partial function applications in eval'd or included code, or in + class constants, reusing a stale cached function with wrong bound + arguments. (Jorg Sowa) - Curl: . Fixed curl_reset() retaining CURLOPT_PRIVATE after reset. (lacatoire) diff --git a/Zend/tests/partial_application/cache_freed_declaring_site_004.phpt b/Zend/tests/partial_application/cache_freed_declaring_site_004.phpt new file mode 100644 index 000000000000..aaa63866acae --- /dev/null +++ b/Zend/tests/partial_application/cache_freed_declaring_site_004.phpt @@ -0,0 +1,16 @@ +--TEST-- +PFAs in constants declared on the same line must not share a cache entry +--FILE-- + +--EXPECT-- +string(3) "1|x" +string(3) "y|2" +string(4) "3|k1" +string(4) "k2|4" diff --git a/Zend/tests/partial_application/cache_freed_declaring_site_005.phpt b/Zend/tests/partial_application/cache_freed_declaring_site_005.phpt new file mode 100644 index 000000000000..37212bfd1f0c --- /dev/null +++ b/Zend/tests/partial_application/cache_freed_declaring_site_005.phpt @@ -0,0 +1,42 @@ +--TEST-- +PFA in included file: request cache must not outlive the freed declaring op_array +--INI-- +opcache.enable_cli=0 +--FILE-- + +--CLEAN-- + +--EXPECT-- +Done diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index a331155dac46..7213abb959c8 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -1361,8 +1361,8 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_inner( if (uses_variadic_placeholder) { flags |= ZEND_PARTIAL_USES_VARIADIC_PLACEHOLDER; } - /* The cached PFA keeps a non-interned name alive, unlike a non-persistent AST */ - const uint32_t *cache_key = ZSTR_IS_INTERNED(fcc_ast->name) + /* A non-SHM AST may be freed, but the cached PFA keeps its name alive */ + const uint32_t *cache_key = (flags & ZEND_PARTIAL_CACHEABLE_IN_SHM) ? &ast->lineno : (const uint32_t *) fcc_ast->name; zend_partial_create(result, scope, &frame->This, fptr, ZEND_CALL_NUM_ARGS(frame), ZEND_CALL_ARG(frame, 1), diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 98732f060170..abed76366229 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -12292,6 +12292,7 @@ static void zend_compile_const_expr_fcc(zend_ast **ast_ptr) if (!is_fcc) { fcc->filename = zend_string_copy(CG(active_op_array)->filename); + /* Not unique by content, but its address is a cache key until persisted */ fcc->name = zend_compile_partial_name(CG(active_op_array), fcc->lineno); } From 31c377be475633a53762a8252c96ff2e9c57e05c Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Tue, 10 Nov 2026 22:01:28 +0100 Subject: [PATCH 3/3] Assert that const expr PFAs are not keyed on SHM names --- Zend/zend_ast.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c index 7213abb959c8..a4903c8fdf1b 100644 --- a/Zend/zend_ast.c +++ b/Zend/zend_ast.c @@ -1361,6 +1361,9 @@ static zend_result ZEND_FASTCALL zend_ast_evaluate_inner( if (uses_variadic_placeholder) { flags |= ZEND_PARTIAL_USES_VARIADIC_PLACEHOLDER; } + /* SHM names are shared by same-line PFAs, so they can't be a cache key */ + ZEND_ASSERT((flags & ZEND_PARTIAL_CACHEABLE_IN_SHM) + || !(GC_FLAGS(fcc_ast->name) & IS_STR_PERMANENT)); /* A non-SHM AST may be freed, but the cached PFA keeps its name alive */ const uint32_t *cache_key = (flags & ZEND_PARTIAL_CACHEABLE_IN_SHM) ? &ast->lineno : (const uint32_t *) fcc_ast->name;