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

Deep recursion in zend_cfg.c causes segfault instead of error #14361

Closed
YuanchengJiang opened this issue May 29, 2024 · 3 comments
Closed

Deep recursion in zend_cfg.c causes segfault instead of error #14361

YuanchengJiang opened this issue May 29, 2024 · 3 comments

Comments

@YuanchengJiang
Copy link

Description

The following code:

<?php
error_reporting(E_ALL ^ E_DEPRECATED);

$binaryOperators = [
    "==",
    "!=",
    "===",
    "!==",
    "<",
    "<=",
    ">",
    ">=",
    "<=>",
    "+",
    "-",
    "*",
    "/",
    "%",
    "**",
    ".",
    "|",
    "&",
    "^",
    "or",
    "and",
    "xor",
    "||",
    "&&",
];
$unaryOperators = [
    "~",
    "-",
    "+",
    "!",
];

$input = [
    0,
    0.0,
    1,
    2,
    -1,
    2.0,
    2.1,
    -2.0,
    -2.1,
    PHP_INT_MAX,
    PHP_INT_MIN,
    PHP_INT_MAX * 2,
    PHP_INT_MIN * 2,
    INF,
    NAN,
    [],
    [1, 2],
    [1, 2, 3],
    [1 => 2, 0 => 1],
    [1, 'a' => 2],
    [1, 4],
    [1, 'a'],
    [1, 2 => 2],
    [1, [ 2 ]],
    null,
    false,
    true,
    "",
    " ",
    "banana",
    "Banana",
    "banan",
    "0",
    "200",
    "20",
    "20a",
    " \t\n\r\v\f20",
    "20  ",
    "2e1",
    "2e150",
    "9179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368",
    "-9179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368",
    "0.1",
    "-0.1",
    "1e-1",
    "-20",
    "-20.0",
    "0x14",
    (string) PHP_INT_MAX * 2,
    (string) PHP_INT_MIN * 2,
];

function makeParam($param) {
    if ($param === PHP_INT_MIN) {
        return "PHP_INT_MIN";
    }
    if ($param === PHP_INT_MAX) {
        return "PHP_INT_MAX";
    }
    if (is_string($param)) {
        return '"' . strtr($param, ["\t" => '\t', "\n" => '\n', "\r" => '\r', "\v" => '\v', "\f" => '\f', '$' => '\$', '"' => '\"']) . '"';
    }
    return "(" . str_replace("\n", "", var_export($param, true)) . ")";
}

$c = 0;
$f = 0;

function prepareBinaryLine($op1, $op2, $cmp, $operator) {
    $op1_p = makeParam($op1);
    $op2_p = makeParam($op2);

    $error = "echo '" . addcslashes("$op1_p $operator $op2_p", "\\'") . '\', "\n"; $f++;';

    $compare = "@($op1_p $operator $op2_p)";
    $line = "\$c++; ";
    try {
        $result = makeParam($cmp());
        $line .= "if (" . ($result === "(NAN)" ? "!is_nan($compare)" : "$compare !== $result") . ") { $error }";
    } catch (Throwable $e) {
        $msg = makeParam($e->getMessage());
        $line .= "try { $compare; $error } catch (Error \$e) { if (\$e->getMessage() !== $msg) { $error } }";
    }
    return $line;
}
function prepareUnaryLine($op, $cmp, $operator) {
    $op_p = makeParam($op);

    $error = "echo '" . addcslashes("$operator $op_p", "\\'") . '\', "\n"; $f++;';

    $compare = "@($operator $op_p)";
    $line = "\$c++; ";
    try {
        $result = makeParam($cmp());
        $line .= "if (" . ($result === "(NAN)" ? "!is_nan($compare)" : "$compare !== $result") . ") { $error }";
    } catch (Throwable $e) {
        $msg = makeParam($e->getMessage());
        $line .= "try { $compare; $error } catch (Error \$e) { if (\$e->getMessage() !== $msg) { $error } }";
    }
    return $line;
}

$filename = __DIR__ . DIRECTORY_SEPARATOR . 'compare_binary_operands_temp.php';
$file = fopen($filename, "w");

fwrite($file, "<?php\n");

foreach ($input as $left) {
    foreach ($input as $right) {
        foreach ($binaryOperators as $operator) {
            $line = prepareBinaryLine($left, $right, function() use ($left, $right, $operator) {
                return eval("return @(\$left $operator \$right);");
            }, $operator);
            fwrite($file, $line . "\n");
        }
    }
}
foreach ($input as $right) {
    foreach ($unaryOperators as $operator) {
        $line = prepareUnaryLine($right, function() use ($right, $operator) {
            return eval("return @($operator \$right);");
        }, $operator);
        fwrite($file, $line . "\n");
    }
}

fclose($file);

include $filename;

if($c === 0) {
    echo "Completely failed\n";
} else {
    echo "Failed: $f\n";
}
?>

This is the php code from Zend/tests/runtime_compile_time_binary_operands.phpt

Resulted in this output:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==4050933==ERROR: AddressSanitizer: stack-overflow on address 0x7fff10533fff (pc 0x55698135c573 bp 0x7fff10534310 sp 0x7fff10533fb0 T0)
    #0 0x55698135c573 in zend_mark_reachable /WorkSpace/phptest/php-src/Zend/Optimizer/zend_cfg.c:89:16
    #1 0x55698135c839 in zend_mark_reachable /WorkSpace/phptest/php-src/Zend/Optimizer/zend_cfg.c:100:6
    #2 0x55698135c839 in zend_mark_reachable /WorkSpace/phptest/php-src/Zend/Optimizer/zend_cfg.c:100:6
    #3 0x55698135c839 in zend_mark_reachable /WorkSpace/phptest/php-src/Zend/Optimizer/zend_cfg.c:100:6
    #4 0x55698135c839 in zend_mark_reachable /WorkSpace/phptest/php-src/Zend/Optimizer/zend_cfg.c:100:6
...
    #246 0x55698135c839 in zend_mark_reachable /WorkSpace/phptest/php-src/Zend/Optimizer/zend_cfg.c:100:6

SUMMARY: AddressSanitizer: stack-overflow /WorkSpace/phptest/php-src/Zend/Optimizer/zend_cfg.c:89:16 in zend_mark_reachable

or

AddressSanitizer:DEADLYSIGNAL
=================================================================
==3759932==ERROR: AddressSanitizer: BUS on unknown address (pc 0x7f7d6b8ab860 bp 0x7ffc84b5c620 sp 0x7ffc84b5c5a8 T0)
==3759932==The signal is caused by a READ memory access.
==3759932==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.
AddressSanitizer:DEADLYSIGNAL
AddressSanitizer: nested bug in the same thread, aborting.

But I expected this output instead:

Failed: 0

To reproduce:

/php-src/sapi/cli/php  -n -c '/php-src/tmp-php.ini' -d "memory_limit=256M" -d "opcache.fast_shutdown=0" -d "opcache.file_update_protection=0" -d "opcache.revalidate_freq=0" -d "opcache.jit_hot_loop=1" -d "opcache.jit_hot_func=1" -d "opcache.jit_hot_return=1" -d "opcache.jit_hot_side_exit=1" -d "opcache.jit_max_root_traces=100000" -d "opcache.jit_max_side_traces=100000" -d "opcache.jit_max_exit_counters=100000" -d "opcache.protect_memory=1" -d "extension_dir=/php-src/modules/" -d "zend_extension=/php-src/modules/opcache.so" -d "session.auto_start=0" -d "opcache.enable=1" -d "opcache.enable_cli=1" -d "opcache.jit=tracing" -d "opcache.optimization_level=0x000000a0" -f ./test.php

I did not meet such infinite loop before. I am not sure if it is expected. Please kindly close it if everything works well.

PHP Version

nightly

Operating System

ubuntu 22.04

@nielsdos
Copy link
Member

nielsdos commented Jun 1, 2024

It's not an infinite loop, it's just very deep recursion, it does terminate with enough stack space.
By default my ulimit -s is set to 8192 which doesn't trigger this, but if I lower it to e.g. 1024 I do get the crash.
We could fix this the same way that some related recursion issues were fixed: by checking the stack size limit (that feature was implemented in #9104). It would be as simple as calling zend_call_stack_overflowed and bailing out. It's probably not expensive to do that even in the recursion, but not sure if it's worth it.
cc @arnaud-lb What do you think?

@arnaud-lb
Copy link
Member

arnaud-lb commented Jun 1, 2024

In my opinion it's worth it, especially if the overhead is small, as it improves the user experience by making it easier to track the origin of the issue.

NB: under ASAN the default value of zend.reserved_stack_size is usually too small for the stack limit to work properly.

@nielsdos
Copy link
Member

nielsdos commented Jun 1, 2024

OK, I'll prepare a patch for 8.3 sometime soon (currently busy with the TSRM ARM stuff)

nielsdos added a commit to nielsdos/php-src that referenced this issue Jun 1, 2024
Recreating this over and over is pointless, cache this as well.
Fixes phpGH-14361.
@nielsdos nielsdos changed the title Infinite Loop in zend_cfg.c:100:6 Deep recursion in zend_cfg.c causes segfault instead of error Jun 1, 2024
nielsdos added a commit to nielsdos/php-src that referenced this issue Jun 1, 2024
… of error

Use the same stack limit check already used elsewhere in compilation.
nielsdos added a commit to nielsdos/php-src that referenced this issue Jun 1, 2024
… of error

Use the same stack limit check already used elsewhere in compilation.
nielsdos added a commit to nielsdos/php-src that referenced this issue Jun 2, 2024
… of error

Use the same stack limit check already used elsewhere in compilation.
nielsdos added a commit to nielsdos/php-src that referenced this issue Jun 2, 2024
… of error

Building the CFG for JIT or optimizations can run into deep recursion,
and depending on the stack limit cause a segfault. This patch uses the
stack limit check to fail out of those cases. This prevents a segfault.

We use this check elsewhere in the compiler as well. However, in this
case we just make optimizations or JIT fail without aborting the
application such that code can still execute.

The attached test case will succeed both without and with this patch, it
is mainly intended to test if propagating failure doesn't cause crashes.
To reproduce the issue, set a low stack limit using `ulimit -s` and run
the original test case from php#14361
nielsdos added a commit to nielsdos/php-src that referenced this issue Jun 2, 2024
… of error

Building the CFG for JIT or optimizations can run into deep recursion,
and depending on the stack limit cause a segfault. This patch uses the
stack limit check to fail out of those cases. This prevents a segfault.

We use this check elsewhere in the compiler as well. However, in this
case we just make optimizations or JIT fail without aborting the
application such that code can still execute.

The attached test case will succeed both without and with this patch, it
is mainly intended to test if propagating failure doesn't cause crashes.
To reproduce the issue, set a low stack limit using `ulimit -s` and run
the original test case from php#14361
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment