Fix ZTS crashes with persistent resources in modules (#13381)

On shutdown in ZTS the following happens:
- https://github.com/php/php-src/blob/master/Zend/zend.c#L1124-L1125
  gets executed. This destroys global persistent resources and destroys
  the modules. Furthermore, the modules are unloaded too.
- Further down, `ts_free_id(executor_globals_id)` gets executed, which
  calls `executor_globals_dtor`. This function destroys persistent
  resources for each thread.

Notice that in the last step, the modules that the persistent resource
belong to may already have been destroyed. This means that accessing
globals will cause a crash (I previously fixed this with ifdef magic),
or when the module is dynamically loaded we'll try jumping to a
destructor that is no longer loaded in memory. These scenarios cause
crashes.

It's not possible to move the `ts_free_id` call upwards, because that
may break assumptions of callers, and furthermore this would deallocate
the executor globals structure, which means that any access to those
will cause a segfault.

This patch adds a new API to the TSRM that allows running a callback on
a certain resource type. We use this API to destroy the persistent
resources in all threads prior to the module destruction, and keep the
rest of the resource dtor intact.

I verified this fix on Apache with postgres, both dynamically and
statically.

Fixes GH-12974.
This commit is contained in:
Niels Dossche 2024-02-13 21:43:03 +01:00 committed by GitHub
parent be34b96975
commit 5941cdaaad
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 38 additions and 17 deletions

View file

@ -576,6 +576,27 @@ void ts_free_id(ts_rsrc_id id)
TSRM_ERROR((TSRM_ERROR_LEVEL_CORE, "Successfully freed resource id %d", id)); TSRM_ERROR((TSRM_ERROR_LEVEL_CORE, "Successfully freed resource id %d", id));
}/*}}}*/ }/*}}}*/
TSRM_API void ts_apply_for_id(ts_rsrc_id id, void (*cb)(void *))
{
int rsrc_id = TSRM_UNSHUFFLE_RSRC_ID(id);
tsrm_mutex_lock(tsmm_mutex);
if (tsrm_tls_table && resource_types_table) {
for (int i = 0; i < tsrm_tls_table_size; i++) {
tsrm_tls_entry *p = tsrm_tls_table[i];
while (p) {
if (p->count > rsrc_id && p->storage[rsrc_id]) {
cb(p->storage[rsrc_id]);
}
p = p->next;
}
}
}
tsrm_mutex_unlock(tsmm_mutex);
}
/* /*
* Utility Functions * Utility Functions

View file

@ -104,6 +104,9 @@ TSRM_API void ts_free_thread(void);
/* deallocates all occurrences of a given id */ /* deallocates all occurrences of a given id */
TSRM_API void ts_free_id(ts_rsrc_id id); TSRM_API void ts_free_id(ts_rsrc_id id);
/* Runs a callback on all resources of the given id.
* The caller is responsible for ensuring the underlying resources don't data-race. */
TSRM_API void ts_apply_for_id(ts_rsrc_id id, void (*cb)(void *));
/* Debug support */ /* Debug support */
#define TSRM_ERROR_LEVEL_ERROR 1 #define TSRM_ERROR_LEVEL_ERROR 1

View file

@ -826,13 +826,19 @@ static void executor_globals_ctor(zend_executor_globals *executor_globals) /* {{
} }
/* }}} */ /* }}} */
static void executor_globals_dtor(zend_executor_globals *executor_globals) /* {{{ */ static void executor_globals_persistent_list_dtor(void *storage)
{ {
zend_ini_dtor(executor_globals->ini_directives); zend_executor_globals *executor_globals = storage;
if (&executor_globals->persistent_list != global_persistent_list) { if (&executor_globals->persistent_list != global_persistent_list) {
zend_destroy_rsrc_list(&executor_globals->persistent_list); zend_destroy_rsrc_list(&executor_globals->persistent_list);
} }
}
static void executor_globals_dtor(zend_executor_globals *executor_globals) /* {{{ */
{
zend_ini_dtor(executor_globals->ini_directives);
if (executor_globals->zend_constants != GLOBAL_CONSTANTS_TABLE) { if (executor_globals->zend_constants != GLOBAL_CONSTANTS_TABLE) {
zend_hash_destroy(executor_globals->zend_constants); zend_hash_destroy(executor_globals->zend_constants);
free(executor_globals->zend_constants); free(executor_globals->zend_constants);
@ -1122,6 +1128,9 @@ void zend_shutdown(void) /* {{{ */
zend_vm_dtor(); zend_vm_dtor();
zend_destroy_rsrc_list(&EG(persistent_list)); zend_destroy_rsrc_list(&EG(persistent_list));
#ifdef ZTS
ts_apply_for_id(executor_globals_id, executor_globals_persistent_list_dtor);
#endif
zend_destroy_modules(); zend_destroy_modules();
virtual_cwd_deactivate(); virtual_cwd_deactivate();

View file

@ -168,13 +168,7 @@ static void _close_odbc_conn(zend_resource *rsrc)
SQLFreeEnv(conn->henv); SQLFreeEnv(conn->henv);
} }
efree(conn); efree(conn);
/* See https://github.com/php/php-src/issues/12974 why we need to check the if */
#ifdef ZTS
if (odbc_module_entry.module_started)
#endif
{
ODBCG(num_links)--; ODBCG(num_links)--;
}
} }
/* }}} */ /* }}} */

View file

@ -315,14 +315,8 @@ static void _close_pgsql_plink(zend_resource *rsrc)
PQclear(res); PQclear(res);
} }
PQfinish(link); PQfinish(link);
/* See https://github.com/php/php-src/issues/12974 why we need to check the if */
#ifdef ZTS
if (pgsql_module_entry.module_started)
#endif
{
PGG(num_persistent)--; PGG(num_persistent)--;
PGG(num_links)--; PGG(num_links)--;
}
rsrc->ptr = NULL; rsrc->ptr = NULL;
} }