Skip to content

Fix mysqlnd pipe crash #13782

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.

Sign up for GitHub

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
wants to merge 6 commits into from
Closed

Fix mysqlnd pipe crash #13782

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6a937eb
Remove broken hack in mysqlnd_vio::close_stream
nielsdos Mar 21, 2024
e98c7ee
Fix memory leak in mysqlnd_vio::open_pipe
nielsdos Mar 21, 2024
c04565a
Factor common code out into mysqlnd_fixup_regular_list()
nielsdos Mar 21, 2024
d5e32cb
Fix crash in shutdown when using a named pipe with a persistent mysql…
nielsdos Mar 21, 2024
9a569d2
Use php_stream_auto_cleanup() consistently
nielsdos Mar 22, 2024
b30796f
Add an assertion for certainty
nielsdos Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 48 additions & 30 deletions ext/mysqlnd/mysqlnd_vio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,27 @@ MYSQLND_METHOD(mysqlnd_vio, network_write)(MYSQLND_VIO * const vio, const zend_u
}
/* }}} */

static void mysqlnd_fixup_regular_list(php_stream *net_stream)
{
/*
Streams are not meant for C extensions! Thus we need a hack. Every connected stream will
be registered as resource (in EG(regular_list). So far, so good. However, it won't be
unregistered until the script ends. So, we need to take care of that.
*/
dtor_func_t origin_dtor = EG(regular_list).pDestructor;
EG(regular_list).pDestructor = NULL;
zend_hash_index_del(&EG(regular_list), net_stream->res->handle);
EG(regular_list).pDestructor = origin_dtor;
efree(net_stream->res);
net_stream->res = NULL;
}

/* {{{ mysqlnd_vio::open_pipe */
static php_stream *
MYSQLND_METHOD(mysqlnd_vio, open_pipe)(MYSQLND_VIO * const vio, const MYSQLND_CSTRING scheme, const bool persistent,
MYSQLND_STATS * const conn_stats, MYSQLND_ERROR_INFO * const error_info)
{
unsigned int streams_options = 0;
dtor_func_t origin_dtor;
php_stream * net_stream = NULL;

DBG_ENTER("mysqlnd_vio::open_pipe");
Expand All @@ -132,16 +145,34 @@ MYSQLND_METHOD(mysqlnd_vio, open_pipe)(MYSQLND_VIO * const vio, const MYSQLND_CS
SET_CLIENT_ERROR(error_info, CR_CONNECTION_ERROR, UNKNOWN_SQLSTATE, "Unknown error while connecting");
DBG_RETURN(NULL);
}
/*
Streams are not meant for C extensions! Thus we need a hack. Every connected stream will
be registered as resource (in EG(regular_list). So far, so good. However, it won't be
unregistered until the script ends. So, we need to take care of that.
*/
origin_dtor = EG(regular_list).pDestructor;
EG(regular_list).pDestructor = NULL;
zend_hash_index_del(&EG(regular_list), net_stream->res->handle); /* ToDO: should it be res->handle, do streams register with addref ?*/
EG(regular_list).pDestructor = origin_dtor;
net_stream->res = NULL;

if (persistent) {
/* This is a similar hack as for mysqlnd_vio::open_tcp_or_unix.
* The main difference here is that we have no access to the hashed key.
* We can however perform a loop over the persistent resource list to find
* which one corresponds to our newly allocated stream.
* This loop is pretty cheap because it will normally either be the last entry or second to last entry
* in the list, depending on whether the socket connection itself is persistent or not.
* That's why we use a reverse loop. */
Bucket *bucket;
/* Use a bucket loop to make deletion cheap. */
ZEND_HASH_MAP_REVERSE_FOREACH_BUCKET(&EG(persistent_list), bucket) {
zend_resource *current_res = Z_RES(bucket->val);
if (current_res->ptr == net_stream) {
dtor_func_t origin_dtor = EG(persistent_list).pDestructor;
EG(persistent_list).pDestructor = NULL;
zend_hash_del_bucket(&EG(persistent_list), bucket);
EG(persistent_list).pDestructor = origin_dtor;
pefree(current_res, 1);
break;
}
} ZEND_HASH_FOREACH_END();
#if ZEND_DEBUG
php_stream_auto_cleanup(net_stream);
#endif
}

mysqlnd_fixup_regular_list(net_stream);

DBG_RETURN(net_stream);
}
Expand Down Expand Up @@ -205,6 +236,7 @@ MYSQLND_METHOD(mysqlnd_vio, open_tcp_or_unix)(MYSQLND_VIO * const vio, const MYS
zend_resource *le;

if ((le = zend_hash_str_find_ptr(&EG(persistent_list), hashed_details, hashed_details_len))) {
ZEND_ASSERT(le->ptr == net_stream);
origin_dtor = EG(persistent_list).pDestructor;
/*
in_free will let streams code skip destructing - big HACK,
Expand All @@ -218,22 +250,12 @@ MYSQLND_METHOD(mysqlnd_vio, open_tcp_or_unix)(MYSQLND_VIO * const vio, const MYS
}
#if ZEND_DEBUG
/* Shut-up the streams, they don't know what they are doing */
net_stream->__exposed = 1;
php_stream_auto_cleanup(net_stream);
#endif
mnd_sprintf_free(hashed_details);
}

/*
Streams are not meant for C extensions! Thus we need a hack. Every connected stream will
be registered as resource (in EG(regular_list). So far, so good. However, it won't be
unregistered until the script ends. So, we need to take care of that.
*/
origin_dtor = EG(regular_list).pDestructor;
EG(regular_list).pDestructor = NULL;
zend_hash_index_del(&EG(regular_list), net_stream->res->handle); /* ToDO: should it be res->handle, do streams register with addref ?*/
efree(net_stream->res);
net_stream->res = NULL;
EG(regular_list).pDestructor = origin_dtor;
mysqlnd_fixup_regular_list(net_stream);
DBG_RETURN(net_stream);
}
/* }}} */
Expand Down Expand Up @@ -652,15 +674,11 @@ MYSQLND_METHOD(mysqlnd_vio, close_stream)(MYSQLND_VIO * const net, MYSQLND_STATS
bool pers = net->persistent;
DBG_INF_FMT("Freeing stream. abstract=%p", net_stream->abstract);
/* We removed the resource from the stream, so pass FREE_RSRC_DTOR now to force
* destruction to occur during shutdown, because it won't happen through the resource. */
/* TODO: The EG(active) check here is dead -- check IN_SHUTDOWN? */
if (pers && EG(active)) {
* destruction to occur during shutdown, because it won't happen through the resource
* because we removed the resource from the EG resource list(s). */
if (pers) {
php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE_PERSISTENT | PHP_STREAM_FREE_RSRC_DTOR);
} else {
/*
otherwise we will crash because the EG(persistent_list) has been freed already,
before the modules are shut down
*/
php_stream_free(net_stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_RSRC_DTOR);
}
net->data->m.set_stream(net, NULL);
Expand Down