Skip to content

dl() of module with aliased class crashes in shutdown #15367

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

Closed
bwoebi opened this issue Aug 12, 2024 · 5 comments
Closed

dl() of module with aliased class crashes in shutdown #15367

bwoebi opened this issue Aug 12, 2024 · 5 comments

Comments

@bwoebi
Copy link
Member

bwoebi commented Aug 12, 2024

Description

Reproducible with a shared build of zend_test.

==81087== Command: php -n -r dl("zend_test.so");
==81087== 
==81087== Invalid read of size 1
==81087==    at 0xA0FF80: clean_module_class (zend_API.c:3111)
==81087==    by 0xA1FE73: zend_hash_apply_with_argument (zend_hash.c:2124)
==81087==    by 0xA0FFE7: clean_module_classes (zend_API.c:3121)
==81087==    by 0xA100C3: module_destructor (zend_API.c:3158)
==81087==    by 0xA1073B: zend_post_deactivate_modules (zend_API.c:3284)
==81087==    by 0x9491DF: php_request_shutdown (main.c:1904)
==81087==    by 0xBA1D1B: do_cli (php_cli.c:1136)
==81087==    by 0xBA21B3: main (php_cli.c:1340)
==81087==  Address 0x893c950 is 0 bytes inside a block of size 512 free'd
==81087==    at 0x4848FE0: free (in /usr/lib/aarch64-linux-gnu/valgrind/vgpreload_memcheck-arm64-linux.so)
==81087==    by 0x9ECEC7: destroy_zend_class (zend_opcode.c:513)
==81087==    by 0xA1DA33: _zend_hash_del_el_ex (zend_hash.c:1488)
==81087==    by 0xA1DB3B: _zend_hash_del_el (zend_hash.c:1515)
==81087==    by 0xA1FEDB: zend_hash_apply_with_argument (zend_hash.c:2128)
==81087==    by 0xA0FFE7: clean_module_classes (zend_API.c:3121)
==81087==    by 0xA100C3: module_destructor (zend_API.c:3158)
==81087==    by 0xA1073B: zend_post_deactivate_modules (zend_API.c:3284)
==81087==    by 0x9491DF: php_request_shutdown (main.c:1904)
==81087==    by 0xBA1D1B: do_cli (php_cli.c:1136)
==81087==    by 0xBA21B3: main (php_cli.c:1340)
==81087==  Block was alloc'd at
==81087==    at 0x4847E4C: malloc (in /usr/lib/aarch64-linux-gnu/valgrind/vgpreload_memcheck-arm64-linux.so)
==81087==    by 0xA1095F: do_register_internal_class (zend_API.c:3312)
==81087==    by 0xA10CD7: zend_register_internal_class (zend_API.c:3387)
==81087==    by 0xA10B4F: zend_register_internal_class_ex (zend_API.c:3350)
==81087==    by 0x8A1BDD3: register_class__ZendTestClass (test_arginfo.h:541)
==81087==    by 0x8A253AF: zm_startup_zend_test (test.c:1163)
==81087==    by 0xA0D08F: zend_startup_module_ex (zend_API.c:2312)
==81087==    by 0x7EEEBB: php_load_extension (dl.c:257)
==81087==    by 0x7EEF87: php_dl (dl.c:300)
==81087==    by 0x7EE82F: zif_dl (dl.c:69)
==81087==    by 0xA49B2F: ZEND_DO_ICALL_SPEC_RETVAL_UNUSED_HANDLER (zend_vm_execute.h:1275)
==81087==    by 0xAD5087: execute_ex (zend_vm_execute.h:57212)

PHP Version

8.3.9

Operating System

No response

@cmb69
Copy link
Member

cmb69 commented Aug 13, 2024

Might be a manifestation of #9196.

@nielsdos
Copy link
Member

I believe this dl function should be deprecated as the functionality is fundamentally broken because it breaks engine constraints. Fixing it is a very big effort.

@cmb69
Copy link
Member

cmb69 commented Aug 23, 2024

@bwoebi, any thoughts about deprecating dl()?

Copy link

github-actions bot commented Sep 7, 2024

No feedback was provided. The issue is being suspended because we assume that you are no longer experiencing the problem. If this is not the case and you are able to provide the information that was requested earlier, please do so. Thank you.

@github-actions github-actions bot closed this as completed Sep 7, 2024
@cmb69 cmb69 closed this as not planned Won't fix, can't repro, duplicate, stale Sep 7, 2024
arnaud-lb added a commit that referenced this issue Mar 14, 2025
We destroy classes of dl()'ed modules in clean_module_classes(), during
shutdown. Child classes of a module use structures of the parent class (such as
inherited properties), which are destroyed earlier, so we have a use-after-free
when destroying a child class.

Here I destroy classes in reverse order, as it is done in zend_shutdown() for
persistent classes.

Fixes GH-17961
Fixes GH-15367
@arnaud-lb
Copy link
Member

Fixed by #17961

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

No branches or pull requests

4 participants