Skip to content

Commit

Permalink
Merge branch 'PHP-8.2'
Browse files Browse the repository at this point in the history
* PHP-8.2:
  [ci skip] NEWS
  Log the cause of error when opcache cannot write to file cache (php#9258)
  Fix high opcache.interned_strings_buffer causing shm corruption (php#9260)
  • Loading branch information
arnaud-lb committed Sep 3, 2022
2 parents e2bdf4d + 349fdc9 commit 745db44
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 13 deletions.
11 changes: 8 additions & 3 deletions ext/opcache/ZendAccelerator.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
42 changes: 42 additions & 0 deletions ext/opcache/tests/file_cache_error.phpt
Original file line number Diff line number Diff line change
@@ -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--
<?php
if (!posix_setrlimit(POSIX_RLIMIT_FSIZE, 1, 1)) die('skip Test requires setrlimit(RLIMIT_FSIZE) to work');
if (ini_parse_quantity(ini_get('opcache.jit_buffer_size')) !== 0) die('skip File cache is disabled when JIT is on');
?>
--FILE--
<?php

// Create new file to ensure that it's not cached accross test runs
$file = tempnam(sys_get_temp_dir(), 'file_cache_error');
register_shutdown_function(function () use ($file) {
unlink($file);
});
file_put_contents($file, '<?php echo "OK";');
touch($file, time() - 3600);

// Some systems will raise SIGXFSZ when RLIMIT_FSIZE is exceeded
if (defined('SIGXFSZ')) {
pcntl_signal(SIGXFSZ, SIG_IGN);
}

// Should cause writing to cache file to fail
var_dump(posix_setrlimit(POSIX_RLIMIT_FSIZE, 1, 1));

// Will attempt to write to cache file, and fail
require $file;
?>
--EXPECTF--
bool(true)
%sWarning opcache cannot write to file %s: %s

OK
18 changes: 18 additions & 0 deletions ext/opcache/tests/gh9259_001.phpt
Original file line number Diff line number Diff line change
@@ -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
18 changes: 18 additions & 0 deletions ext/opcache/tests/gh9259_002.phpt
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions ext/opcache/tests/gh9259_003.phpt
Original file line number Diff line number Diff line change
@@ -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)
22 changes: 21 additions & 1 deletion ext/opcache/zend_accelerator_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand Down
49 changes: 41 additions & 8 deletions ext/opcache/zend_file_cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -1004,23 +1004,56 @@ 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) },
{ .iov_base = (void *)buf, .iov_len = script->size },
{ .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
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion ext/opcache/zend_shared_alloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
} \
Expand All @@ -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");
Expand Down

0 comments on commit 745db44

Please sign in to comment.