|
|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2015-12-14 11:50 UTC] bobby dot mihalca at touchtech dot ro
Description:
------------
There is a data race on string refcount that could/will cause any multithreaded sapi to segfault.
This happens because during request startup ts_resource, php_request_startup, etc will copy auto_globals, internal functions, etc from php to request.
Since the name is an interned string thus a zend_string* if ZTS enabled, the refcount will be used, but the refcount is not atomic and we have a data race resulting in segfaults.
Ex while copy _SERVER auto global name (refcount=1):
thread 1 load refcount = 1
thread 2 load refount = 1
thread 1 inc refcount = 2
thread 2 inc refcount = 2
thread 1 store refount
thread 2 store refount
refcount=2 instead of 3
thread 1 release string, refcount = 1
thread 2 release string, refount = 0, free string
thread 3 start, segfault due to accessing freed memory
A quick workaround for this problem is to mark all the names as interned after startup and remove the flag before shutdown.
This will prevent reference counting and thus data race, premature release and segfaults.
I use the following code for my startup/shutdown:
static int startup(sapi_module_struct * sapi_module)
{
if (php_module_startup(sapi_module, NULL, 0) == SUCCESS) {
zend_auto_global *global;
zend_internal_function *function;
zend_class_entry* class;
ZEND_HASH_FOREACH_PTR(CG(auto_globals), global) {
GC_FLAGS(global->name) |= IS_STR_INTERNED;
zend_string_hash_val(global->name);
} ZEND_HASH_FOREACH_END();
ZEND_HASH_FOREACH_PTR(CG(function_table), function) {
GC_FLAGS(function->function_name) |= IS_STR_INTERNED;
zend_string_hash_val(function->function_name);
} ZEND_HASH_FOREACH_END();
ZEND_HASH_FOREACH_PTR(CG(class_table), class) {
zend_property_info *property;
GC_FLAGS(class->name) |= IS_STR_INTERNED;
zend_string_hash_val(class->name);
ZEND_HASH_FOREACH_PTR(&class->properties_info, property) {
GC_FLAGS(property->name) |= IS_STR_INTERNED;
zend_string_hash_val(property->name);
if (property->doc_comment) {
GC_FLAGS(property->doc_comment) |= IS_STR_INTERNED;
}
} ZEND_HASH_FOREACH_END();
} ZEND_HASH_FOREACH_END();
return SUCCESS;
}
return FAILURE;
}
int shutdown(sapi_module_struct *sapi_globals)
{
zend_auto_global *global;
zend_internal_function *function;
zend_class_entry* class;
ZEND_HASH_FOREACH_PTR(CG(auto_globals), global) {
GC_FLAGS(global->name) &= ~IS_STR_INTERNED;
} ZEND_HASH_FOREACH_END();
ZEND_HASH_FOREACH_PTR(CG(function_table), function) {
GC_FLAGS(function->function_name) &= ~IS_STR_INTERNED;
} ZEND_HASH_FOREACH_END();
ZEND_HASH_FOREACH_PTR(CG(class_table), class) {
zend_property_info *property;
GC_FLAGS(class->name) &= ~IS_STR_INTERNED;
ZEND_HASH_FOREACH_PTR(&class->properties_info, property) {
GC_FLAGS(property->name) &= ~IS_STR_INTERNED;
if (property->doc_comment) {
GC_FLAGS(property->doc_comment) &= ~IS_STR_INTERNED;
}
} ZEND_HASH_FOREACH_END();
} ZEND_HASH_FOREACH_END();
php_module_shutdown();
return SUCCESS;
}
Note that zend_string_hash_val is not necessary but will prevent data race on hash (witch is probably harmless anyway)
PatchesPull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||||||||||||
Copyright © 2001-2025 The PHP GroupAll rights reserved. |
Last updated: Thu Oct 30 22:00:01 2025 UTC |
@ab sent you the sources for the sapi @krakjoe being persistent allocated just allows my workaround to work as they are not freed at request shutdown. Now "_GET" is copied to a new persistent string as part php_module_startup in php_startup_auto_globals zend_register_auto_global(zend_string_init("_GET", sizeof("_GET")-1, 1), 0,php_auto_globals_create_get); then each request will copy it in compiler_globals_ctor zend_hash_copy(compiler_globals->auto_globals, global_auto_globals_table, auto_global_copy_ctor); When 2 threads/requests do the copy at the same time you have the described data race and segfault Sice auto globals are allocated by sapi, they live as long as the sapi and there is no need to copy them on each request, we can safely mark them as interned. Before module shutdown i remove the interned flag and module shutdown will free the memory. The same goes to internal function names, classes,etc For dynamic extensions/dl() the making/unmarking should be part of module load/unload. Since ZVAL_STR_COPY (as in zend_lookup_class_ex) will increase refcount for non interned strings i see no viable solution other then marking them interned. I was also thinking about making the refcount atomic but that will have performance implications. Besides would break ABI