archive/php-src
1
0
Fork 0
mirror of https://github.com/php/php-src.git synced 2025-08-15 21:48:51 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Fix GH-13834: Applying non-zero offset 36 to null pointer in zend_jit.c (#13846)

Browse source 
* Fix GH-13834: Applying non-zero offset 36 to null pointer in zend_jit.c

ssa_op can be NULL in function JIT. Doing pointer arithmetic on a NULL
pointer is undefined behaviour. Undefined behaviour can be dangerous
because the optimizer may assume then that the variable is not actually
NULL.

To solve this:
1. Add ADVANCE_SSA_OP() to safely add an offset to ssa_op in zend_jit.c
2. For inference, add an extra offset argument to the helper functions.

To reproduce this, use Clang (not GCC) on a test like
sapi/cli/tests/gh12363.phpt (or other tests also work).

* Remove -fno-sanitize=pointer-overflow flag from CI

* Fix NULL pointer offsets added to the stack_map

* Fix an offset add on a potentially NULL ssa->ops

* Fix NULL pointer arithmetic in zend_range_info()

* Address review comments
This commit is contained in:
Niels Dossche 2024-04-01 13:37:15 +02:00 • committed by GitHub
parent c3f5bbde2a
commit 00c6d538ab
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 29 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

2
.github/workflows/push.yml vendored
View file

@ -122,7 +122,7 @@ jobs:
configurationParameters: >- configurationParameters: >-
--${{ matrix.debug && 'enable' || 'disable' }}-debug --${{ matrix.debug && 'enable' || 'disable' }}-debug
--${{ matrix.zts && 'enable' || 'disable' }}-zts --${{ matrix.zts && 'enable' || 'disable' }}-zts
${{ matrix.asan && 'CFLAGS="-fsanitize=undefined,address -fno-sanitize=pointer-overflow -DZEND_TRACK_ARENA_ALLOC" LDFLAGS="-fsanitize=undefined,address -fno-sanitize=pointer-overflow" CC=clang-16 CXX=clang++-16' || '' }} ${{ matrix.asan && 'CFLAGS="-fsanitize=undefined,address -DZEND_TRACK_ARENA_ALLOC" LDFLAGS="-fsanitize=undefined,address" CC=clang-16 CXX=clang++-16' || '' }}
skipSlow: ${{ matrix.asan }} skipSlow: ${{ matrix.asan }}
- name: make - name: make
run: make -j$(/usr/bin/nproc) >/dev/null run: make -j$(/usr/bin/nproc) >/dev/null

12
Zend/Optimizer/zend_func_info.c
View file

@ -58,16 +58,16 @@ static uint32_t zend_range_info(const zend_call_info *call_info, const zend_ssa
&& ssa && ssa
&& !(ssa->cfg.flags & ZEND_SSA_TSSA)) { && !(ssa->cfg.flags & ZEND_SSA_TSSA)) {
zend_op_array *op_array = call_info->caller_op_array; zend_op_array *op_array = call_info->caller_op_array;
uint32_t t1 = _ssa_op1_info(op_array, ssa, call_info->arg_info[0].opline, uint32_t t1 = _ssa_op1_info(op_array, ssa, op_array->opcodes,
&ssa->ops[call_info->arg_info[0].opline - op_array->opcodes]); ssa->ops ? &ssa->ops[call_info->arg_info[0].opline - op_array->opcodes] : NULL);
uint32_t t2 = _ssa_op1_info(op_array, ssa, call_info->arg_info[1].opline, uint32_t t2 = _ssa_op1_info(op_array, ssa, op_array->opcodes,
&ssa->ops[call_info->arg_info[1].opline - op_array->opcodes]); ssa->ops ? &ssa->ops[call_info->arg_info[1].opline - op_array->opcodes] : NULL);
uint32_t t3 = 0; uint32_t t3 = 0;
uint32_t tmp = MAY_BE_RC1 | MAY_BE_ARRAY; uint32_t tmp = MAY_BE_RC1 | MAY_BE_ARRAY;
if (call_info->num_args == 3) { if (call_info->num_args == 3) {
t3 = _ssa_op1_info(op_array, ssa, call_info->arg_info[2].opline, t3 = _ssa_op1_info(op_array, ssa, op_array->opcodes,
&ssa->ops[call_info->arg_info[2].opline - op_array->opcodes]); ssa->ops ? &ssa->ops[call_info->arg_info[2].opline - op_array->opcodes] : NULL);
} }
if ((t1 & MAY_BE_STRING) && (t2 & MAY_BE_STRING)) { if ((t1 & MAY_BE_STRING) && (t2 & MAY_BE_STRING)) {
tmp |= MAY_BE_ARRAY_OF_LONG | MAY_BE_ARRAY_OF_DOUBLE | MAY_BE_ARRAY_OF_STRING; tmp |= MAY_BE_ARRAY_OF_LONG | MAY_BE_ARRAY_OF_DOUBLE | MAY_BE_ARRAY_OF_STRING;

4
Zend/Optimizer/zend_inference.c
View file

@ -5185,7 +5185,7 @@ ZEND_API bool zend_may_throw_ex(const zend_op *opline, const zend_ssa_op *ssa_op
return 0; return 0;
case ZEND_ASSIGN_DIM: case ZEND_ASSIGN_DIM:
if ((opline+1)->op1_type == IS_CV) { if ((opline+1)->op1_type == IS_CV) {
if (_ssa_op1_info(op_array, ssa, opline+1, ssa_op+1) & MAY_BE_UNDEF) { if (OP1_DATA_INFO() & MAY_BE_UNDEF) {
return 1; return 1;
} }
} }
@ -5196,7 +5196,7 @@ ZEND_API bool zend_may_throw_ex(const zend_op *opline, const zend_ssa_op *ssa_op
return 1; return 1;
} }
if ((opline+1)->op1_type == IS_CV) { if ((opline+1)->op1_type == IS_CV) {
if (_ssa_op1_info(op_array, ssa, opline+1, ssa_op+1) & MAY_BE_UNDEF) { if (OP1_DATA_INFO() & MAY_BE_UNDEF) {
return 1; return 1;
} }
} }

8
Zend/Optimizer/zend_inference.h
View file

@ -197,13 +197,13 @@ DEFINE_SSA_OP_DEF_INFO(result)
#define OP1_INFO() (_ssa_op1_info(op_array, ssa, opline, ssa_op)) #define OP1_INFO() (_ssa_op1_info(op_array, ssa, opline, ssa_op))
#define OP2_INFO() (_ssa_op2_info(op_array, ssa, opline, ssa_op)) #define OP2_INFO() (_ssa_op2_info(op_array, ssa, opline, ssa_op))
#define OP1_DATA_INFO() (_ssa_op1_info(op_array, ssa, (opline+1), (ssa_op+1))) #define OP1_DATA_INFO() (_ssa_op1_info(op_array, ssa, (opline+1), ssa_op ? (ssa_op+1) : NULL))
#define OP2_DATA_INFO() (_ssa_op2_info(op_array, ssa, (opline+1), (ssa_op+1))) #define OP2_DATA_INFO() (_ssa_op2_info(op_array, ssa, (opline+1), ssa_op ? (ssa_op+1) : NULL))
#define RES_USE_INFO() (_ssa_result_info(op_array, ssa, opline, ssa_op)) #define RES_USE_INFO() (_ssa_result_info(op_array, ssa, opline, ssa_op))
#define OP1_DEF_INFO() (_ssa_op1_def_info(op_array, ssa, opline, ssa_op)) #define OP1_DEF_INFO() (_ssa_op1_def_info(op_array, ssa, opline, ssa_op))
#define OP2_DEF_INFO() (_ssa_op2_def_info(op_array, ssa, opline, ssa_op)) #define OP2_DEF_INFO() (_ssa_op2_def_info(op_array, ssa, opline, ssa_op))
#define OP1_DATA_DEF_INFO() (_ssa_op1_def_info(op_array, ssa, (opline+1), (ssa_op+1))) #define OP1_DATA_DEF_INFO() (_ssa_op1_def_info(op_array, ssa, (opline+1), ssa_op ? (ssa_op+1) : NULL))
#define OP2_DATA_DEF_INFO() (_ssa_op2_def_info(op_array, ssa, (opline+1), (ssa_op+1))) #define OP2_DATA_DEF_INFO() (_ssa_op2_def_info(op_array, ssa, (opline+1), ssa_op ? (ssa_op+1) : NULL))
#define RES_INFO() (_ssa_result_def_info(op_array, ssa, opline, ssa_op)) #define RES_INFO() (_ssa_result_def_info(op_array, ssa, opline, ssa_op))
static zend_always_inline bool zend_add_will_overflow(zend_long a, zend_long b) { static zend_always_inline bool zend_add_will_overflow(zend_long a, zend_long b) {

19
ext/opcache/jit/zend_jit.c
View file

@ -243,6 +243,11 @@ static int zend_jit_is_constant_cmp_long_long(const zend_op *opline,
return 0; return 0;
} }
#define ADVANCE_SSA_OP(ssa_op, offset) \
do { \
if (ssa_op) ssa_op += offset; \
} while (0)
static int zend_jit_needs_call_chain(zend_call_info *call_info, uint32_t b, const zend_op_array *op_array, zend_ssa *ssa, const zend_ssa_op *ssa_op, const zend_op *opline, int call_level, zend_jit_trace_rec *trace) static int zend_jit_needs_call_chain(zend_call_info *call_info, uint32_t b, const zend_op_array *op_array, zend_ssa *ssa, const zend_ssa_op *ssa_op, const zend_op *opline, int call_level, zend_jit_trace_rec *trace)
{ {
int skip; int skip;
@ -250,7 +255,7 @@ static int zend_jit_needs_call_chain(zend_call_info *call_info, uint32_t b, cons
if (trace) { if (trace) {
zend_jit_trace_rec *p = trace; zend_jit_trace_rec *p = trace;
ssa_op++; ADVANCE_SSA_OP(ssa_op, 1);
while (1) { while (1) {
if (p->op == ZEND_JIT_TRACE_VM) { if (p->op == ZEND_JIT_TRACE_VM) {
switch (p->opline->opcode) { switch (p->opline->opcode) {
@ -305,7 +310,7 @@ static int zend_jit_needs_call_chain(zend_call_info *call_info, uint32_t b, cons
return 1; return 1;
} }
} }
ssa_op += zend_jit_trace_op_len(opline); ADVANCE_SSA_OP(ssa_op, zend_jit_trace_op_len(opline));
} else if (p->op == ZEND_JIT_TRACE_ENTER || } else if (p->op == ZEND_JIT_TRACE_ENTER ||
p->op == ZEND_JIT_TRACE_BACK || p->op == ZEND_JIT_TRACE_BACK ||
p->op == ZEND_JIT_TRACE_END) { p->op == ZEND_JIT_TRACE_END) {
@ -319,7 +324,7 @@ static int zend_jit_needs_call_chain(zend_call_info *call_info, uint32_t b, cons
const zend_op *end = op_array->opcodes + op_array->last; const zend_op *end = op_array->opcodes + op_array->last;
opline++; opline++;
ssa_op++; ADVANCE_SSA_OP(ssa_op, 1);
skip = (call_level == 1); skip = (call_level == 1);
while (opline != end) { while (opline != end) {
if (!skip) { if (!skip) {
@ -381,7 +386,7 @@ static int zend_jit_needs_call_chain(zend_call_info *call_info, uint32_t b, cons
return 0; return 0;
} }
opline++; opline++;
ssa_op++; ADVANCE_SSA_OP(ssa_op, 1);
} }
return 1; return 1;
@ -395,7 +400,7 @@ static int zend_jit_needs_call_chain(zend_call_info *call_info, uint32_t b, cons
} }
opline++; opline++;
ssa_op++; ADVANCE_SSA_OP(ssa_op, 1);
skip = (call_level == 1); skip = (call_level == 1);
while (opline != end) { while (opline != end) {
if (skip) { if (skip) {
@ -421,7 +426,7 @@ static int zend_jit_needs_call_chain(zend_call_info *call_info, uint32_t b, cons
} }
} }
opline++; opline++;
ssa_op++; ADVANCE_SSA_OP(ssa_op, 1);
} }
return 0; return 0;
@ -441,7 +446,7 @@ static uint32_t skip_valid_arguments(const zend_op_array *op_array, zend_ssa *ss
if (ZEND_TYPE_IS_SET(arg_info->type)) { if (ZEND_TYPE_IS_SET(arg_info->type)) {
if (ZEND_TYPE_IS_ONLY_MASK(arg_info->type)) { if (ZEND_TYPE_IS_ONLY_MASK(arg_info->type)) {
zend_op *opline = call_info->arg_info[num_args].opline; zend_op *opline = call_info->arg_info[num_args].opline;
zend_ssa_op *ssa_op = &ssa->ops[opline - op_array->opcodes]; zend_ssa_op *ssa_op = ssa->ops ? &ssa->ops[opline - op_array->opcodes] : NULL;
uint32_t type_mask = ZEND_TYPE_PURE_MASK(arg_info->type); uint32_t type_mask = ZEND_TYPE_PURE_MASK(arg_info->type);
if ((OP1_INFO() & (MAY_BE_ANY|MAY_BE_UNDEF)) & ~type_mask) { if ((OP1_INFO() & (MAY_BE_ANY|MAY_BE_UNDEF)) & ~type_mask) {
break; break;

4
ext/opcache/jit/zend_jit_ir.c
View file

@ -15241,7 +15241,7 @@ static int zend_jit_switch(zend_jit_ctx *jit, const zend_op *opline, const zend_
jit->b = -1; jit->b = -1;
} }
} else { } else {
zend_ssa_op *ssa_op = &ssa->ops[opline - op_array->opcodes]; zend_ssa_op *ssa_op = ssa->ops ? &ssa->ops[opline - op_array->opcodes] : NULL;
uint32_t op1_info = OP1_INFO(); uint32_t op1_info = OP1_INFO();
zend_jit_addr op1_addr = OP1_ADDR(); zend_jit_addr op1_addr = OP1_ADDR();
const zend_op *default_opline = ZEND_OFFSET_TO_OPLINE(opline, opline->extended_value); const zend_op *default_opline = ZEND_OFFSET_TO_OPLINE(opline, opline->extended_value);
@ -16302,7 +16302,7 @@ static int zend_jit_trace_start(zend_jit_ctx *jit,
if (parent) { if (parent) {
int i; int i;
int parent_vars_count = parent->exit_info[exit_num].stack_size; int parent_vars_count = parent->exit_info[exit_num].stack_size;
zend_jit_trace_stack *parent_stack = zend_jit_trace_stack *parent_stack = parent_vars_count == 0 ? NULL :
parent->stack_map + parent->stack_map +
parent->exit_info[exit_num].stack_offset; parent->exit_info[exit_num].stack_offset;

4
ext/opcache/jit/zend_jit_trace.c
View file

@ -2779,7 +2779,7 @@ static zend_jit_reg_var* zend_jit_trace_allocate_registers(zend_jit_trace_rec *t
zend_jit_trace_stack *stack; zend_jit_trace_stack *stack;
uint32_t parent_vars_count = parent_trace ? uint32_t parent_vars_count = parent_trace ?
zend_jit_traces[parent_trace].exit_info[exit_num].stack_size : 0; zend_jit_traces[parent_trace].exit_info[exit_num].stack_size : 0;
zend_jit_trace_stack *parent_stack = parent_trace ? zend_jit_trace_stack *parent_stack = parent_vars_count ?
zend_jit_traces[parent_trace].stack_map + zend_jit_traces[parent_trace].stack_map +
zend_jit_traces[parent_trace].exit_info[exit_num].stack_offset : NULL; zend_jit_traces[parent_trace].exit_info[exit_num].stack_offset : NULL;
checkpoint = zend_arena_checkpoint(CG(arena)); checkpoint = zend_arena_checkpoint(CG(arena));
@ -8386,7 +8386,7 @@ int ZEND_FASTCALL zend_jit_trace_exit(uint32_t exit_num, zend_jit_registers_buf
/* Deoptimization of VM stack state */ /* Deoptimization of VM stack state */
uint32_t i; uint32_t i;
uint32_t stack_size = t->exit_info[exit_num].stack_size; uint32_t stack_size = t->exit_info[exit_num].stack_size;
zend_jit_trace_stack *stack = t->stack_map + t->exit_info[exit_num].stack_offset; zend_jit_trace_stack *stack = stack_size ? t->stack_map + t->exit_info[exit_num].stack_offset : NULL;
if (t->exit_info[exit_num].flags & ZEND_JIT_EXIT_RESTORE_CALL) { if (t->exit_info[exit_num].flags & ZEND_JIT_EXIT_RESTORE_CALL) {
zend_execute_data *call = (zend_execute_data *)regs->gpr[ZREG_RX]; zend_execute_data *call = (zend_execute_data *)regs->gpr[ZREG_RX];