krakjoe / uopz

User Operations for Zend
Other
356 stars 47 forks source link

uopz_find_hook segfault with PHP 8.0 #145

Closed rlerdorf closed 3 years ago

rlerdorf commented 3 years ago

I am seeing a segfault with PHP 8.0 and uopz when calling uopz_call_user_func_array() that hits a __call() method on a tricky wrapped object. The wrapper is a test helper that changes the access level of private and protected methods if they have been marked as accessible for testing.

To reproduce, grab uopz_test.tar.gz, composer install and run vendor/bin/phpunit tests/accObjTest.php with PHP 8.0 and uopz, of course. It passes with PHP 7.4 and strangely enough, it also passes with PHP 8.1. With PHP 8.0.8 I am seeing a crash at the function->common.function_name check because function is 0x0. (recompiled with -O0 to verify that)

GNU gdb (Debian 10.1-1.7) 10.1.90.20210103-git
Core was generated by `php vendor/bin/phpunit tests/accObjTest.php'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 uopz_find_hook (function=<optimized out>) at /home/rasmus/src/uopz-73a6b2d85548e09702a60a35cfa1df2e94902247/src/hook.c:128
128 if (!function->common.function_name) {

(gdb) bt
#0 uopz_find_hook (function=<optimized out>) at /home/rasmus/src/uopz-73a6b2d85548e09702a60a35cfa1df2e94902247/src/hook.c:128
#1 0x00007fdb69a0c141 in zif_uopz_call_user_func_array (execute_data=0x7fdb698193c0, return_value=0x7ffef9be6610)
at /home/rasmus/src/uopz-73a6b2d85548e09702a60a35cfa1df2e94902247/src/util.c:285
#2 0x000055d883236a45 in ZEND_DO_FCALL_SPEC_RETVAL_UNUSED_HANDLER () at /home/rasmus/php-src/Zend/zend_vm_execute.h:1757
#3 0x000055d88362b663 in execute_ex (ex=0x0) at /home/rasmus/php-src/Zend/zend_vm_execute.h:54240
#4 0x000055d883633df4 in zend_execute (op_array=0x7fdb69862100, return_value=0x0)
at /home/rasmus/php-src/Zend/zend_vm_execute.h:58475
#5 0x000055d8835cf2fb in zend_execute_scripts (type=1770099360, type@entry=8, retval=retval@entry=0x0,
file_count=file_count@entry=3) at /home/rasmus/php-src/Zend/zend.c:1680
#6 0x000055d88356d8ba in php_execute_script (primary_file=primary_file@entry=0x7ffef9be8bd0)
at /home/rasmus/php-src/main/main.c:2524
#7 0x000055d883657975 in do_cli (argc=3, argv=0x55d885058930) at /home/rasmus/php-src/sapi/cli/php_cli.c:949
#8 0x000055d88324d028 in main (argc=3, argv=0x55d885058930) at /home/rasmus/php-src/sapi/cli/php_cli.c:1337
(gdb) l
123 uopz_hook_t uopz_find_hook(zend_function function) { /* {{{ /
124     zend_string key;
125     uopz_hook_t uhook;
126     HashTable hooks;
127
128     if (!function->common.function_name) {
129         return NULL;
130     }
131
132     if (function->common.scope) {

(gdb) dump_bt executor_globals.current_execute_data
[0x7fdb698193c0] call_user_func_array(array(2)[0x7fdb69819410], array(1)[0x7fdb69819420]) [internal function]
[0x7fdb698192a0] accObjTest->test_segfault() /home/rasmus/test/tests/accObjTest.php:11
[0x7fdb69818d70] PHPUnit\Framework\TestCase->runTest() /home/rasmus/test/vendor/phpunit/phpunit/src/Framework/TestCase.php:1527
[0x7fdb698185d0] PHPUnit\Framework\TestCase->runBare() /home/rasmus/test/vendor/phpunit/phpunit/src/Framework/TestCase.php:1133
[0x7fdb69817380] PHPUnit\Framework\TestResult->run(object[0x7fdb698173d0])
/home/rasmus/test/vendor/phpunit/phpunit/src/Framework/TestResult.php:722
[0x7fdb69816720] PHPUnit\Framework\TestCase->run(object[0x7fdb69816770])
/home/rasmus/test/vendor/phpunit/phpunit/src/Framework/TestCase.php:885
[0x7fdb69816000] PHPUnit\Framework\TestSuite->run(object[0x7fdb69816050])
/home/rasmus/test/vendor/phpunit/phpunit/src/Framework/TestSuite.php:677
[0x7fdb69813920] PHPUnit\TextUI\TestRunner->run(object[0x7fdb69813970], reference, array(0)[0x7fdb69813990], true)
/home/rasmus/test/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:670
[0x7fdb69813540] PHPUnit\TextUI\Command->run(array(2)[0x7fdb69813590], true)
/home/rasmus/test/vendor/phpunit/phpunit/src/TextUI/Command.php:143
[0x7fdb69813430] PHPUnit\TextUI\Command->main() /home/rasmus/test/vendor/phpunit/phpunit/src/TextUI/Command.php:96
[0x7fdb69813020] (main) /home/rasmus/test/vendor/phpunit/phpunit/phpunit:76
krakjoe commented 3 years ago

I'm having a hard time reproducing this:

krakjoe@Fiji:/opt/src/uopz_test$ php -v
PHP 8.0.8 (cli) (built: Jul 28 2021 09:25:18) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.8, Copyright (c) Zend Technologies
    with Zend OPcache v8.0.8, Copyright (c), by Zend Technologies
krakjoe@Fiji:/opt/src/uopz_test$ php -n vendor/bin/phpunit tests/accObjTest.php
PHPUnit 9.5.7 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 00:00.003, Memory: 4.00 MB

OK (1 test, 1 assertion)
krakjoe@Fiji:/opt/src/uopz_test$ php vendor/bin/phpunit tests/accObjTest.php
PHPUnit 9.5.7 by Sebastian Bergmann and contributors.

.                                                                   1 / 1 (100%)

Time: 00:00.007, Memory: 4.00 MB

OK (1 test, 1 assertion)

I've also tried tip of 8.0 branch, and debug builds, both with and without opcache ...

Any more clues ?

krakjoe commented 3 years ago

Here is a stab in the dark ...

diff --git a/src/util.c b/src/util.c
index 8f8d2cd..8ebbfdf 100644
--- a/src/util.c
+++ b/src/util.c
@@ -218,7 +218,11 @@ static void uopz_callers_shutdown(void) { /* {{{ */
        uopz_caller_switch(&uopz_call_user_func_array_ptr->handler, &zend_call_user_func_array_ptr->handler);
 } /* }}} */

-#define UOPZ_CALL_HOOKS(variadic) \
+#define UOPZ_CALL_HOOKS(variadic) do { \
+       if (!fcc.function_handler) { \
+               break; \
+       } \
+       \
        { \
                uopz_hook_t *uhook = uopz_find_hook(fcc.function_handler); \
                \
@@ -243,8 +247,8 @@ static void uopz_callers_shutdown(void) { /* {{{ */
                        ZVAL_COPY(return_value, &ureturn->value); \
                        return; \
                } \
-       } while (0)
-
+       } while (0); \
+} while (0)
 /* {{{ proto mixed uopz_call_user_func(callable function, ... args) */
 PHP_FUNCTION(uopz_call_user_func) {
        zval retval;

Any good ?

krakjoe commented 3 years ago

I managed to reproduce, the above patch was committed and should fix it: Magic may nullify handler, I'm surprised we didn't see this before.

rlerdorf commented 3 years ago

Thanks for the quick fix. I just realized now when I went to check the other PHP versions that I didn't have uopz enabled correctly for those. So ignore the part about 7.4 and 8.1 not being affected from my original report.