Skip to content

Commit e217138

Browse files
MaxKellermanndevnexen
authored andcommitted
ext/opcache/jit/zend_jit_trace: add missing lock for EXIT_INVALIDATE
Commit 6c25413 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.
1 parent b26b758 commit e217138

File tree

2 files changed

+25
-12
lines changed

2 files changed

+25
-12
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ PHP NEWS
2525
. Fix inverted bailout value in zend_runtime_jit() (Max Kellermann).
2626
. Fix access to uninitialized variable in accel_preload(). (nielsdos)
2727
. Fix zend_jit_find_trace() crashes. (Max Kellermann)
28+
. Added missing lock for EXIT_INVALIDATE in zend_jit_trace_exit. (Max Kellermann)
2829

2930
- PHPDBG:
3031
. Fix undefined behaviour in phpdbg_load_module_or_extension(). (nielsdos)

ext/opcache/jit/zend_jit_trace.c

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8141,22 +8141,34 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num, zend_jit_registers_buf
81418141
t = &zend_jit_traces[num];
81428142
}
81438143

8144-
SHM_UNPROTECT();
8145-
zend_jit_unprotect();
8144+
zend_shared_alloc_lock();
81468145

81478146
jit_extension = (zend_jit_op_array_trace_extension*)ZEND_FUNC_INFO(t->op_array);
8148-
if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_LOOP) {
8149-
((zend_op*)(t->opline))->handler = (const void*)zend_jit_loop_trace_counter_handler;
8150-
} else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_ENTER) {
8151-
((zend_op*)(t->opline))->handler = (const void*)zend_jit_func_trace_counter_handler;
8152-
} else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_RETURN) {
8153-
((zend_op*)(t->opline))->handler = (const void*)zend_jit_ret_trace_counter_handler;
8147+
8148+
/* Checks under lock, just in case something has changed while we were waiting for the lock */
8149+
if (!(ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & (ZEND_JIT_TRACE_JITED|ZEND_JIT_TRACE_BLACKLISTED))) {
8150+
/* skip: not JIT-ed nor blacklisted */
8151+
} else if (ZEND_JIT_TRACE_NUM >= JIT_G(max_root_traces)) {
8152+
/* skip: too many root traces */
8153+
} else {
8154+
SHM_UNPROTECT();
8155+
zend_jit_unprotect();
8156+
8157+
if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_LOOP) {
8158+
((zend_op*)(t->opline))->handler = (const void*)zend_jit_loop_trace_counter_handler;
8159+
} else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_ENTER) {
8160+
((zend_op*)(t->opline))->handler = (const void*)zend_jit_func_trace_counter_handler;
8161+
} else if (ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags & ZEND_JIT_TRACE_START_RETURN) {
8162+
((zend_op*)(t->opline))->handler = (const void*)zend_jit_ret_trace_counter_handler;
8163+
}
8164+
ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags &=
8165+
ZEND_JIT_TRACE_START_LOOP|ZEND_JIT_TRACE_START_ENTER|ZEND_JIT_TRACE_START_RETURN;
8166+
8167+
zend_jit_protect();
8168+
SHM_PROTECT();
81548169
}
8155-
ZEND_OP_TRACE_INFO(t->opline, jit_extension->offset)->trace_flags &=
8156-
ZEND_JIT_TRACE_START_LOOP|ZEND_JIT_TRACE_START_ENTER|ZEND_JIT_TRACE_START_RETURN;
81578170

8158-
zend_jit_protect();
8159-
SHM_PROTECT();
8171+
zend_shared_alloc_unlock();
81608172

81618173
return 0;
81628174
}

0 commit comments

Comments
 (0)