Skip to content

Commit 8ea9b04

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 Closes GH-17576
1 parent 556def7 commit 8ea9b04

File tree

3 files changed

+25
-16
lines changed

3 files changed

+25
-16
lines changed

NEWS

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ PHP NEWS
1717
. Fix may_have_extra_named_args flag for ZEND_AST_UNPACK. (nielsdos)
1818
. Fix NULL arithmetic in System V shared memory emulation for Windows. (cmb)
1919

20-
2120
- DOM:
2221
. Fixed bug GH-17500 (Segfault with requesting nodeName on nameless doctype).
2322
(nielsdos)
@@ -43,6 +42,8 @@ PHP NEWS
4342

4443
- Opcache:
4544
. Fixed bug GH-17307 (Internal closure causes JIT failure). (nielsdos)
45+
. Fixed bug GH-17564 (Potential UB when reading from / writing to struct
46+
padding). (ilutov)
4647

4748
- PDO:
4849
. Fixed a memory leak when the GC is used to free a PDOStatment. (Girgias)

ext/opcache/ZendAccelerator.c

Lines changed: 22 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,8 @@ zend_string *accel_make_persistent_key(zend_string *str)
11901192
char *key;
11911193
int key_length;
11921194

1193-
ZSTR_LEN(&ZCG(key)) = 0;
1195+
ZEND_ASSERT(GC_REFCOUNT(ZCG(key)) == 1);
1196+
ZSTR_LEN(ZCG(key)) = 0;
11941197

11951198
/* CWD and include_path don't matter for absolute file names and streams */
11961199
if (IS_ABSOLUTE_PATH(path, path_length)) {
@@ -1300,7 +1303,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13001303
}
13011304

13021305
/* Calculate key length */
1303-
if (UNEXPECTED((size_t)(cwd_len + path_length + include_path_len + 2) >= sizeof(ZCG(_key)))) {
1306+
if (UNEXPECTED((size_t)(cwd_len + path_length + include_path_len + 2) >= ZCG_KEY_LEN)) {
13041307
return NULL;
13051308
}
13061309

@@ -1309,7 +1312,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13091312
* since in itself, it may include colons (which we use to separate
13101313
* different components of the key)
13111314
*/
1312-
key = ZSTR_VAL(&ZCG(key));
1315+
key = ZSTR_VAL(ZCG(key));
13131316
memcpy(key, path, path_length);
13141317
key[path_length] = ':';
13151318
key_length = path_length + 1;
@@ -1333,7 +1336,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13331336
parent_script_len = ZSTR_LEN(parent_script);
13341337
while ((--parent_script_len > 0) && !IS_SLASH(ZSTR_VAL(parent_script)[parent_script_len]));
13351338

1336-
if (UNEXPECTED((size_t)(key_length + parent_script_len + 1) >= sizeof(ZCG(_key)))) {
1339+
if (UNEXPECTED((size_t)(key_length + parent_script_len + 1) >= ZCG_KEY_LEN)) {
13371340
return NULL;
13381341
}
13391342
key[key_length] = ':';
@@ -1342,11 +1345,9 @@ zend_string *accel_make_persistent_key(zend_string *str)
13421345
key_length += parent_script_len;
13431346
}
13441347
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);
1348+
ZSTR_H(ZCG(key)) = 0;
1349+
ZSTR_LEN(ZCG(key)) = key_length;
1350+
return ZCG(key);
13501351
}
13511352

13521353
/* not use_cwd */
@@ -2014,8 +2015,8 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
20142015
ZCG(cache_opline) == EG(current_execute_data)->opline))) {
20152016

20162017
persistent_script = ZCG(cache_persistent_script);
2017-
if (ZSTR_LEN(&ZCG(key))) {
2018-
key = &ZCG(key);
2018+
if (ZSTR_LEN(ZCG(key))) {
2019+
key = ZCG(key);
20192020
}
20202021

20212022
} else {
@@ -2555,7 +2556,7 @@ static zend_string* persistent_zend_resolve_path(zend_string *filename)
25552556
SHM_PROTECT();
25562557
HANDLE_UNBLOCK_INTERRUPTIONS();
25572558
} else {
2558-
ZSTR_LEN(&ZCG(key)) = 0;
2559+
ZSTR_LEN(ZCG(key)) = 0;
25592560
}
25602561
ZCG(cache_opline) = EG(current_execute_data) ? EG(current_execute_data)->opline : NULL;
25612562
ZCG(cache_persistent_script) = persistent_script;
@@ -2927,7 +2928,15 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
29272928
ZEND_TSRMLS_CACHE_UPDATE();
29282929
#endif
29292930
memset(accel_globals, 0, sizeof(zend_accel_globals));
2931+
accel_globals->key = zend_string_alloc(ZCG_KEY_LEN, true);
2932+
}
2933+
2934+
#ifdef ZTS
2935+
static void accel_globals_dtor(zend_accel_globals *accel_globals)
2936+
{
2937+
zend_string_free(accel_globals->key);
29302938
}
2939+
#endif
29312940

29322941
#ifdef HAVE_HUGE_CODE_PAGES
29332942
# ifndef _WIN32
@@ -3100,7 +3109,7 @@ static void accel_move_code_to_huge_pages(void)
31003109
static int accel_startup(zend_extension *extension)
31013110
{
31023111
#ifdef ZTS
3103-
accel_globals_id = ts_allocate_id(&accel_globals_id, sizeof(zend_accel_globals), (ts_allocate_ctor) accel_globals_ctor, NULL);
3112+
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);
31043113
#else
31053114
accel_globals_ctor(&accel_globals);
31063115
#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)