Skip to content

Commit af2110e

Browse files
committed
Fix freeing of incompletely initialized closures
Addref to relevant fields before allocating any memory. Also only set/remove the ZEND_ACC_HEAP_RT_CACHE flag after allocating memory. Fixes GH-12073 Closes GH-12074
1 parent e3df233 commit af2110e

File tree

3 files changed

+37
-6
lines changed

3 files changed

+37
-6
lines changed

NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ PHP NEWS
66
. Fixed bug GH-11937 (Constant ASTs containing objects). (ilutov)
77
. Fixed bug GH-11790 (On riscv64 require libatomic if actually needed).
88
(Jeremie Courreges-Anglas)
9+
. Fixed bug GH-12073 (Segfault when freeing incompletely initialized
10+
closures). (ilutov)
911

1012
- DOM:
1113
. Fix memory leak when setting an invalid DOMDocument encoding. (nielsdos)

Zend/tests/gh12073.phpt

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
GH-12073: Freeing of non-ZMM pointer of incompletely allocated closure
3+
--SKIPIF--
4+
<?php
5+
if (getenv("USE_ZEND_ALLOC") === "0" && getenv("USE_TRACKED_ALLOC") !== "1") {
6+
die("skip Zend MM disabled");
7+
}
8+
?>
9+
--FILE--
10+
<?php
11+
12+
function test() {
13+
$k = 1;
14+
return function () use ($k) {
15+
foo();
16+
};
17+
}
18+
19+
ini_set('memory_limit', '2M');
20+
21+
$array = [];
22+
for ($i = 0; $i < 10_000; $i++) {
23+
$array[] = test();
24+
}
25+
26+
?>
27+
--EXPECTF--
28+
Fatal error: Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes) in %s on line %d

Zend/zend_closures.c

+7-6
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,11 @@ static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_en
689689
closure->func.common.fn_flags |= ZEND_ACC_CLOSURE;
690690
closure->func.common.fn_flags &= ~ZEND_ACC_IMMUTABLE;
691691

692+
zend_string_addref(closure->func.op_array.function_name);
693+
if (closure->func.op_array.refcount) {
694+
(*closure->func.op_array.refcount)++;
695+
}
696+
692697
/* For fake closures, we want to reuse the static variables of the original function. */
693698
if (!is_fake) {
694699
if (closure->func.op_array.static_variables) {
@@ -722,24 +727,20 @@ static void zend_create_closure_ex(zval *res, zend_function *func, zend_class_en
722727
if (func->common.scope != scope) {
723728
func->common.scope = scope;
724729
}
725-
closure->func.op_array.fn_flags &= ~ZEND_ACC_HEAP_RT_CACHE;
726730
ptr = zend_arena_alloc(&CG(arena), func->op_array.cache_size);
727731
ZEND_MAP_PTR_SET(func->op_array.run_time_cache, ptr);
728732
ZEND_MAP_PTR_SET(closure->func.op_array.run_time_cache, ptr);
733+
closure->func.op_array.fn_flags &= ~ZEND_ACC_HEAP_RT_CACHE;
729734
} else {
730735
/* Otherwise, we use a non-shared runtime cache */
731-
closure->func.op_array.fn_flags |= ZEND_ACC_HEAP_RT_CACHE;
732736
ptr = emalloc(sizeof(void*) + func->op_array.cache_size);
733737
ZEND_MAP_PTR_INIT(closure->func.op_array.run_time_cache, ptr);
734738
ptr = (char*)ptr + sizeof(void*);
735739
ZEND_MAP_PTR_SET(closure->func.op_array.run_time_cache, ptr);
740+
closure->func.op_array.fn_flags |= ZEND_ACC_HEAP_RT_CACHE;
736741
}
737742
memset(ptr, 0, func->op_array.cache_size);
738743
}
739-
zend_string_addref(closure->func.op_array.function_name);
740-
if (closure->func.op_array.refcount) {
741-
(*closure->func.op_array.refcount)++;
742-
}
743744
} else {
744745
memcpy(&closure->func, func, sizeof(zend_internal_function));
745746
closure->func.common.fn_flags |= ZEND_ACC_CLOSURE;

0 commit comments

Comments
 (0)