Bug #71115 [NEW]: data race on auto globals names and internal functions names refcount

From: Date: Mon, 14 Dec 2015 11:50:25 +0000
Subject: Bug #71115 [NEW]: data race on auto globals names and internal functions names refcount
Groups: php.bugs 
Request: Send a blank email to php-bugs+get-197857@lists.php.net to get a copy of this message
From:             bobby dot mihalca at touchtech dot ro
Operating system: Any
PHP version:      7.0.0
Package:          Reproducible crash
Bug Type:         Bug
Bug description:data race on auto globals names and internal functions names refcount

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 bug report at https://bugs.php.net/bug.php?id=71115&edit=1
-- 
Try a snapshot (PHP 5.4):   https://bugs.php.net/fix.php?id=71115&r=trysnapshot54
Try a snapshot (PHP 5.5):   https://bugs.php.net/fix.php?id=71115&r=trysnapshot55
Try a snapshot (trunk):     https://bugs.php.net/fix.php?id=71115&r=trysnapshottrunk
Fixed in SVN:               https://bugs.php.net/fix.php?id=71115&r=fixed
Fixed in release:           https://bugs.php.net/fix.php?id=71115&r=alreadyfixed
Need backtrace:             https://bugs.php.net/fix.php?id=71115&r=needtrace
Need Reproduce Script:      https://bugs.php.net/fix.php?id=71115&r=needscript
Try newer version:          https://bugs.php.net/fix.php?id=71115&r=oldversion
Not developer issue:        https://bugs.php.net/fix.php?id=71115&r=support
Expected behavior:          https://bugs.php.net/fix.php?id=71115&r=notwrong
Not enough info:            https://bugs.php.net/fix.php?id=71115&r=notenoughinfo
Submitted twice:            https://bugs.php.net/fix.php?id=71115&r=submittedtwice
register_globals:           https://bugs.php.net/fix.php?id=71115&r=globals
PHP 4 support discontinued: https://bugs.php.net/fix.php?id=71115&r=php4
Daylight Savings:           https://bugs.php.net/fix.php?id=71115&r=dst
IIS Stability:              https://bugs.php.net/fix.php?id=71115&r=isapi
Install GNU Sed:            https://bugs.php.net/fix.php?id=71115&r=gnused
Floating point limitations: https://bugs.php.net/fix.php?id=71115&r=float
No Zend Extensions:         https://bugs.php.net/fix.php?id=71115&r=nozend
MySQL Configuration Error:  https://bugs.php.net/fix.php?id=71115&r=mysqlcfg



Thread (18 messages)

« previous php.bugs (#197857) next »