Skip to content

Commit ff70469

Browse files
iluuu1994charmitro
authored andcommitted
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 phpGH-17564 Closes phpGH-17576
1 parent 2414cd9 commit ff70469

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
*/
@@ -1198,7 +1200,8 @@ zend_string *accel_make_persistent_key(zend_string *str)
11981200
char *key;
11991201
int key_length;
12001202

1201-
ZSTR_LEN(&ZCG(key)) = 0;
1203+
ZEND_ASSERT(GC_REFCOUNT(ZCG(key)) == 1);
1204+
ZSTR_LEN(ZCG(key)) = 0;
12021205

12031206
/* CWD and include_path don't matter for absolute file names and streams */
12041207
if (IS_ABSOLUTE_PATH(path, path_length)) {
@@ -1308,7 +1311,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13081311
}
13091312

13101313
/* Calculate key length */
1311-
if (UNEXPECTED((size_t)(cwd_len + path_length + include_path_len + 2) >= sizeof(ZCG(_key)))) {
1314+
if (UNEXPECTED((size_t)(cwd_len + path_length + include_path_len + 2) >= ZCG_KEY_LEN)) {
13121315
return NULL;
13131316
}
13141317

@@ -1317,7 +1320,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13171320
* since in itself, it may include colons (which we use to separate
13181321
* different components of the key)
13191322
*/
1320-
key = ZSTR_VAL(&ZCG(key));
1323+
key = ZSTR_VAL(ZCG(key));
13211324
memcpy(key, path, path_length);
13221325
key[path_length] = ':';
13231326
key_length = path_length + 1;
@@ -1341,7 +1344,7 @@ zend_string *accel_make_persistent_key(zend_string *str)
13411344
parent_script_len = ZSTR_LEN(parent_script);
13421345
while ((--parent_script_len > 0) && !IS_SLASH(ZSTR_VAL(parent_script)[parent_script_len]));
13431346

1344-
if (UNEXPECTED((size_t)(key_length + parent_script_len + 1) >= sizeof(ZCG(_key)))) {
1347+
if (UNEXPECTED((size_t)(key_length + parent_script_len + 1) >= ZCG_KEY_LEN)) {
13451348
return NULL;
13461349
}
13471350
key[key_length] = ':';
@@ -1350,11 +1353,9 @@ zend_string *accel_make_persistent_key(zend_string *str)
13501353
key_length += parent_script_len;
13511354
}
13521355
key[key_length] = '\0';
1353-
GC_SET_REFCOUNT(&ZCG(key), 1);
1354-
GC_TYPE_INFO(&ZCG(key)) = GC_STRING;
1355-
ZSTR_H(&ZCG(key)) = 0;
1356-
ZSTR_LEN(&ZCG(key)) = key_length;
1357-
return &ZCG(key);
1356+
ZSTR_H(ZCG(key)) = 0;
1357+
ZSTR_LEN(ZCG(key)) = key_length;
1358+
return ZCG(key);
13581359
}
13591360

13601361
/* not use_cwd */
@@ -2026,8 +2027,8 @@ zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type)
20262027
ZCG(cache_opline) == EG(current_execute_data)->opline))) {
20272028

20282029
persistent_script = ZCG(cache_persistent_script);
2029-
if (ZSTR_LEN(&ZCG(key))) {
2030-
key = &ZCG(key);
2030+
if (ZSTR_LEN(ZCG(key))) {
2031+
key = ZCG(key);
20312032
}
20322033

20332034
} else {
@@ -2581,7 +2582,7 @@ static zend_string* persistent_zend_resolve_path(zend_string *filename)
25812582
SHM_PROTECT();
25822583
HANDLE_UNBLOCK_INTERRUPTIONS();
25832584
} else {
2584-
ZSTR_LEN(&ZCG(key)) = 0;
2585+
ZSTR_LEN(ZCG(key)) = 0;
25852586
}
25862587
ZCG(cache_opline) = EG(current_execute_data) ? EG(current_execute_data)->opline : NULL;
25872588
ZCG(cache_persistent_script) = persistent_script;
@@ -2952,7 +2953,15 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
29522953
ZEND_TSRMLS_CACHE_UPDATE();
29532954
#endif
29542955
memset(accel_globals, 0, sizeof(zend_accel_globals));
2956+
accel_globals->key = zend_string_alloc(ZCG_KEY_LEN, true);
2957+
}
2958+
2959+
#ifdef ZTS
2960+
static void accel_globals_dtor(zend_accel_globals *accel_globals)
2961+
{
2962+
zend_string_free(accel_globals->key);
29552963
}
2964+
#endif
29562965

29572966
#ifdef HAVE_HUGE_CODE_PAGES
29582967
# ifndef _WIN32
@@ -3125,7 +3134,7 @@ static void accel_move_code_to_huge_pages(void)
31253134
static int accel_startup(zend_extension *extension)
31263135
{
31273136
#ifdef ZTS
3128-
accel_globals_id = ts_allocate_id(&accel_globals_id, sizeof(zend_accel_globals), (ts_allocate_ctor) accel_globals_ctor, NULL);
3137+
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);
31293138
#else
31303139
accel_globals_ctor(&accel_globals);
31313140
#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)