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

Nullify references once not used to prevent double-frees #73

Merged

Conversation

mikhainin
Copy link

Basically, it looks self-explaining.

In a couple of places, we check if the value is NULL and free() it.
However, if we start/stop the profiler a couple of times, the next time we (potentially) may not allow the value but still will try to free it and corrupt memory.

The issue with XHPROF_G(root) happened on our production, it was corrupting memory in another place (as the freed() chunk was given to another code), the problem with names[i] can be seen if we run tests having PHP built with --enable-debug.

Could you please take a look and let me know if there's something I can improve?

@longxinH
Copy link
Owner

Can you provide an example of repeated release of memory? Or an error message?

@longxinH longxinH merged commit 676885b into longxinH:master Dec 13, 2022
@mikhainin
Copy link
Author

Hi, it's unfortunately quite complicated to reproduce - I failed to isolate the example, the crash happened on our production application (about 100,000 source files).

Something similar we can get if we build PHP with Address Sanitizer:

--with-iconv=/opt/homebrew/opt/libiconv --prefix=/opt/phpbuild --enable-debug --enable-debug-assertions --disable-phar --with-pcre-jit --enable-fpm --enable-mbstring=all CFLAGS="-O0 -ggdb3 -fno-omit-frame-pointer -fPIC -fsanitize=address -DZEND_TRACK_ARENA_ALLOC"

Build the extension with this PHP:

/opt/phpbuild/bin/phpize
./configure --with-php-config=/opt/phpbuild/bin/php-config

And then run the following code:

class Test {
};


for ($i = 0; $i < 1000000; ++ $i) {
  xhprof_enable();

  xhprof_disable();
  preg_match("/test$i/", "string");
  $a = new Test;
}

The output looks confusing as the problem is reported in another place. And I couldn't repeat it using USE_ZEND_ALLOC=0 with ASAN

AddressSanitizer:DEADLYSIGNAL
=================================================================
==7178==ERROR: AddressSanitizer: BUS on unknown address (pc 0x010122f29c00 bp 0x00016ef48090 sp 0x00016ef47f80 T0)
==7178==The signal is caused by a UNKNOWN memory access.
==7178==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.
    #0 0x10122f29c00  (<unknown module>)
    #1 0x10122f684 in php_efree_pcre_cache php_pcre.c:171
    #2 0x101c4866c in zend_hash_destroy zend_hash.c:1577
    #3 0x10122ac70 in zm_deactivate_pcre php_pcre.c:518
    #4 0x101c15cf0 in zend_deactivate_modules zend_API.c:2711
    #5 0x1019cd7ec in php_request_shutdown main.c:1849
    #6 0x101f9f748 in do_cli php_cli.c:1111
    #7 0x101f9c4e0 in main php_cli.c:1341
    #8 0x103635088 in start+0x204 (dyld:arm64e+0x5088)

==7178==Register values:
 x[0] = 0x000000011669a3bf   x[1] = 0x0000000000000000   x[2] = 0x0000000000000008   x[3] = 0x000000000000013c  
 x[4] = 0x0000000000000000   x[5] = 0x0000000000000000   x[6] = 0x000000016af50000   x[7] = 0x0000000000000001  
 x[8] = 0x0000010122f29c00   x[9] = 0x000000011669a3bf  x[10] = 0x0000000000000008  x[11] = 0x0000000102d30560  
x[12] = 0xc0c06443ff0618c7  x[13] = 0x00000000ffffffff  x[14] = 0x000000016ef480c0  x[15] = 0x000000016ef482e0  
x[16] = 0x000000018c7ef0fc  x[17] = 0x0000000103de0720  x[18] = 0x0000000000000000  x[19] = 0x00000001036e4060  
x[20] = 0x0000000101f9b740  x[21] = 0x0000000103690070  x[22] = 0x0000000000000000  x[23] = 0x0000000000000000  
x[24] = 0x0000000000000000  x[25] = 0x0000000000000000  x[26] = 0x0000000000000000  x[27] = 0x0000000000000000  
x[28] = 0x0000000000000000     fp = 0x000000016ef48090     lr = 0x0000000100ff54bc     sp = 0x000000016ef47f80  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: BUS (<unknown module>) 
==7178==ABORTING

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants