Skip to content

Commit 745db44

Browse files
committed
Merge branch 'PHP-8.2'
* 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)
2 parents e2bdf4d + 349fdc9 commit 745db44

8 files changed

+168
-13
lines changed

ext/opcache/ZendAccelerator.c

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2840,18 +2840,23 @@ static inline int accel_find_sapi(void)
28402840
static int zend_accel_init_shm(void)
28412841
{
28422842
int i;
2843+
size_t accel_shared_globals_size;
28432844

28442845
zend_shared_alloc_lock();
28452846

28462847
if (ZCG(accel_directives).interned_strings_buffer) {
2847-
accel_shared_globals = zend_shared_alloc((ZCG(accel_directives).interned_strings_buffer * 1024 * 1024));
2848+
accel_shared_globals_size = ZCG(accel_directives).interned_strings_buffer * 1024 * 1024;
28482849
} else {
28492850
/* Make sure there is always at least one interned string hash slot,
28502851
* so the table can be queried unconditionally. */
2851-
accel_shared_globals = zend_shared_alloc(sizeof(zend_accel_shared_globals) + sizeof(uint32_t));
2852+
accel_shared_globals_size = sizeof(zend_accel_shared_globals) + sizeof(uint32_t);
28522853
}
2854+
2855+
accel_shared_globals = zend_shared_alloc(accel_shared_globals_size);
28532856
if (!accel_shared_globals) {
2854-
zend_accel_error_noreturn(ACCEL_LOG_FATAL, "Insufficient shared memory!");
2857+
zend_accel_error_noreturn(ACCEL_LOG_FATAL,
2858+
"Insufficient shared memory for interned strings buffer! (tried to allocate %zu bytes)",
2859+
accel_shared_globals_size);
28552860
zend_shared_alloc_unlock();
28562861
return FAILURE;
28572862
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
--TEST--
2+
File cache error 001
3+
--EXTENSIONS--
4+
opcache
5+
posix
6+
pcntl
7+
--INI--
8+
opcache.enable_cli=1
9+
opcache.file_cache={TMP}
10+
opcache.log_verbosity_level=2
11+
--SKIPIF--
12+
<?php
13+
if (!posix_setrlimit(POSIX_RLIMIT_FSIZE, 1, 1)) die('skip Test requires setrlimit(RLIMIT_FSIZE) to work');
14+
if (ini_parse_quantity(ini_get('opcache.jit_buffer_size')) !== 0) die('skip File cache is disabled when JIT is on');
15+
?>
16+
--FILE--
17+
<?php
18+
19+
// Create new file to ensure that it's not cached accross test runs
20+
$file = tempnam(sys_get_temp_dir(), 'file_cache_error');
21+
register_shutdown_function(function () use ($file) {
22+
unlink($file);
23+
});
24+
file_put_contents($file, '<?php echo "OK";');
25+
touch($file, time() - 3600);
26+
27+
// Some systems will raise SIGXFSZ when RLIMIT_FSIZE is exceeded
28+
if (defined('SIGXFSZ')) {
29+
pcntl_signal(SIGXFSZ, SIG_IGN);
30+
}
31+
32+
// Should cause writing to cache file to fail
33+
var_dump(posix_setrlimit(POSIX_RLIMIT_FSIZE, 1, 1));
34+
35+
// Will attempt to write to cache file, and fail
36+
require $file;
37+
?>
38+
--EXPECTF--
39+
bool(true)
40+
%sWarning opcache cannot write to file %s: %s
41+
42+
OK

ext/opcache/tests/gh9259_001.phpt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
Bug GH-9259 001 (Setting opcache.interned_strings_buffer to a very high value leads to corruption of shm)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.interned_strings_buffer=131072
7+
opcache.log_verbosity_level=2
8+
opcache.enable_cli=1
9+
--FILE--
10+
<?php
11+
12+
echo 'OK';
13+
14+
?>
15+
--EXPECTF--
16+
%sWarning opcache.interned_strings_buffer must be less than or equal to 4095, 131072 given%s
17+
18+
OK

ext/opcache/tests/gh9259_002.phpt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
--TEST--
2+
Bug GH-9259 002 (Setting opcache.interned_strings_buffer to a very high value leads to corruption of shm)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.interned_strings_buffer=-1
7+
opcache.log_verbosity_level=2
8+
opcache.enable_cli=1
9+
--FILE--
10+
<?php
11+
12+
echo 'OK';
13+
14+
?>
15+
--EXPECTF--
16+
%sWarning opcache.interned_strings_buffer must be greater than or equal to 0, -1 given%s
17+
18+
OK

ext/opcache/tests/gh9259_003.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Bug GH-9259 003 (Setting opcache.interned_strings_buffer to a very high value leads to corruption of shm)
3+
--EXTENSIONS--
4+
opcache
5+
--INI--
6+
opcache.interned_strings_buffer=500
7+
opcache.enable_cli=1
8+
--FILE--
9+
<?php
10+
11+
echo 'OK';
12+
13+
?>
14+
--EXPECTF--
15+
%sFatal Error Insufficient shared memory for interned strings buffer! (tried to allocate %d bytes)

ext/opcache/zend_accelerator_module.c

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#define STRING_NOT_NULL(s) (NULL == (s)?"":s)
4141
#define MIN_ACCEL_FILES 200
4242
#define MAX_ACCEL_FILES 1000000
43+
#define MAX_INTERNED_STRINGS_BUFFER_SIZE ((zend_long)((UINT32_MAX-PLATFORM_ALIGNMENT)/(1024*1024)))
4344
#define TOKENTOSTR(X) #X
4445

4546
static zif_handler orig_file_exists = NULL;
@@ -78,6 +79,25 @@ static ZEND_INI_MH(OnUpdateMemoryConsumption)
7879
return SUCCESS;
7980
}
8081

82+
static ZEND_INI_MH(OnUpdateInternedStringsBuffer)
83+
{
84+
zend_long *p = (zend_long *) ZEND_INI_GET_ADDR();
85+
zend_long size = zend_ini_parse_quantity_warn(new_value, entry->name);
86+
87+
if (size < 0) {
88+
zend_accel_error(ACCEL_LOG_WARNING, "opcache.interned_strings_buffer must be greater than or equal to 0, " ZEND_LONG_FMT " given.\n", size);
89+
return FAILURE;
90+
}
91+
if (size > MAX_INTERNED_STRINGS_BUFFER_SIZE) {
92+
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);
93+
return FAILURE;
94+
}
95+
96+
*p = size;
97+
98+
return SUCCESS;
99+
}
100+
81101
static ZEND_INI_MH(OnUpdateMaxAcceleratedFiles)
82102
{
83103
zend_long *p = (zend_long *) ZEND_INI_GET_ADDR();
@@ -239,7 +259,7 @@ ZEND_INI_BEGIN()
239259

240260
STD_PHP_INI_ENTRY("opcache.log_verbosity_level" , "1" , PHP_INI_SYSTEM, OnUpdateLong, accel_directives.log_verbosity_level, zend_accel_globals, accel_globals)
241261
STD_PHP_INI_ENTRY("opcache.memory_consumption" , "128" , PHP_INI_SYSTEM, OnUpdateMemoryConsumption, accel_directives.memory_consumption, zend_accel_globals, accel_globals)
242-
STD_PHP_INI_ENTRY("opcache.interned_strings_buffer", "8" , PHP_INI_SYSTEM, OnUpdateLong, accel_directives.interned_strings_buffer, zend_accel_globals, accel_globals)
262+
STD_PHP_INI_ENTRY("opcache.interned_strings_buffer", "8" , PHP_INI_SYSTEM, OnUpdateInternedStringsBuffer, accel_directives.interned_strings_buffer, zend_accel_globals, accel_globals)
243263
STD_PHP_INI_ENTRY("opcache.max_accelerated_files" , "10000", PHP_INI_SYSTEM, OnUpdateMaxAcceleratedFiles, accel_directives.max_accelerated_files, zend_accel_globals, accel_globals)
244264
STD_PHP_INI_ENTRY("opcache.max_wasted_percentage" , "5" , PHP_INI_SYSTEM, OnUpdateMaxWastedPercentage, accel_directives.max_wasted_percentage, zend_accel_globals, accel_globals)
245265
STD_PHP_INI_ENTRY("opcache.consistency_checks" , "0" , PHP_INI_ALL , OnUpdateLong, accel_directives.consistency_checks, zend_accel_globals, accel_globals)

ext/opcache/zend_file_cache.c

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,23 +1004,56 @@ static char *zend_file_cache_get_bin_file_path(zend_string *script_path)
10041004
/**
10051005
* Helper function for zend_file_cache_script_store().
10061006
*
1007-
* @return true on success, false on error
1007+
* @return true on success, false on error and errno is set to indicate the cause of the error
10081008
*/
10091009
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)
10101010
{
1011+
ssize_t written;
1012+
const ssize_t total_size = (ssize_t)(sizeof(*info) + script->size + info->str_size);
1013+
10111014
#ifdef HAVE_SYS_UIO_H
10121015
const struct iovec vec[] = {
10131016
{ .iov_base = (void *)info, .iov_len = sizeof(*info) },
10141017
{ .iov_base = (void *)buf, .iov_len = script->size },
10151018
{ .iov_base = (void *)ZSTR_VAL(s), .iov_len = info->str_size },
10161019
};
10171020

1018-
return writev(fd, vec, sizeof(vec) / sizeof(vec[0])) == (ssize_t)(sizeof(*info) + script->size + info->str_size);
1021+
written = writev(fd, vec, sizeof(vec) / sizeof(vec[0]));
1022+
if (EXPECTED(written == total_size)) {
1023+
return true;
1024+
}
1025+
1026+
errno = written == -1 ? errno : EAGAIN;
1027+
return false;
10191028
#else
1020-
return ZEND_LONG_MAX >= (zend_long)(sizeof(*info) + script->size + info->str_size) &&
1021-
write(fd, info, sizeof(*info)) == sizeof(*info) &&
1022-
write(fd, buf, script->size) == script->size &&
1023-
write(fd, ZSTR_VAL(s), info->str_size) == info->str_size;
1029+
if (UNEXPECTED(ZEND_LONG_MAX < (zend_long)total_size)) {
1030+
# ifdef EFBIG
1031+
errno = EFBIG;
1032+
# else
1033+
errno = ERANGE;
1034+
# endif
1035+
return false;
1036+
}
1037+
1038+
written = write(fd, info, sizeof(*info));
1039+
if (UNEXPECTED(written != sizeof(*info))) {
1040+
errno = written == -1 ? errno : EAGAIN;
1041+
return false;
1042+
}
1043+
1044+
written = write(fd, buf, script->size);
1045+
if (UNEXPECTED(written != script->size)) {
1046+
errno = written == -1 ? errno : EAGAIN;
1047+
return false;
1048+
}
1049+
1050+
written = write(fd, ZSTR_VAL(s), info->str_size);
1051+
if (UNEXPECTED(written != info->str_size)) {
1052+
errno = written == -1 ? errno : EAGAIN;
1053+
return false;
1054+
}
1055+
1056+
return true;
10241057
#endif
10251058
}
10261059

@@ -1095,7 +1128,7 @@ int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm)
10951128
#endif
10961129

10971130
if (!zend_file_cache_script_write(fd, script, &info, buf, s)) {
1098-
zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot write to file '%s'\n", filename);
1131+
zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot write to file '%s': %s\n", filename, strerror(errno));
10991132
zend_string_release_ex(s, 0);
11001133
close(fd);
11011134
efree(mem);
@@ -1107,7 +1140,7 @@ int zend_file_cache_script_store(zend_persistent_script *script, bool in_shm)
11071140
zend_string_release_ex(s, 0);
11081141
efree(mem);
11091142
if (zend_file_cache_flock(fd, LOCK_UN) != 0) {
1110-
zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s'\n", filename);
1143+
zend_accel_error(ACCEL_LOG_WARNING, "opcache cannot unlock file '%s': %s\n", filename, strerror(errno));
11111144
}
11121145
close(fd);
11131146
efree(filename);

ext/opcache/zend_shared_alloc.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ static size_t zend_shared_alloc_get_largest_free_block(void)
328328
#define MIN_FREE_MEMORY 64*1024
329329

330330
#define SHARED_ALLOC_FAILED() do { \
331-
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)); \
331+
zend_accel_error(ACCEL_LOG_WARNING, "Not enough free shared space to allocate %zu bytes (%zu bytes free)", size, ZSMMG(shared_free)); \
332332
if (zend_shared_alloc_get_largest_free_block() < MIN_FREE_MEMORY) { \
333333
ZSMMG(memory_exhausted) = 1; \
334334
} \
@@ -339,6 +339,10 @@ void *zend_shared_alloc(size_t size)
339339
int i;
340340
unsigned int block_size = ZEND_ALIGNED_SIZE(size);
341341

342+
if (UNEXPECTED(block_size < size)) {
343+
zend_accel_error_noreturn(ACCEL_LOG_ERROR, "Possible integer overflow in shared memory allocation (%zu + %zu)", size, PLATFORM_ALIGNMENT);
344+
}
345+
342346
#if 1
343347
if (!ZCG(locked)) {
344348
zend_accel_error_noreturn(ACCEL_LOG_ERROR, "Shared memory lock not obtained");

0 commit comments

Comments
 (0)