-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
Segmentation fault under high loads in openssl_encrypt in ZTS build (FrankenPHP) #13648
Comments
Some extra information: FrankenPHP static builds are created using @crazywhalecc's static-php-cli, which uses Musl under the hood. |
does it build it statically with OpenSSL as well or is that linked as shared lib? What OpenSSL version is used? |
I've done a bit of digging, I still don't know exactly what the underlying problem is, but here are my guesses so far:
Line 7736 in 8595bea
EVP_CipherInit_ex() (Line 7553 in 8595bea
Line 7596 in 8595bea
Line 7686 in 8595bea
EVP_CIPHER_CTX_reset() .
Under heavy loads (this crash is only triggered during load tests), Another unrelated thing: Take all of this with a grain of salt as I know nothing about OpenSSL and this code is a bit complicated. |
@bukka OpenSSL is compiled statically as well, the latest version if used. |
Calling |
It might be irrelevant but how the toolchain has been built might count here too, see topic here. |
@devnexen This build uses the default options for static-php-cli: https://github.com/dunglas/frankenphp/blob/main/build-static.sh#L140 |
So I have been looking through this properly. It took me longest to actually create the build with OpenSSL debug symbols and recreate the issue. It was recreatable only on my old Intel laptop (couldn't re-create on my more powerful AMD desktop). The backtrace with all symbols looked like this
It says that it is failing on de-allocation fetched cipher The cipher context is however reset after freeing which can be seen here: https://github.com/openssl/openssl/blob/openssl-3.2.1/crypto/evp/evp_enc.c#L46-L48 so there doesn't seem any way to keep the freed context cipher around. I have been also checking if there is a way to maybe just partially initialize the context in The actual cipher type is set here: https://github.com/openssl/openssl/blob/openssl-3.2.1/crypto/evp/evp_enc.c#L1589 . The The PHP code does not seem to have any issue either except the unnecessary EVP_CIPHER_CTX_reset in https://github.com/php/php-src/blob/php-8.3.3/ext/openssl/openssl.c#L7700-L7701 . Removing that won't change much though as internally Another possibility is a problem in allocator. I would suggest to try to replace allocator as using MUSL one is not ideal anyway for highly multi-threaded application. It might be worth to put some effort into using jemalloc or simillar and see if it fixes the issue. |
I created a build of FrankenPHP that uses https://github.com/microsoft/mimalloc instead of mallogng (dunglas/frankenphp#666), and there is a similar problem (but not exactly the same).
|
A user of FrankenPHP reported a crash that is a bit different but may be related (dunglas/frankenphp#676 (comment)).
|
I think I'm running into this as well but with a different stack trace, if that helps:
Running a FrankenPHP v1.1.5 amd64 static build with added Of note, I cannot reproduce using |
Description
Code using
openssl_encrpyt()
causes segmentation faults under high loads with static ZTS builds of PHP.For instance, this part of Laravel triggers the bug: https://github.com/laravel/framework/blob/10.x/src/Illuminate/Encryption/Encrypter.php#L102-L105
Here is a backtrace gathered with GDB:
Backtrace
Several Laravel users have confirmed the issue at laravel/octane#791.
A reproducer is available in the linked PR.
Using a static build of FrankenPHP with debug symbols like the one available here https://github.com/dunglas/frankenphp/actions/runs/8206010369?pr=635 (
frankenphp-linux-x86_64-debug
) always triggers the error on Linux/amd64. I'm not able to reproduce on Apple Silicon (both on macOS and Linux).PHP Version
PHP 8.3.3
Operating System
Ubuntu 22.04.4 LTS
The text was updated successfully, but these errors were encountered: