Skip to content

unregister INI entries and fix invalid read on shutdown#8042

Merged
TeBoring merged 1 commit intoprotocolbuffers:masterfrom
tony2001:unregister_ini_entries
Nov 25, 2020
Merged

unregister INI entries and fix invalid read on shutdown#8042
TeBoring merged 1 commit intoprotocolbuffers:masterfrom
tony2001:unregister_ini_entries

Conversation

@tony2001
Copy link
Copy Markdown
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.

5 participants