From f4fb77ed61b38fa1b85b7d32e611bc3e4be64cc7 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 25 Dec 2024 00:26:14 +0100 Subject: [PATCH] Fix GH-17257: UBSAN warning in ext/opcache/jit/zend_jit_vm_helpers.c EX(opline) / opline can be stale if the IP is not stored, like in this case on a trace enter. We always need to make sure that the opline is up to date to make sure we don't use stale data. Closes GH-17260. --- NEWS | 2 ++ ext/opcache/jit/zend_jit_internal.h | 1 + ext/opcache/jit/zend_jit_ir.c | 9 +++++++-- ext/opcache/jit/zend_jit_vm_helpers.c | 14 ++++++++++++-- ext/opcache/tests/jit/gh17257.phpt | 25 +++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 ext/opcache/tests/jit/gh17257.phpt diff --git a/NEWS b/NEWS index 86c13fdc475..898dda52a4d 100644 --- a/NEWS +++ b/NEWS @@ -74,6 +74,8 @@ PHP NEWS . Fixed bug GH-17151 (Incorrect RC inference of op1 of FETCH_OBJ and INIT_METHOD_CALL). (Dmitry, ilutov) . Fixed bug GH-17246 (GC during SCCP causes segfault). (Dmitry) + . Fixed bug GH-17257 (UBSAN warning in ext/opcache/jit/zend_jit_vm_helpers.c). + (nielsdos, Dmitry) - PCNTL: . Fix memory leak in cleanup code of pcntl_exec() when a non stringable diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h index 4fe07222d8f..ce245bdb198 100644 --- a/ext/opcache/jit/zend_jit_internal.h +++ b/ext/opcache/jit/zend_jit_internal.h @@ -231,6 +231,7 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_func_counter_helper(ZEND_OPCODE_H ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_loop_counter_helper(ZEND_OPCODE_HANDLER_ARGS); void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D); +void ZEND_FASTCALL zend_jit_copy_extra_args_helper_no_skip_recv(EXECUTE_DATA_D); bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D); void ZEND_FASTCALL zend_jit_undefined_long_key(EXECUTE_DATA_D); void ZEND_FASTCALL zend_jit_undefined_long_key_ex(zend_long key EXECUTE_DATA_DC); diff --git a/ext/opcache/jit/zend_jit_ir.c b/ext/opcache/jit/zend_jit_ir.c index 6b74621517f..e14345215f6 100644 --- a/ext/opcache/jit/zend_jit_ir.c +++ b/ext/opcache/jit/zend_jit_ir.c @@ -3050,6 +3050,7 @@ static void zend_jit_setup_disasm(void) REGISTER_HELPER(zend_jit_undefined_long_key_ex); REGISTER_HELPER(zend_jit_undefined_string_key); REGISTER_HELPER(zend_jit_copy_extra_args_helper); + REGISTER_HELPER(zend_jit_copy_extra_args_helper_no_skip_recv); REGISTER_HELPER(zend_jit_vm_stack_free_args_helper); REGISTER_HELPER(zend_free_extra_named_params); REGISTER_HELPER(zend_jit_free_call_frame); @@ -10110,6 +10111,7 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen } } } else { + ir_ref helper; if (!trace || (trace->op == ZEND_JIT_TRACE_END && trace->stop == ZEND_JIT_TRACE_STOP_INTERPRETER)) { ir_ref ip; @@ -10123,11 +10125,14 @@ static int zend_jit_do_fcall(zend_jit_ctx *jit, const zend_op *opline, const zen ip = ir_LOAD_A(ir_ADD_OFFSET(func_ref, offsetof(zend_op_array, opcodes))); } jit_LOAD_IP(jit, ip); + helper = ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper); + } else { + helper = ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper_no_skip_recv); } if (GCC_GLOBAL_REGS) { - ir_CALL(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper)); + ir_CALL(IR_VOID, helper); } else { - ir_CALL_1(IR_VOID, ir_CONST_FC_FUNC(zend_jit_copy_extra_args_helper), jit_FP(jit)); + ir_CALL_1(IR_VOID, helper, jit_FP(jit)); } } } else { diff --git a/ext/opcache/jit/zend_jit_vm_helpers.c b/ext/opcache/jit/zend_jit_vm_helpers.c index 08370fc5323..f4d261cd753 100644 --- a/ext/opcache/jit/zend_jit_vm_helpers.c +++ b/ext/opcache/jit/zend_jit_vm_helpers.c @@ -120,7 +120,7 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_leave_func_helper(EXECUTE_DATA_D) } } -void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) +static void ZEND_FASTCALL zend_jit_copy_extra_args_helper_ex(bool skip_recv EXECUTE_DATA_DC) { zend_op_array *op_array = &EX(func)->op_array; @@ -130,7 +130,7 @@ void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) zval *end, *src, *dst; uint32_t type_flags = 0; - if (EXPECTED((op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS) == 0)) { + if (skip_recv && EXPECTED((op_array->fn_flags & ZEND_ACC_HAS_TYPE_HINTS) == 0)) { /* Skip useless ZEND_RECV and ZEND_RECV_INIT opcodes */ #ifdef HAVE_GCC_GLOBAL_REGS opline += first_extra_arg; @@ -166,6 +166,16 @@ void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) } } +void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D) +{ + zend_jit_copy_extra_args_helper_ex(true EXECUTE_DATA_CC); +} + +void ZEND_FASTCALL zend_jit_copy_extra_args_helper_no_skip_recv(EXECUTE_DATA_D) +{ + zend_jit_copy_extra_args_helper_ex(false EXECUTE_DATA_CC); +} + bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D) { zend_execute_data *call = (zend_execute_data *) opline; diff --git a/ext/opcache/tests/jit/gh17257.phpt b/ext/opcache/tests/jit/gh17257.phpt new file mode 100644 index 00000000000..177e00decf9 --- /dev/null +++ b/ext/opcache/tests/jit/gh17257.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-17257 (SEGV ext/opcache/jit/zend_jit_vm_helpers.c) +--EXTENSIONS-- +opcache +--INI-- +opcache.jit_buffer_size=32M +opcache.jit=1254 +opcache.jit_hot_func=1 +--FILE-- + +--EXPECT-- +Done