From c5cccc8b04aef985ca38fe62d5446854ebb734e1 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 19 Dec 2022 22:35:22 +0100 Subject: [PATCH] ext/opcache/jit/zend_jit_trace: add missing lock for EXIT_INVALIDATE Commit 6c254131831 added the flag ZEND_JIT_EXIT_INVALIDATE which resets the trace handlers in zend_jit_trace_exit(), but forgot to lock the shared memory section. This could cause another worker process who still saw the ZEND_JIT_TRACE_JITED flag to schedule ZEND_JIT_TRACE_STOP_LINK, but when it arrived at the ZEND_JIT_DEBUG_TRACE_STOP, the handler was already reverted by the first worker process and thus zend_jit_find_trace() fails. This in turn generated a bogus jump offset in the JITed code, crashing the PHP process. --- ext/opcache/jit/zend_jit_trace.c | 36 +++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index fc42add8bf1e..56b01e7daca0 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -8141,22 +8141,34 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num, zend_jit_registers_buf t = &zend_jit_traces[num]; } - SHM_UNPROTECT(); - zend_jit_unprotect(); + zend_shared_alloc_lock(); jit_extension = (zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(t->op_array); - if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_LOOP) { - ((zend_op*)(t->opline))->handler = (const void*)zend_jit_loop_trace_counter_handler; - } else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_ENTER) { - ((zend_op*)(t->opline))->handler = (const void*)zend_jit_func_trace_counter_handler; - } else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_RETURN) { - ((zend_op*)(t->opline))->handler = (const void*)zend_jit_ret_trace_counter_handler; + + /* Checks under lock, just in case something has changed while we were waiting for the lock */ + if (!(ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & (ZEND_JIT_TRACE_JITED|ZEND_JIT_TRACE_BLACKLISTED))) { + /* skip: not JIT-ed nor blacklisted */ + } else if (ZEND_JIT_TRACE_NUM >= JIT_G(max_root_traces)) { + /* skip: too many root traces */ + } else { + SHM_UNPROTECT(); + zend_jit_unprotect(); + + if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_LOOP) { + ((zend_op*)(t->opline))->handler = (const void*)zend_jit_loop_trace_counter_handler; + } else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_ENTER) { + ((zend_op*)(t->opline))->handler = (const void*)zend_jit_func_trace_counter_handler; + } else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_RETURN) { + ((zend_op*)(t->opline))->handler = (const void*)zend_jit_ret_trace_counter_handler; + } + ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags &= + ZEND_JIT_TRACE_START_LOOP|ZEND_JIT_TRACE_START_ENTER|ZEND_JIT_TRACE_START_RETURN; + + zend_jit_protect(); + SHM_PROTECT(); } - ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags &= - ZEND_JIT_TRACE_START_LOOP|ZEND_JIT_TRACE_START_ENTER|ZEND_JIT_TRACE_START_RETURN; - zend_jit_protect(); - SHM_PROTECT(); + zend_shared_alloc_unlock(); return 0; }