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

unregister INI entries and fix invalid read on shutdown #8042

Merged
merged 1 commit into from Nov 25, 2020

Conversation

tony2001
Copy link
Contributor

At the moment protobuf PHP extension registers an INI entry and doesn't unregister it on shtudown.
This causes invalid read if you use dl("protobuf.so") because the interned string allocated in REGISTER_INI_ENTRIES() is still present in the list of interned strings on PHP shutdown, but the extension itself is already unloaded:

==30061== Invalid read of size 4
==30061==    at 0x7420EE: zend_string_release_ex (zend_string.h:284)
==30061==    by 0x7424A5: free_ini_entry (zend_ini.c:78)
==30061==    by 0x732D05: zend_hash_destroy (zend_hash.c:1546)
==30061==    by 0x7425A1: zend_ini_dtor (zend_ini.c:113)
==30061==    by 0x742582: zend_ini_shutdown (zend_ini.c:106)
==30061==    by 0x686692: php_module_shutdown (main.c:2543)
==30061==    by 0x7F36DD: main (php_cli.c:1377)
==30061==  Address 0x81d63f4 is 4 bytes inside a block of size 72 free'd
==30061==    at 0x4C2F50B: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30061==    by 0x7585D9: free (zend_signal.c:109)
==30061==    by 0x757B0F: _str_dtor (zend_string.c:60)
==30061==    by 0x732CC7: zend_hash_destroy (zend_hash.c:1541)
==30061==    by 0x75834E: zend_interned_strings_deactivate (zend_string.c:307)
==30061==    by 0x68575A: php_request_shutdown (main.c:1991)
==30061==    by 0x7F2F26: do_cli (php_cli.c:1129)
==30061==    by 0x7F367B: main (php_cli.c:1362)
==30061==  Block was alloc'd at
==30061==    at 0x4C2E2DF: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==30061==    by 0x758507: malloc (zend_signal.c:91)
==30061==    by 0x6E481A: __zend_malloc (zend_alloc.c:2976)
==30061==    by 0x75745F: zend_string_alloc (zend_string.h:133)
==30061==    by 0x7574FC: zend_string_init (zend_string.h:155)
==30061==    by 0x7582E7: zend_string_init_interned_request (zend_string.c:293)
==30061==    by 0x742846: zend_register_ini_entries (zend_ini.c:230)
==30061==    by 0xA58B9FE: zm_startup_protobuf (protobuf.c:290)
==30061==    by 0x7241DA: zend_startup_module_ex (zend_API.c:1868)
==30061==    by 0x5CE3C1: php_load_extension (dl.c:232)
==30061==    by 0x5CE47C: php_dl (dl.c:253)
==30061==    by 0x5CDD83: zif_dl (dl.c:64)

Unregistering the INI entries properly frees this interned string in the correct way and fixes this issue.

@TeBoring TeBoring merged commit 51b620a into protocolbuffers:master Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants