Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT packed type guard crash #17577

Open
YuanchengJiang opened this issue Jan 26, 2025 · 1 comment · May be fixed by #17584
Open

JIT packed type guard crash #17577

YuanchengJiang opened this issue Jan 26, 2025 · 1 comment · May be fixed by #17584

Comments

@YuanchengJiang
Copy link

Description

The following code:

<?php
$a = array(
array(1,2,3),
-PHP_INT_MAX-1,
);
$var_cnt = count($a);
function my_dump($var) {
}
foreach($a as $a) {
for ($i = 0; $i < $var_cnt; $i++) {
my_dump($a[$i]);
}
}

Resulted in this output:

=================================================================
==2945471==ERROR: AddressSanitizer: SEGV on unknown address (pc 0x0000496f3928 bp 0x7ffe12994080 sp 0x7ffe12993f30 T0)
==2945471==The signal is caused by a READ memory access.
==2945471==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
LLVMSymbolizer: error reading file: No such file or directory
    #0 0x496f3928  (/dev/zero (deleted)+0x8000928)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/dev/zero (deleted)+0x8000928) 
==2945471==ABORTING

To reproduce:

-d "opcache.jit_hot_func=1" -d "zend_extension=/home/phpfuzz/WorkSpace/flowfusion/php-src/modules/opcache.so" -d "opcache.enable_cli=1" -d "opcache.jit=1254"

PHP Version

nightly

Operating System

No response

@nielsdos
Copy link
Member

nielsdos commented Jan 26, 2025

Slightly simplified:

<?php
$a = array(
    array(1,2,3),
    0,
);
function my_dump($var) {
}
foreach($a as $b) {
    for ($i = 0; $i < 1; $i++) {
        my_dump($b[$i]);
    }
}

Apparently, it %rax is 0, causing a NULL page deref when the type check for IS_LONG happens:

TRACE-1$/run/media/niels/MoreData/php-src-multitasking/x.php$9:
    49464b20:	subq $0x30, %rsp
    49464b24:	movabsq $EG(jit_trace_num), %rax
    49464b2e:	movl $1, (%rax)
    49464b34:	movq 0x60(%r14), %rax ; %rax = Z_PTR_P(`$b`), however $b is IS_LONG with value of 0, so %rax==0
    49464b38:	testl $4, 8(%rax) ; guard for zend_jit_packed_guard
    49464b3f:	jne .L4
    49464b41:	jmp jit$$trace_exit_0

The problem is that a IS_ARRAY type check is missing when emitting the packed guard.
If you were to change the 0 in $a to another value, you can see a dereference on that address+8.

@nielsdos nielsdos changed the title JIT 1254 SEGV JIT packed type guard crash Jan 26, 2025
nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 26, 2025
When a guard check is created for a variable to check if it's a packed array,
it is possible that there was no prior type check for that variable.
This happens in the global scope for example when the variable aliases.
In the test, this causes a dereference of address 8 because the integer
element in `$a` is interpreted as an array address.
This patch adds a type check if a prior one was not inserted, which
should only be possible is NO_ALIAS is set (hence the assertion).
In this case we also cannot set the stack type nor clear the
MAY_BE_GUARD flag due to the aliasing.
@nielsdos nielsdos linked a pull request Jan 26, 2025 that will close this issue
nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 26, 2025
When a guard check is created for a variable to check if it's a packed array,
it is possible that there was no prior type check for that variable.
This happens in the global scope for example when the variable aliases.
In the test, this causes a dereference of address 8 because the integer
element in `$a` is interpreted as an array address.
This patch adds a type check if a prior one was not inserted.
In the aliasing case we also cannot set the stack type nor clear the
MAY_BE_GUARD flag.
nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 26, 2025
When a guard check is created for a variable to check if it's a packed array,
it is possible that there was no prior type check for that variable.
This happens in the global scope for example when the variable aliases.
In the test, this causes a dereference of address 8 because the integer
element in `$a` is interpreted as an array address.
This patch adds a type check if a prior one was not inserted.
nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 26, 2025
When a guard check is created for a variable to check if it's a packed array,
it is possible that there was no prior type check for that variable.
This happens in the global scope for example when the variable aliases.
In the test, this causes a dereference of address 8 because the integer
element in `$a` is interpreted as an array address.

This patch adds a check to see if the zval type is an array.
If we were not able to determine or guard the type then we also cannot know the array is packed.
nielsdos added a commit to nielsdos/php-src that referenced this issue Jan 26, 2025
When a guard check is created for a variable to check if it's a packed array,
it is possible that there was no prior type check for that variable.
This happens in the global scope for example when the variable aliases.
In the test, this causes a dereference of address 8 because the integer
element in `$a` is interpreted as an array address.

This patch adds a check to see if the guard is handled.
If we were not able to determine or guard the type then we also cannot know the array is packed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants