Fix high opcache.interned_strings_buffer causing shm corruption (#9260)

ext/opcache/ZendAccelerator.c

@@ -2840,18 +2840,23 @@ static inline int accel_find_sapi(void)
 static int zend_accel_init_shm(void)
 {
 	int i;
+	size_t accel_shared_globals_size;
 
 	zend_shared_alloc_lock();
 
 	if (ZCG(accel_directives).interned_strings_buffer) {
-		accel_shared_globals = zend_shared_alloc((ZCG(accel_directives).interned_strings_buffer * 1024 * 1024));
+		accel_shared_globals_size = ZCG(accel_directives).interned_strings_buffer * 1024 * 1024;
 	} else {
 		/* Make sure there is always at least one interned string hash slot,
 		 * so the table can be queried unconditionally. */
-		accel_shared_globals = zend_shared_alloc(sizeof(zend_accel_shared_globals) + sizeof(uint32_t));
+		accel_shared_globals_size = sizeof(zend_accel_shared_globals) + sizeof(uint32_t);
 	}
+
+	accel_shared_globals = zend_shared_alloc(accel_shared_globals_size);
 	if (!accel_shared_globals) {
-		zend_accel_error_noreturn(ACCEL_LOG_FATAL, "Insufficient shared memory!");
+		zend_accel_error_noreturn(ACCEL_LOG_FATAL,
+				"Insufficient shared memory for interned strings buffer! (tried to allocate %zu bytes)",
+				accel_shared_globals_size);
 		zend_shared_alloc_unlock();
 		return FAILURE;
 	}

ext/opcache/tests/gh9259_001.phpt

@@ -0,0 +1,18 @@
+--TEST--
+Bug GH-9259 001 (Setting opcache.interned_strings_buffer to a very high value leads to corruption of shm)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.interned_strings_buffer=131072
+opcache.log_verbosity_level=2
+opcache.enable_cli=1
+--FILE--
+<?php
+
+echo 'OK';
+
+?>
+--EXPECTF--
+%sWarning opcache.interned_strings_buffer must be less than or equal to 4095, 131072 given%s
+
+OK

ext/opcache/tests/gh9259_002.phpt

@@ -0,0 +1,18 @@
+--TEST--
+Bug GH-9259 002 (Setting opcache.interned_strings_buffer to a very high value leads to corruption of shm)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.interned_strings_buffer=-1
+opcache.log_verbosity_level=2
+opcache.enable_cli=1
+--FILE--
+<?php
+
+echo 'OK';
+
+?>
+--EXPECTF--
+%sWarning opcache.interned_strings_buffer must be greater than or equal to 0, -1 given%s
+
+OK

ext/opcache/tests/gh9259_003.phpt

@@ -0,0 +1,15 @@
+--TEST--
+Bug GH-9259 003 (Setting opcache.interned_strings_buffer to a very high value leads to corruption of shm)
+--EXTENSIONS--
+opcache
+--INI--
+opcache.interned_strings_buffer=500
+opcache.enable_cli=1
+--FILE--
+<?php
+
+echo 'OK';
+
+?>
+--EXPECTF--
+%sFatal Error Insufficient shared memory for interned strings buffer! (tried to allocate %d bytes)

ext/opcache/zend_accelerator_module.c

@@ -40,6 +40,7 @@
 #define STRING_NOT_NULL(s) (NULL == (s)?"":s)
 #define MIN_ACCEL_FILES 200
 #define MAX_ACCEL_FILES 1000000
+#define MAX_INTERNED_STRINGS_BUFFER_SIZE ((zend_long)((UINT32_MAX-PLATFORM_ALIGNMENT)/(1024*1024)))
 #define TOKENTOSTR(X) #X
 
 static zif_handler orig_file_exists = NULL;
@@ -78,6 +79,25 @@ static ZEND_INI_MH(OnUpdateMemoryConsumption)
 	return SUCCESS;
 }
 
+static ZEND_INI_MH(OnUpdateInternedStringsBuffer)
+{
+	zend_long *p = (zend_long *) ZEND_INI_GET_ADDR();
+	zend_long size = zend_ini_parse_quantity_warn(new_value, entry->name);
+
+	if (size < 0) {
+		zend_accel_error(ACCEL_LOG_WARNING, "opcache.interned_strings_buffer must be greater than or equal to 0, " ZEND_LONG_FMT " given.\n", size);
+		return FAILURE;
+	}
+	if (size > MAX_INTERNED_STRINGS_BUFFER_SIZE) {
+		zend_accel_error(ACCEL_LOG_WARNING, "opcache.interned_strings_buffer must be less than or equal to " ZEND_LONG_FMT ", " ZEND_LONG_FMT " given.\n", MAX_INTERNED_STRINGS_BUFFER_SIZE, size);
+		return FAILURE;
+	}
+
+	*p = size;
+
+	return SUCCESS;
+}
+
 static ZEND_INI_MH(OnUpdateMaxAcceleratedFiles)
 {
 	zend_long *p = (zend_long *) ZEND_INI_GET_ADDR();
@@ -239,7 +259,7 @@ ZEND_INI_BEGIN()
 
 	STD_PHP_INI_ENTRY("opcache.log_verbosity_level"   , "1"   , PHP_INI_SYSTEM, OnUpdateLong, accel_directives.log_verbosity_level,       zend_accel_globals, accel_globals)
 	STD_PHP_INI_ENTRY("opcache.memory_consumption"    , "128"  , PHP_INI_SYSTEM, OnUpdateMemoryConsumption,    accel_directives.memory_consumption,        zend_accel_globals, accel_globals)
-	STD_PHP_INI_ENTRY("opcache.interned_strings_buffer", "8"  , PHP_INI_SYSTEM, OnUpdateLong,                 accel_directives.interned_strings_buffer,   zend_accel_globals, accel_globals)
+	STD_PHP_INI_ENTRY("opcache.interned_strings_buffer", "8"  , PHP_INI_SYSTEM, OnUpdateInternedStringsBuffer,	 accel_directives.interned_strings_buffer,   zend_accel_globals, accel_globals)
 	STD_PHP_INI_ENTRY("opcache.max_accelerated_files" , "10000", PHP_INI_SYSTEM, OnUpdateMaxAcceleratedFiles,	 accel_directives.max_accelerated_files,     zend_accel_globals, accel_globals)
 	STD_PHP_INI_ENTRY("opcache.max_wasted_percentage" , "5"   , PHP_INI_SYSTEM, OnUpdateMaxWastedPercentage,	 accel_directives.max_wasted_percentage,     zend_accel_globals, accel_globals)
 	STD_PHP_INI_ENTRY("opcache.consistency_checks"    , "0"   , PHP_INI_ALL   , OnUpdateLong,	             accel_directives.consistency_checks,        zend_accel_globals, accel_globals)

ext/opcache/zend_shared_alloc.c

@@ -328,7 +328,7 @@ static size_t zend_shared_alloc_get_largest_free_block(void)
 #define MIN_FREE_MEMORY 64*1024
 
 #define SHARED_ALLOC_FAILED() do {		\
-		zend_accel_error(ACCEL_LOG_WARNING, "Not enough free shared space to allocate "ZEND_LONG_FMT" bytes ("ZEND_LONG_FMT" bytes free)", (zend_long)size, (zend_long)ZSMMG(shared_free)); \
+		zend_accel_error(ACCEL_LOG_WARNING, "Not enough free shared space to allocate %zu bytes (%zu bytes free)", size, ZSMMG(shared_free)); \
 		if (zend_shared_alloc_get_largest_free_block() < MIN_FREE_MEMORY) { \
 			ZSMMG(memory_exhausted) = 1; \
 		} \
@@ -339,6 +339,10 @@ void *zend_shared_alloc(size_t size)
 	int i;
 	unsigned int block_size = ZEND_ALIGNED_SIZE(size);
 
+	if (UNEXPECTED(block_size < size)) {
+		zend_accel_error_noreturn(ACCEL_LOG_ERROR, "Possible integer overflow in shared memory allocation (%zu + %zu)", size, PLATFORM_ALIGNMENT);
+	}
+
 #if 1
 	if (!ZCG(locked)) {
 		zend_accel_error_noreturn(ACCEL_LOG_ERROR, "Shared memory lock not obtained");