diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index fd2792028b1d6..a74ad34ab8ea1 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/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; } diff --git a/ext/opcache/tests/file_cache_error.phpt b/ext/opcache/tests/file_cache_error.phpt new file mode 100644 index 0000000000000..75b63cd21cce0 --- /dev/null +++ b/ext/opcache/tests/file_cache_error.phpt @@ -0,0 +1,42 @@ +--TEST-- +File cache error 001 +--EXTENSIONS-- +opcache +posix +pcntl +--INI-- +opcache.enable_cli=1 +opcache.file_cache={TMP} +opcache.log_verbosity_level=2 +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +bool(true) +%sWarning opcache cannot write to file %s: %s + +OK diff --git a/ext/opcache/tests/gh9259_001.phpt b/ext/opcache/tests/gh9259_001.phpt new file mode 100644 index 0000000000000..bcc0f113c57f6 --- /dev/null +++ b/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-- + +--EXPECTF-- +%sWarning opcache.interned_strings_buffer must be less than or equal to 4095, 131072 given%s + +OK diff --git a/ext/opcache/tests/gh9259_002.phpt b/ext/opcache/tests/gh9259_002.phpt new file mode 100644 index 0000000000000..8b74949b49469 --- /dev/null +++ b/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-- + +--EXPECTF-- +%sWarning opcache.interned_strings_buffer must be greater than or equal to 0, -1 given%s + +OK diff --git a/ext/opcache/tests/gh9259_003.phpt b/ext/opcache/tests/gh9259_003.phpt new file mode 100644 index 0000000000000..91bfcad917b36 --- /dev/null +++ b/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-- + +--EXPECTF-- +%sFatal Error Insufficient shared memory for interned strings buffer! (tried to allocate %d bytes) diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 6a5eb2b69b344..270ceda7c6293 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/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) diff --git a/ext/opcache/zend_file_cache.c b/ext/opcache/zend_file_cache.c index badbac2185535..a15736b72e5fe 100644 --- a/ext/opcache/zend_file_cache.c +++ b/ext/opcache/zend_file_cache.c @@ -1004,10 +1004,13 @@ static char *zend_file_cache_get_bin_file_path(zend_string *script_path) /** * Helper function for zend_file_cache_script_store(). * - * @return true on success, false on error + * @return true on success, false on error and errno is set to indicate the cause of the error */ static bool zend_file_cache_script_write(int fd, const zend_persistent_script *script, const zend_file_cache_metainfo *info, const void *buf, const zend_string *s) { + ssize_t written; + const ssize_t total_size = (ssize_t)(sizeof(*info) + script->size + info->str_size); + #ifdef HAVE_SYS_UIO_H const struct iovec vec[] = { { .iov_base = (void *)info, .iov_len = sizeof(*info) }, @@ -1015,12 +1018,42 @@ static bool zend_file_cache_script_write(int fd, const zend_persistent_script *s { .iov_base = (void *)ZSTR_VAL(s), .iov_len = info->str_size }, }; - return writev(fd, vec, sizeof(vec) / sizeof(vec[0])) == (ssize_t)(sizeof(*info) + script->size + info->str_size); + written = writev(fd, vec, sizeof(vec) / sizeof(vec[0])); + if (EXPECTED(written == total_size)) { + return true; + } + + errno = written == -1 ? errno : EAGAIN; + return false; #else - return ZEND_LONG_MAX >= (zend_long)(sizeof(*info) + script->size + info->str_size) && - write(fd, info, sizeof(*info)) == sizeof(*info) && - write(fd, buf, script->size) == script->size && - write(fd, ZSTR_VAL(s), info->str_size) == info->str_size; + if (UNEXPECTED(ZEND_LONG_MAX < (zend_long)total_size)) { +# ifdef EFBIG + errno = EFBIG; +# else + errno = ERANGE; +# endif + return false; + } + + written = write(fd, info, sizeof(*info)); + if (UNEXPECTED(written != sizeof(*info))) { + errno = written == -1 ? errno : EAGAIN; + return false; + } + + written = write(fd, buf, script->size); + if (UNEXPECTED(written != script->size)) { + errno = written == -1 ? errno : EAGAIN; + return false; + } + + written = write(fd, ZSTR_VAL(s), info->str_size); + if (UNEXPECTED(written != info->str_size)) { + errno = written == -1 ? errno : EAGAIN; + return false; + } + + return true; #endif } @@ -1095,7 +1128,7 @@ int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm) #endif if (!zend_file_cache_script_write(fd, script, &info, buf, s)) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot write to file '%s'\n", filename); + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot write to file '%s': %s\n", filename, strerror(errno)); zend_string_release_ex(s, 0); close(fd); efree(mem); @@ -1107,7 +1140,7 @@ int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm) zend_string_release_ex(s, 0); efree(mem); if (zend_file_cache_flock(fd, LOCK_UN) != 0) { - zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename); + zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s': %s\n", filename, strerror(errno)); } close(fd); efree(filename); diff --git a/ext/opcache/zend_shared_alloc.c b/ext/opcache/zend_shared_alloc.c index 7f820fcdc27b0..3f18a4db6040e 100644 --- a/ext/opcache/zend_shared_alloc.c +++ b/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");