Edit report at https://bugs.php.net/bug.php?id=71115&edit=1
ID: 71115
Updated by: krakjoe@php.net
Reported by: bobby dot mihalca at touchtech dot ro
Summary: data race on auto globals names and internal
functions names refcount
Status: Open
Type: Bug
Package: Reproducible crash
Operating System: Any
PHP Version: 7.0.0
Block user comment: N
Private report: N
New Comment:
This is definitely a problem ...
pthreads uses critical sections around startup/shutdown of user threads to work around the problem
... I'd like to remove those sections ...
I'm not sure what the proper fix should look like though, disabling refcounts on those strings
will hide internal programmer error (leaks).
Previous Comments:
------------------------------------------------------------------------
[2015-12-14 11:50:17] 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)
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=71115&edit=1