mirror of
https://github.com/php/php-src.git
synced 2025-08-15 21:48:51 +02:00
Fix GH-8979: Possible Memory Leak with SSL-enabled MySQL connections
The stream context inside `mysqlnd_vio::enable_ssl()` is leaking. In particular: when `php_stream_context_set()` get called the refcount of `context` is increased by 1, which means that `context` will now have a refcount of 2. Later on we remove the context from the stream by calling `php_stream_context_set(stream, NULL)` but that leaves our `context` with a refcount of 1, and therefore it's never destroyed. In my test case this yielded a leak of 1456 bytes per connection (but could be more depending on your settings ofc). Annoyingly, Valgrind doesn't find it because the context is still in the `EG(regular_list)` and will thus be destroyed at the end of the request. However, I still think this bug needs to be fixed because as the users in the issue report already mentioned: there can be long-running PHP scripts. Fix it by decreasing the refcount to transfer the ownership. Closes GH-10909.
This commit is contained in:
parent
90f5b2b4ff
commit
8930bf8c33
2 changed files with 8 additions and 0 deletions
4
NEWS
4
NEWS
|
@ -31,6 +31,10 @@ PHP NEWS
|
||||||
. Fixed bug GH-10521 (ftp_get/ftp_nb_get resumepos offset is maximum 10GB).
|
. Fixed bug GH-10521 (ftp_get/ftp_nb_get resumepos offset is maximum 10GB).
|
||||||
(nielsdos)
|
(nielsdos)
|
||||||
|
|
||||||
|
- MySQLnd:
|
||||||
|
. Fixed bug GH-8979 (Possible Memory Leak with SSL-enabled MySQL
|
||||||
|
connections). (nielsdos)
|
||||||
|
|
||||||
- Opcache:
|
- Opcache:
|
||||||
. Fixed build for macOS to cater with pkg-config settings. (David Carlier)
|
. Fixed build for macOS to cater with pkg-config settings. (David Carlier)
|
||||||
. Fixed bug GH-8065 (opcache.consistency_checks > 0 causes segfaults in
|
. Fixed bug GH-8065 (opcache.consistency_checks > 0 causes segfaults in
|
||||||
|
|
|
@ -561,6 +561,10 @@ MYSQLND_METHOD(mysqlnd_vio, enable_ssl)(MYSQLND_VIO * const net)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
php_stream_context_set(net_stream, context);
|
php_stream_context_set(net_stream, context);
|
||||||
|
/* php_stream_context_set() increases the refcount of context, but we just want to transfer ownership
|
||||||
|
* hence the need to decrease the refcount so the refcount will be equal to 1. */
|
||||||
|
ZEND_ASSERT(GC_REFCOUNT(context->res) == 2);
|
||||||
|
GC_DELREF(context->res);
|
||||||
if (php_stream_xport_crypto_setup(net_stream, STREAM_CRYPTO_METHOD_TLS_CLIENT, NULL) < 0 ||
|
if (php_stream_xport_crypto_setup(net_stream, STREAM_CRYPTO_METHOD_TLS_CLIENT, NULL) < 0 ||
|
||||||
php_stream_xport_crypto_enable(net_stream, 1) < 0)
|
php_stream_xport_crypto_enable(net_stream, 1) < 0)
|
||||||
{
|
{
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue