Skip to content

Commit f8861be

Browse files
committed
Fix inline zend_string using struct padding
As explained by Snape3058: On 64-bit machines, we typically have 7 bytes of padding between the zend_string.val[0] char and the following char[]. This means that zend_string.val[1-7] write to and read from the struct padding, which is a bad idea. Allocate the given string separately instead. Fixes GH-17564
1 parent 2e02cdf commit f8861be

File tree

2 files changed

+24
-15
lines changed

2 files changed

+24
-15
lines changed

ext/opcache/ZendAccelerator.c

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ static void preload_restart(void);
141141
# define LOCKVAL(v) (ZCSG(v))
142142
#endif
143143

144+
#define ZCG_KEY_LEN (MAXPATHLEN * 8)
145+
144146
/**
145147
* Clear AVX/SSE2-aligned memory.
146148
*/
@@ -1190,7 +1192,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
11901192
char *key;
11911193
int key_length;
11921194

1193-
ZSTR_LEN(&ZCG(key)) = 0;
1195+
ZSTR_LEN(ZCG(key)) = 0;
11941196

11951197
/* CWD and include_path don't matter for absolute file names and streams */
11961198
if (IS_ABSOLUTE_PATH(path, path_length)) {
@@ -1300,7 +1302,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13001302
}
13011303

13021304
/* Calculate key length */
1303-
if (UNEXPECTED((size_t)(cwd_len + path_length + include_path_len + 2) >= sizeof(ZCG(_key)))) {
1305+
if (UNEXPECTED((size_t)(cwd_len + path_length + include_path_len + 2) >= ZCG_KEY_LEN)) {
13041306
return NULL;
13051307
}
13061308

@@ -1309,7 +1311,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13091311
* since in itself, it may include colons (which we use to separate
13101312
* different components of the key)
13111313
*/
1312-
key = ZSTR_VAL(&ZCG(key));
1314+
key = ZSTR_VAL(ZCG(key));
13131315
memcpy(key, path, path_length);
13141316
key[path_length] = ':';
13151317
key_length = path_length + 1;
@@ -1333,7 +1335,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13331335
parent_script_len = ZSTR_LEN(parent_script);
13341336
while ((--parent_script_len > 0) && !IS_SLASH(ZSTR_VAL(parent_script)[parent_script_len]));
13351337

1336-
if (UNEXPECTED((size_t)(key_length + parent_script_len + 1) >= sizeof(ZCG(_key)))) {
1338+
if (UNEXPECTED((size_t)(key_length + parent_script_len + 1) >= ZCG_KEY_LEN)) {
13371339
return NULL;
13381340
}
13391341
key[key_length] = ':';
@@ -1342,11 +1344,11 @@ zend_string *accel_make_persistent_key(zend_string *str)
13421344
key_length += parent_script_len;
13431345
}
13441346
key[key_length] = '\0';
1345-
GC_SET_REFCOUNT(&ZCG(key), 1);
1346-
GC_TYPE_INFO(&ZCG(key)) = GC_STRING;
1347-
ZSTR_H(&ZCG(key)) = 0;
1348-
ZSTR_LEN(&ZCG(key)) = key_length;
1349-
return &ZCG(key);
1347+
GC_SET_REFCOUNT(ZCG(key), 1);
1348+
GC_TYPE_INFO(ZCG(key)) = (GC_STRING|IS_STR_PERSISTENT);
1349+
ZSTR_H(ZCG(key)) = 0;
1350+
ZSTR_LEN(ZCG(key)) = key_length;
1351+
return ZCG(key);
13501352
}
13511353

13521354
/* not use_cwd */
@@ -2014,8 +2016,8 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
20142016
ZCG(cache_opline) == EG(current_execute_data)->opline))) {
20152017

20162018
persistent_script = ZCG(cache_persistent_script);
2017-
if (ZSTR_LEN(&ZCG(key))) {
2018-
key = &ZCG(key);
2019+
if (ZSTR_LEN(ZCG(key))) {
2020+
key = ZCG(key);
20192021
}
20202022

20212023
} else {
@@ -2555,7 +2557,7 @@ static zend_string* persistent_zend_resolve_path(zend_string *filename)
25552557
SHM_PROTECT();
25562558
HANDLE_UNBLOCK_INTERRUPTIONS();
25572559
} else {
2558-
ZSTR_LEN(&ZCG(key)) = 0;
2560+
ZSTR_LEN(ZCG(key)) = 0;
25592561
}
25602562
ZCG(cache_opline) = EG(current_execute_data) ? EG(current_execute_data)->opline : NULL;
25612563
ZCG(cache_persistent_script) = persistent_script;
@@ -2927,7 +2929,15 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
29272929
ZEND_TSRMLS_CACHE_UPDATE();
29282930
#endif
29292931
memset(accel_globals, 0, sizeof(zend_accel_globals));
2932+
accel_globals->key = zend_string_alloc(ZCG_KEY_LEN, true);
2933+
}
2934+
2935+
#ifdef ZTS
2936+
static void accel_globals_dtor(zend_accel_globals *accel_globals)
2937+
{
2938+
zend_string_free(accel_globals->key);
29302939
}
2940+
#endif
29312941

29322942
#ifdef HAVE_HUGE_CODE_PAGES
29332943
# ifndef _WIN32
@@ -3100,7 +3110,7 @@ static void accel_move_code_to_huge_pages(void)
31003110
static int accel_startup(zend_extension *extension)
31013111
{
31023112
#ifdef ZTS
3103-
accel_globals_id = ts_allocate_id(&accel_globals_id, sizeof(zend_accel_globals), (ts_allocate_ctor) accel_globals_ctor, NULL);
3113+
accel_globals_id = ts_allocate_id(&accel_globals_id, sizeof(zend_accel_globals), (ts_allocate_ctor) accel_globals_ctor, (ts_allocate_dtor) accel_globals_dtor);
31043114
#else
31053115
accel_globals_ctor(&accel_globals);
31063116
#endif

ext/opcache/ZendAccelerator.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -223,8 +223,7 @@ typedef struct _zend_accel_globals {
223223
const zend_op *cache_opline;
224224
zend_persistent_script *cache_persistent_script;
225225
/* preallocated buffer for keys */
226-
zend_string key;
227-
char _key[MAXPATHLEN * 8];
226+
zend_string *key;
228227
} zend_accel_globals;
229228

230229
typedef struct _zend_string_table {

0 commit comments

Comments
 (0)