Skip to content

Commit 353f7ff

Browse files
committed
Delref only after successful allocation
Otherwise we may have inconsistent refcounts after OOM. I expect this problem is much more prevalent, but this at least fixes some string/array separation cases. Fixes oss-fuzz #30999.
1 parent 0cdc634 commit 353f7ff

File tree

2 files changed

+13
-5
lines changed

2 files changed

+13
-5
lines changed

Zend/zend_operators.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2323,8 +2323,10 @@ static void ZEND_FASTCALL increment_string(zval *str) /* {{{ */
23232323
Z_STR_P(str) = zend_string_init(Z_STRVAL_P(str), Z_STRLEN_P(str), 0);
23242324
Z_TYPE_INFO_P(str) = IS_STRING_EX;
23252325
} else if (Z_REFCOUNT_P(str) > 1) {
2326-
Z_DELREF_P(str);
2326+
/* Only release string after allocation succeeded. */
2327+
zend_string *orig_str = Z_STR_P(str);
23272328
Z_STR_P(str) = zend_string_init(Z_STRVAL_P(str), Z_STRLEN_P(str), 0);
2329+
GC_DELREF(orig_str);
23282330
} else {
23292331
zend_string_forget_hash_val(Z_STR_P(str));
23302332
}

Zend/zend_types.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,7 @@ static zend_always_inline zend_uchar zval_get_type(const zval* pz) {
625625
#define GC_ADDREF_EX(p, rc) zend_gc_addref_ex(&(p)->gc, rc)
626626
#define GC_DELREF_EX(p, rc) zend_gc_delref_ex(&(p)->gc, rc)
627627
#define GC_TRY_ADDREF(p) zend_gc_try_addref(&(p)->gc)
628+
#define GC_TRY_DELREF(p) zend_gc_try_delref(&(p)->gc)
628629

629630
#define GC_TYPE_MASK 0x0000000f
630631
#define GC_FLAGS_MASK 0x000003f0
@@ -1180,6 +1181,13 @@ static zend_always_inline void zend_gc_try_addref(zend_refcounted_h *p) {
11801181
}
11811182
}
11821183

1184+
static zend_always_inline void zend_gc_try_delref(zend_refcounted_h *p) {
1185+
if (!(p->u.type_info & GC_IMMUTABLE)) {
1186+
ZEND_RC_MOD_CHECK(p);
1187+
--p->refcount;
1188+
}
1189+
}
1190+
11831191
static zend_always_inline uint32_t zend_gc_delref(zend_refcounted_h *p) {
11841192
ZEND_ASSERT(p->refcount > 0);
11851193
ZEND_RC_MOD_CHECK(p);
@@ -1351,20 +1359,18 @@ static zend_always_inline uint32_t zval_delref_p(zval* pz) {
13511359
zend_string *_str = Z_STR_P(_zv); \
13521360
ZEND_ASSERT(Z_REFCOUNTED_P(_zv)); \
13531361
ZEND_ASSERT(!ZSTR_IS_INTERNED(_str)); \
1354-
Z_DELREF_P(_zv); \
13551362
ZVAL_NEW_STR(_zv, zend_string_init( \
13561363
ZSTR_VAL(_str), ZSTR_LEN(_str), 0)); \
1364+
GC_DELREF(_str); \
13571365
} \
13581366
} while (0)
13591367

13601368
#define SEPARATE_ARRAY(zv) do { \
13611369
zval *__zv = (zv); \
13621370
zend_array *_arr = Z_ARR_P(__zv); \
13631371
if (UNEXPECTED(GC_REFCOUNT(_arr) > 1)) { \
1364-
if (Z_REFCOUNTED_P(__zv)) { \
1365-
GC_DELREF(_arr); \
1366-
} \
13671372
ZVAL_ARR(__zv, zend_array_dup(_arr)); \
1373+
GC_TRY_DELREF(_arr); \
13681374
} \
13691375
} while (0)
13701376

0 commit comments

Comments
 (0)