tsrm environment lock

7 files changed, 61 insertions, 3 deletions

diff --git a/TSRM/TSRM.c b/TSRM/TSRM.c
index 670c260678..d70c09d2b5 100644
--- a/TSRM/TSRM.c
+++ b/TSRM/TSRM.c
@@ -52,7 +52,8 @@ static int					resource_types_table_size;
 static size_t tsrm_reserved_pos  = 0;
 static size_t tsrm_reserved_size = 0;
 
-static MUTEX_T tsmm_mutex;	/* thread-safe memory manager mutex */
+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;
@@ -168,6 +169,8 @@ TSRM_API int tsrm_startup(int expected_threads, int expected_resources, int debu
 	tsrm_reserved_pos  = 0;
 	tsrm_reserved_size = 0;
 
+	tsrm_env_mutex = tsrm_mutex_alloc();
+
 	return 1;
 }/*}}}*/
 
@@ -214,6 +217,8 @@ TSRM_API void tsrm_shutdown(void)
 	}
 	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);
@@ -237,6 +242,15 @@ TSRM_API void tsrm_shutdown(void)
 	tsrm_reserved_size = 0;
 }/*}}}*/
 
+/* {{{ */
+/* 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);
+} /* }}} */
 
 /* enlarge the arrays for the already active threads */
 static void tsrm_update_active_threads(void)
diff --git a/TSRM/TSRM.h b/TSRM/TSRM.h
index a14e31e1f0..5e1f8164ee 100644
--- a/TSRM/TSRM.h
+++ b/TSRM/TSRM.h
@@ -99,6 +99,10 @@ extern "C" {
 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);
 
@@ -205,6 +209,9 @@ TSRM_API const char *tsrm_api_name(void);
 
 #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 --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS
index 626c71e663..aba4dc90cb 100644
--- a/UPGRADING.INTERNALS
+++ b/UPGRADING.INTERNALS
@@ -18,7 +18,8 @@ PHP 7.4 INTERNALS UPGRADE NOTES
   o. ZEND_COMPILE_EXTENDED_INFO split
   p. ZEND_EXT_FCALL_BEGIN can access arguments
   q. ZEND_COMPILE_IGNORE_USER_FUNCTIONS and ZEND_COMPILE_IGNORE_INTERNAL_FUNCTIONS
-
+  r. TSRM environment locking
+  
 2. Build system changes
   a. Abstract
   b. Unix build system changes
@@ -184,6 +185,19 @@ PHP 7.4 INTERNALS UPGRADE NOTES
      call opcodes are ZEND_DO_FCALL and ZEND_DO_FCALL_BY_NAME, previously they
      were ignored by zend_get_call_op.
      
+  r. 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 --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c
index d281730afa..ba4b5ee68c 100644
--- a/ext/standard/basic_functions.c
+++ b/ext/standard/basic_functions.c
@@ -3832,7 +3832,9 @@ PHP_RSHUTDOWN_FUNCTION(basic) /* {{{ */
 	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;
@@ -4141,11 +4143,22 @@ PHP_FUNCTION(getenv)
 		}
 	}
 #else
+
+    tsrm_env_lock();
+    
 	/* system method returns a const */
 	ptr = getenv(str);
+	
 	if (ptr) {
-		RETURN_STRING(ptr);
+		RETVAL_STRING(ptr);
 	}
+
+    tsrm_env_unlock();
+
+    if (ptr) {
+        return;
+    }
+
 #endif
 	RETURN_FALSE;
 }
@@ -4196,6 +4209,7 @@ PHP_FUNCTION(putenv)
 	}
 #endif
 
+	tsrm_env_lock();
 	zend_hash_str_del(&BG(putenv_ht), pe.key, pe.key_len);
 
 	/* find previous value */
@@ -4256,6 +4270,7 @@ PHP_FUNCTION(putenv)
 			tzset();
 		}
 #endif
+		tsrm_env_unlock();
 #if defined(PHP_WIN32)
 		free(keyw);
 		free(valw);
diff --git a/ext/standard/info.c b/ext/standard/info.c
index 570c92393c..4600d66d17 100644
--- a/ext/standard/info.c
+++ b/ext/standard/info.c
@@ -954,6 +954,7 @@ PHPAPI void php_print_info(int flag)
 		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? */
@@ -965,6 +966,7 @@ PHPAPI void php_print_info(int flag)
 			php_info_print_table_row(2, tmp1, tmp2);
 			efree(tmp1);
 		}
+        tsrm_env_unlock();
 		php_info_print_table_end();
 	}
 
diff --git a/main/php_variables.c b/main/php_variables.c
index decf73a80f..f5692ede4b 100644
--- a/main/php_variables.c
+++ b/main/php_variables.c
@@ -548,6 +548,8 @@ void _php_import_environment_variables(zval *array_ptr)
 	zval val;
 	zend_ulong idx;
 
+	tsrm_env_lock();
+
 	for (env = environ; env != NULL && *env != NULL; env++) {
 		p = strchr(*env, '=');
 		if (!p
@@ -572,6 +574,8 @@ void _php_import_environment_variables(zval *array_ptr)
 			php_register_variable_quick(*env, name_len, &val, Z_ARRVAL_P(array_ptr));
 		}
 	}
+	
+	tsrm_env_unlock();
 }
 
 zend_bool php_std_auto_global_callback(char *name, uint32_t name_len)
diff --git a/sapi/litespeed/lsapi_main.c b/sapi/litespeed/lsapi_main.c
index 64e95d5ceb..2d8eb75326 100644
--- a/sapi/litespeed/lsapi_main.c
+++ b/sapi/litespeed/lsapi_main.c
@@ -244,6 +244,7 @@ static void litespeed_php_import_environment_variables(zval *array_ptr)
         return;
     }
 
+    tsrm_env_lock();
     for (env = environ; env != NULL && *env != NULL; env++) {
         p = strchr(*env, '=');
         if (!p) {               /* malformed entry? */
@@ -258,6 +259,7 @@ static void litespeed_php_import_environment_variables(zval *array_ptr)
         t[nlen] = '\0';
         add_variable(t, nlen, p + 1, strlen( p + 1 ), array_ptr);
     }
+    tsrm_env_unlock();
     if (t != buf && t != NULL) {
         efree(t);
     }