Bug #79696 [Wfx]: Child exited Segmentation fault, __strchr_sse2, putenv

From: Date: Fri, 21 May 2021 17:46:39 +0000
Subject: Bug #79696 [Wfx]: Child exited Segmentation fault, __strchr_sse2, putenv
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-233960@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=79696&edit=1

 ID:                 79696
 User updated by:    andrixnet at yahoo dot com
 Reported by:        andrixnet at yahoo dot com
 Summary:            Child exited Segmentation fault, __strchr_sse2,
                     putenv
 Status:             Wont fix
 Type:               Bug
 Package:            Apache2 related
 Operating System:   Linux Slackware-14.2
 PHP Version:        7.3.19
 Block user comment: N
 Private report:     N

 New Comment:

I know there are many components and many dependencies. 
However I can't but wander: same webapp on same PHP version, yet built on another  distro
doesn't seem to have this problem. 
And I am talking from personal experience and related experience of colegues involved in similar
projects.


Previous Comments:
------------------------------------------------------------------------
[2021-05-21 11:46:39] krakjoe@php.net

The environment is inherently not thread safe. The only fix we can provide, and did, is very narrow
in scope - it can only ensure that core uses the environment safely, but obviously all the other
code you are loading is not obliged to use our lock on the environment and has no way too, they are
still going to manipulate it in a non-thread safe way.

There is nothing more we can do about this, sorry.

------------------------------------------------------------------------
[2021-05-15 10:04:38] php at linocomm dot net

I am running into the same problem with PHP 7.4.19 which would indicate that the problem is not
fully fixed with https://github.com/php/php-src/commit/072eb6dd77b079a6f90ca5b155f9b0add1b5f2d4.

Environment is Centos 7 with the only site running on the server a WooCommerce site, latest version.

The problem doesn't happen with the same site on another server running Centos 8 with Apache
version 7.2.24. This leads me to think that it is either a problem specific to PHP 7.3/7.4, or that
some incompatible modules are causing it.

In all cases, the mpm_event module is used. I cannot reproduce the problem with mpm_prefork, which
is reason to think it is a thread-safe problem.

------------------------------------------------------------------------
[2020-06-12 21:25:54] andrixnet at yahoo dot com

In my backporting I tried this, without success. 
It behaves like it's not there, like the locking mechanism has no effect. 

PHP is compiled with --enable-maintainer-zts and thus all TSRM functionality should be available.

At this point my C knowledge gets overwhelmed. Please help.

=============================================================================
diff -urb php-7.3.19.orig/TSRM/TSRM.c php-7.3.19/TSRM/TSRM.c
--- php-7.3.19.orig/TSRM/TSRM.c	2020-06-09 11:06:31.000000000 +0300
+++ php-7.3.19/TSRM/TSRM.c	2020-06-12 21:21:06.233248609 +0300
@@ -53,6 +53,7 @@
 
 
 static MUTEX_T tsmm_mutex;	/* thread-safe memory manager mutex */
+static MUTEX_T tsrm_env_mutex; /* tsrm environ mutex */
 
 /* New thread handlers */
 static tsrm_thread_begin_func_t tsrm_new_thread_begin_handler = NULL;
@@ -164,6 +165,9 @@
 	tsmm_mutex = tsrm_mutex_alloc();
 
 	TSRM_ERROR((TSRM_ERROR_LEVEL_CORE, "Started up TSRM, %d expected threads, %d expected
resources", expected_threads, expected_resources));
+
+	tsrm_env_mutex = tsrm_mutex_alloc();
+
 	return 1;
 }/*}}}*/
 
@@ -208,6 +212,8 @@
 	}
 	tsrm_mutex_free(tsmm_mutex);
 	tsmm_mutex = NULL;
+	tsrm_mutex_free(tsrm_env_mutex);
+	tsrm_env_mutex = NULL;
 	TSRM_ERROR((TSRM_ERROR_LEVEL_CORE, "Shutdown TSRM"));
 	if (tsrm_error_file!=stderr) {
 		fclose(tsrm_error_file);
@@ -228,6 +234,15 @@
 	tsrm_shutdown_handler = NULL;
 }/*}}}*/
 
+/* {{{ */
+/* environ lock api */
+TSRM_API int tsrm_env_lock() {
+    return tsrm_mutex_lock(tsrm_env_mutex);
+}
+
+TSRM_API int tsrm_env_unlock() {
+    return tsrm_mutex_unlock(tsrm_env_mutex);
+} /* }}} */
 
 /* allocates a new thread-safe-resource id */
 TSRM_API ts_rsrc_id ts_allocate_id(ts_rsrc_id *rsrc_id, size_t size, ts_allocate_ctor ctor,
ts_allocate_dtor dtor)
diff -urb php-7.3.19.orig/TSRM/TSRM.h php-7.3.19/TSRM/TSRM.h
--- php-7.3.19.orig/TSRM/TSRM.h	2020-06-09 11:06:31.000000000 +0300
+++ php-7.3.19/TSRM/TSRM.h	2020-06-12 21:04:02.716411401 +0300
@@ -91,6 +91,10 @@
 TSRM_API int tsrm_startup(int expected_threads, int expected_resources, int debug_level, char
*debug_filename);
 TSRM_API void tsrm_shutdown(void);
 
+/* environ lock API */
+TSRM_API int tsrm_env_lock();
+TSRM_API int tsrm_env_unlock();
+
 /* allocates a new thread-safe-resource id */
 TSRM_API ts_rsrc_id ts_allocate_id(ts_rsrc_id *rsrc_id, size_t size, ts_allocate_ctor ctor,
ts_allocate_dtor dtor);
 
@@ -183,6 +187,9 @@
 
 #else /* non ZTS */
 
+#define tsrm_env_lock()    0
+#define tsrm_env_unlock()  0
+
 #define TSRMLS_FETCH()
 #define TSRMLS_FETCH_FROM_CTX(ctx)
 #define TSRMLS_SET_CTX(ctx)
diff -urb php-7.3.19.orig/UPGRADING.INTERNALS php-7.3.19/UPGRADING.INTERNALS
--- php-7.3.19.orig/UPGRADING.INTERNALS	2020-06-09 11:06:31.000000000 +0300
+++ php-7.3.19/UPGRADING.INTERNALS	2020-06-12 21:10:30.352344136 +0300
@@ -29,6 +29,7 @@
   z. HAVE_ST_BLKSIZE and HAVE_ST_RDEV
   aa. RETSIGTYPE
   bb. php_setcookie
+  cc. TSRM environment locking
 
 2. Build system changes
   a. Unix build system changes
@@ -181,6 +182,19 @@
                       zend_string *path, zend_string *domain, int secure,
                       int httponly, zend_string *samesite, int url_encode);
 
+ cc. TSRM adds tsrm_env_lock() and tsrm_env_unlock() for ZTS:
+     code that may change environ and may run concurrently with user code in ZTS
+     is expected to use this exclusion API to maintain as much safety as reasonable.
+     This results in "thread safe" getenv/putenv in Windows and Unix, however
+     functions that may read the environment without exclusion still exist,
+     for example:
+       - setlocale
+       - mktime
+       - tzset
+     The above is not an exhaustive list of such functions, while getenv/putenv will
+     behave as if they are safe, care should still be taken in multi-threaded
+     environments.
+
 ========================
 2. Build system changes
 ========================
diff -urb php-7.3.19.orig/ext/standard/basic_functions.c php-7.3.19/ext/standard/basic_functions.c
--- php-7.3.19.orig/ext/standard/basic_functions.c	2020-06-09 11:06:36.000000000 +0300
+++ php-7.3.19/ext/standard/basic_functions.c	2020-06-12 23:08:03.612663536 +0300
@@ -3472,6 +3472,7 @@
 {
 	putenv_entry *pe = Z_PTR_P(zv);
 
+
 	if (pe->previous_value) {
 # if defined(PHP_WIN32)
 		/* MSVCRT has a bug in putenv() when setting a variable that
@@ -3511,6 +3512,7 @@
 	}
 #endif
 
+
 	efree(pe->putenv_string);
 	efree(pe->key);
 	efree(pe);
@@ -3811,7 +3813,9 @@
 	BG(page_inode) = -1;
 	BG(page_mtime) = -1;
 #ifdef HAVE_PUTENV
+	tsrm_env_lock();
 	zend_hash_init(&BG(putenv_ht), 1, NULL, php_putenv_destructor, 0);
+	tsrm_env_unlock();
 #endif
 	BG(user_shutdown_function_names) = NULL;
 
@@ -3841,7 +3845,9 @@
 	ZVAL_UNDEF(&BG(strtok_zval));
 	BG(strtok_string) = NULL;
 #ifdef HAVE_PUTENV
+	tsrm_env_lock();
 	zend_hash_destroy(&BG(putenv_ht));
+	tsrm_env_unlock();
 #endif
 
 	BG(mt_rand_is_seeded) = 0;
@@ -4150,11 +4156,22 @@
 		}
 	}
 #else
+
+	tsrm_env_lock();
+
 	/* system method returns a const */
 	ptr = getenv(str);
+
+	if (ptr) {
+		RETVAL_STRING(ptr);
+	}
+
+	tsrm_env_unlock();
+
 	if (ptr) {
-		RETURN_STRING(ptr);
+	    return;
 	}
+
 #endif
 	RETURN_FALSE;
 }
@@ -4205,6 +4222,7 @@
 	}
 #endif
 
+	tsrm_env_lock();
 	zend_hash_str_del(&BG(putenv_ht), pe.key, pe.key_len);
 
 	/* find previous value */
@@ -4265,6 +4283,7 @@
 			tzset();
 		}
 #endif
+		tsrm_env_unlock();
 #if defined(PHP_WIN32)
 		free(keyw);
 		free(valw);
diff -urb php-7.3.19.orig/ext/standard/info.c php-7.3.19/ext/standard/info.c
--- php-7.3.19.orig/ext/standard/info.c	2020-06-09 11:06:36.000000000 +0300
+++ php-7.3.19/ext/standard/info.c	2020-06-12 21:16:19.887288448 +0300
@@ -965,6 +965,7 @@
 		SECTION("Environment");
 		php_info_print_table_start();
 		php_info_print_table_header(2, "Variable", "Value");
+		tsrm_env_lock();
 		for (env=environ; env!=NULL && *env !=NULL; env++) {
 			tmp1 = estrdup(*env);
 			if (!(tmp2=strchr(tmp1,'='))) { /* malformed entry? */
@@ -976,6 +977,7 @@
 			php_info_print_table_row(2, tmp1, tmp2);
 			efree(tmp1);
 		}
+		tsrm_env_unlock();
 		php_info_print_table_end();
 	}
 
diff -urb php-7.3.19.orig/main/php_variables.c php-7.3.19/main/php_variables.c
--- php-7.3.19.orig/main/php_variables.c	2020-06-09 11:06:31.000000000 +0300
+++ php-7.3.19/main/php_variables.c	2020-06-12 21:18:31.840268566 +0300
@@ -580,6 +580,8 @@
 	char *environment, *env;
 #endif
 
+	tsrm_env_lock();
+
 #ifndef PHP_WIN32
 	for (env = environ; env != NULL && *env != NULL; env++) {
 		import_environment_variable(Z_ARRVAL_P(array_ptr), *env);
@@ -591,6 +593,8 @@
 	}
 	FreeEnvironmentStringsA(environment);
 #endif
+
+	tsrm_env_unlock();
 }
 
 zend_bool php_std_auto_global_callback(char *name, uint32_t name_len)
diff -urb php-7.3.19.orig/sapi/litespeed/lsapi_main.c php-7.3.19/sapi/litespeed/lsapi_main.c
--- php-7.3.19.orig/sapi/litespeed/lsapi_main.c	2020-06-09 11:06:31.000000000 +0300
+++ php-7.3.19/sapi/litespeed/lsapi_main.c	2020-06-12 21:04:02.720411400 +0300
@@ -249,6 +249,7 @@
         return;
     }
 
+    tsrm_env_lock();
     for (env = environ; env != NULL && *env != NULL; env++) {
         p = strchr(*env, '=');
         if (!p) {               /* malformed entry? */
@@ -263,6 +264,7 @@
         t[nlen] = '\0';
         add_variable(t, nlen, p + 1, strlen( p + 1 ), array_ptr);
     }
+    tsrm_env_unlock();
     if (t != buf && t != NULL) {
         efree(t);
     }

------------------------------------------------------------------------
[2020-06-12 20:58:32] andrixnet at yahoo dot com

I am trying to backport myself that code to 7.3.
Yet one of the blocks that now has tsrm_env_lock yield this: 

Thread 1 (Thread 0x7f97857fa700 (LWP 16428)):
#0  0x00007f97b8d3a028 in __strncmp_sse2 () at /lib64/libc.so.6
#1  0x00007f97ac5e7d0a in zif_putenv (execute_data=0x7f9746c26700, return_value=0x7f97857f5060)
    at /tmp/php-7.3.19/ext/standard/basic_functions.c:4231
        setting = 0x7f978f486b78 "MAGICK_THREAD_LIMIT=1"
        setting_len = 21
        p = 0x7f97767f5eb3 ""
        env = 0x7f97781bb448
        pe =
          {putenv_string = 0x7f97767f5eb8 "MAGICK_THREAD_LIMIT=1", previous_value = 0x0,
key = 0x7f97767f5ea0 "MAGICK_THREAD_LIMIT", key_len = 19}
#2  0x00007f97ac982ebd in execute_ex () at /tmp/php-7.3.19/Zend/zend_vm_execute.h:649
        call = 0x7f9746c26700
        fbc = 0x7f975c0f3840
        ret = 0x7f97857f5060
        retval =
            {value = {lval = 140288916162632, dval = 6.931193396825927e-310, counted =
0x7f978f038848, str = 0x7f978f038848, arr = 0x7f978f038848, obj = 0x7f978f038848, res =
0x7f978f038848, ref = 0x7f978f038848, ast = 0x7f978f038848, zv = 0x7f978f038848, ptr =
0x7f978f038848, ce = 0x7f978f038848, func = 0x7f978f038848, ww = {w1 = 2399373384, w2 = 32663}}, u1
= {v = {type = 1 '\001', type_flags = 0 '\000', u = {call_info = 0, extra = 0}},
type_info = 1}, u2 = {next = 0, cache_slot = 0, opline_num = 0, lineno = 0, num_args = 0, fe_pos =
0, fe_iter_idx = 0, access_flags = 0, property_guard = 0, constant_flags = 0, extra = 0}}
        orig_opline = 0x0
        orig_execute_data = 0x7f97857fa700
#3  0x00007f97ac982ebd in execute_ex (ex=0x7f9746c26030) at
/tmp/php-7.3.19/Zend/zend_vm_execute.h:55503

the zif_putenv call leads to PHP_FUNCTION(putenv) in basic_functions.c: 

        tsrm_env_lock();
        zend_hash_str_del(&BG(putenv_ht), pe.key, pe.key_len);

        /* find previous value */
        pe.previous_value = NULL;
        for (env = environ; env != NULL && *env != NULL; env++) {
            if (!strncmp(*env, pe.key, pe.key_len) && (*env)[pe.key_len] == '=') {
 /* found it */
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this is the line refered to in the core dump.
#if defined(PHP_WIN32)
                        /* must copy previous value because MSVCRT's putenv can free the string
without notice */
                        pe.previous_value = estrdup(*env);
#else
                        pe.previous_value = *env;
#endif
                        break;
                }
        }

#if HAVE_UNSETENV
        if (!p) { /* no '=' means we want to unset it */
                unsetenv(pe.putenv_string);
        }
        if (!p || putenv(pe.putenv_string) == 0) { /* success */
#else
# ifndef PHP_WIN32
        if (putenv(pe.putenv_string) == 0) { /* success */
# else
                wchar_t *keyw, *valw = NULL;

                keyw = php_win32_cp_any_to_w(pe.key);
                if (value) {
                        valw = php_win32_cp_any_to_w(value);
                }
                /* valw may be NULL, but the failed conversion still needs to be checked. */
                if (!keyw || !valw && value) {
                        efree(pe.putenv_string);
                        efree(pe.key);
                        free(keyw);
                        free(valw);
                        RETURN_FALSE;
                }

        error_code = SetEnvironmentVariableW(keyw, valw);

        if (error_code != 0
# ifndef ZTS
        /* We need both SetEnvironmentVariable and _putenv here as some
                dependency lib could use either way to read the environment.
                Obviously the CRT version will be useful more often. But
                generally, doing both brings us on the safe track at least
                in NTS build. */
        && _wputenv_s(keyw, valw ? valw : L"") == 0
# endif
        ) { /* success */
# endif
#endif
                zend_hash_str_add_mem(&BG(putenv_ht), pe.key, pe.key_len, &pe,
sizeof(putenv_entry));
#ifdef HAVE_TZSET
                if (!strncmp(pe.key, "TZ", pe.key_len)) {
                        tzset();
                }
#endif
                tsrm_env_unlock();

They correspond to the 3rd and 4th changes in basic_functions.c here https://github.com/php/php-src/commit/072eb6dd77b079a6f90ca5b155f9b0add1b5f2d4

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=79696


--
Edit this bug report at https://bugs.php.net/bug.php?id=79696&edit=1


Thread (9 messages)

« previous php.bugs (#233960) next »