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.
php · Dec 26, 2024 · f4fb77e · f4fb77e
1 parent 956576b
commit f4fb77e
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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--
+<?php
+function get_const() {
+}
+function test() {
+    call_user_func('get_const', 1); // need an extra arg to trigger the issue
+}
+function main(){
+    for ($i = 0; $i < 10; $i++) {
+        test();
+    }
+    echo "Done\n";
+}
+main();
+?>
+--EXPECT--
+Done