Skip to content

Fix segmentation fault in Randomizer::getBytes() if a user engine throws #9055

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

Merged

Conversation

TimWolla
Copy link
Member

I'm a bit surprised that the exception check also fixes the SEGV in the exit() case. Does exit() use an exception internally or is my fix not actually correct and just happens to work for whatever reason?

/cc @zeriyoshi

This fixes:

    ==374077== Use of uninitialised value of size 8
    ==374077==    at 0x532B06: generate (engine_user.c:39)
    ==374077==    by 0x533F71: zim_Random_Randomizer_getBytes (randomizer.c:152)
    ==374077==    by 0x7F581D: ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (zend_vm_execute.h:1885)
    ==374077==    by 0x8725BE: execute_ex (zend_vm_execute.h:55930)
    ==374077==    by 0x877DB4: zend_execute (zend_vm_execute.h:60253)
    ==374077==    by 0x7B0FD4: zend_execute_scripts (zend.c:1770)
    ==374077==    by 0x6F1647: php_execute_script (main.c:2535)
    ==374077==    by 0x937DA4: do_cli (php_cli.c:964)
    ==374077==    by 0x938C3A: main (php_cli.c:1333)
    ==374077==
    ==374077== Invalid read of size 8
    ==374077==    at 0x532B06: generate (engine_user.c:39)
    ==374077==    by 0x533F71: zim_Random_Randomizer_getBytes (randomizer.c:152)
    ==374077==    by 0x7F581D: ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER (zend_vm_execute.h:1885)
    ==374077==    by 0x8725BE: execute_ex (zend_vm_execute.h:55930)
    ==374077==    by 0x877DB4: zend_execute (zend_vm_execute.h:60253)
    ==374077==    by 0x7B0FD4: zend_execute_scripts (zend.c:1770)
    ==374077==    by 0x6F1647: php_execute_script (main.c:2535)
    ==374077==    by 0x937DA4: do_cli (php_cli.c:964)
    ==374077==    by 0x938C3A: main (php_cli.c:1333)
    ==374077==  Address 0x11 is not stack'd, malloc'd or (recently) free'd
@zeriyoshi
Copy link
Contributor

@TimWolla
I was missing some considerations. Thanks!

P.S.: The CI is probably a random failure, my macOS tests succeeded.

@zeriyoshi
Copy link
Contributor

zeriyoshi commented Jul 20, 2022

I'm a bit surprised that the exception check also fixes the SEGV in the exit() case. Does exit() use an exception internally or is my fix not actually correct and just happens to work for whatever reason?

Hmmm. I am not sure about this either. I will find time to check the stack trace during SEGV.

@kocsismate
Copy link
Member

Does exit() use an exception internally or is my fix not actually correct and just happens to work for whatever reason?

Yes, it uses an exception: #5768 Discussion: https://externals.io/message/107497

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Not sure if we need the exit test, but it doesn't hurt either.

@TimWolla TimWolla merged commit 998ede7 into php:master Jul 20, 2022
@TimWolla TimWolla deleted the random-randomizer-getBytes-user-throws branch July 20, 2022 15:32
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.

4 participants