Skip to content

Commit 16c4910

Browse files
committed
Fix bug #78752
NULL out the execute_data before destroying it, otherwise GC may trigger while the execute_data is partially destroyed, resulting in double-frees. The handling of call stack unfreezing is a bit awkward because it's a ZEND_API function, so we can't change the signature.
1 parent 5249993 commit 16c4910

File tree

3 files changed

+38
-8
lines changed

3 files changed

+38
-8
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ PHP NEWS
55
- Core:
66
. Fixed bug #78656 (Parse errors classified as highest log-level). (Erik
77
Lundin)
8+
. Fixed bug #78752 (Segfault if GC triggered while generator stack frame is
9+
being destroyed). (Nikita)
810

911
- COM:
1012
. Fixed bug #78694 (Appending to a variant array causes segfault). (cmb)

Zend/tests/bug78752.phpt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
Bug #78752: Segfault if GC triggered while generator stack frame is being destroyed
3+
--FILE--
4+
<?php
5+
6+
function gen(&$gen) {
7+
$a = new stdClass;
8+
$a->a = $a;
9+
$b = new stdClass;
10+
$b->b = $b;
11+
yield 1;
12+
}
13+
14+
$gen = gen($gen);
15+
var_dump($gen->current());
16+
for ($i = 0; $i < 9999; $i++) {
17+
$a = new stdClass;
18+
$a->a = $a;
19+
}
20+
$gen->next();
21+
22+
?>
23+
--EXPECT--
24+
int(1)

Zend/zend_generators.c

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,18 @@ ZEND_API zend_execute_data* zend_generator_freeze_call_stack(zend_execute_data *
9797
/* }}} */
9898

9999
static void zend_generator_cleanup_unfinished_execution(
100-
zend_generator *generator, uint32_t catch_op_num) /* {{{ */
100+
zend_generator *generator, zend_execute_data *execute_data, uint32_t catch_op_num) /* {{{ */
101101
{
102-
zend_execute_data *execute_data = generator->execute_data;
103-
104102
if (execute_data->opline != execute_data->func->op_array.opcodes) {
105103
/* -1 required because we want the last run opcode, not the next to-be-run one. */
106104
uint32_t op_num = execute_data->opline - execute_data->func->op_array.opcodes - 1;
107105

108106
if (UNEXPECTED(generator->frozen_call_stack)) {
107+
/* Temporarily restore generator->execute_data if it has been NULLed out already. */
108+
zend_execute_data *save_ex = generator->execute_data;
109+
generator->execute_data = execute_data;
109110
zend_generator_restore_call_stack(generator);
111+
generator->execute_data = save_ex;
110112
}
111113
zend_cleanup_unfinished_execution(execute_data, op_num, catch_op_num);
112114
}
@@ -117,6 +119,9 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
117119
{
118120
if (EXPECTED(generator->execute_data)) {
119121
zend_execute_data *execute_data = generator->execute_data;
122+
/* Null out execute_data early, to prevent double frees if GC runs while we're
123+
* already cleaning up execute_data. */
124+
generator->execute_data = NULL;
120125

121126
if (EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE) {
122127
zend_clean_and_cache_symbol_table(execute_data->symbol_table);
@@ -135,12 +140,12 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
135140
return;
136141
}
137142

138-
zend_vm_stack_free_extra_args(generator->execute_data);
143+
zend_vm_stack_free_extra_args(execute_data);
139144

140145
/* Some cleanups are only necessary if the generator was closed
141146
* before it could finish execution (reach a return statement). */
142147
if (UNEXPECTED(!finished_execution)) {
143-
zend_generator_cleanup_unfinished_execution(generator, 0);
148+
zend_generator_cleanup_unfinished_execution(generator, execute_data, 0);
144149
}
145150

146151
/* Free closure object */
@@ -154,8 +159,7 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
154159
generator->gc_buffer = NULL;
155160
}
156161

157-
efree(generator->execute_data);
158-
generator->execute_data = NULL;
162+
efree(execute_data);
159163
}
160164
}
161165
/* }}} */
@@ -216,7 +220,7 @@ static void zend_generator_dtor_storage(zend_object *object) /* {{{ */
216220
if (finally_op_num) {
217221
zval *fast_call;
218222

219-
zend_generator_cleanup_unfinished_execution(generator, finally_op_num);
223+
zend_generator_cleanup_unfinished_execution(generator, ex, finally_op_num);
220224

221225
fast_call = ZEND_CALL_VAR(ex, ex->func->op_array.opcodes[finally_op_end].op1.var);
222226
Z_OBJ_P(fast_call) = EG(exception);

0 commit comments

Comments
 (0)