summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNikita Popov <nikita.ppv@gmail.com>2019-10-28 10:23:20 +0100
committerNikita Popov <nikita.ppv@gmail.com>2019-10-28 10:27:32 +0100
commit16c49108763db251151b350e433dde6d1a076250 (patch)
treef9b2f85035a0fa2835c8730d27a892c96a081201
parent52499938142351d20a46c2b941e957ab0d618109 (diff)
downloadphp-git-16c49108763db251151b350e433dde6d1a076250.tar.gz
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.
-rw-r--r--NEWS2
-rw-r--r--Zend/tests/bug78752.phpt24
-rw-r--r--Zend/zend_generators.c20
3 files changed, 38 insertions, 8 deletions
diff --git a/NEWS b/NEWS
index ac9a2c2716..1d5c6d3d7c 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,8 @@ PHP NEWS
- Core:
. Fixed bug #78656 (Parse errors classified as highest log-level). (Erik
Lundin)
+ . Fixed bug #78752 (Segfault if GC triggered while generator stack frame is
+ being destroyed). (Nikita)
- COM:
. Fixed bug #78694 (Appending to a variant array causes segfault). (cmb)
diff --git a/Zend/tests/bug78752.phpt b/Zend/tests/bug78752.phpt
new file mode 100644
index 0000000000..367a44eab5
--- /dev/null
+++ b/Zend/tests/bug78752.phpt
@@ -0,0 +1,24 @@
+--TEST--
+Bug #78752: Segfault if GC triggered while generator stack frame is being destroyed
+--FILE--
+<?php
+
+function gen(&$gen) {
+ $a = new stdClass;
+ $a->a = $a;
+ $b = new stdClass;
+ $b->b = $b;
+ yield 1;
+}
+
+$gen = gen($gen);
+var_dump($gen->current());
+for ($i = 0; $i < 9999; $i++) {
+ $a = new stdClass;
+ $a->a = $a;
+}
+$gen->next();
+
+?>
+--EXPECT--
+int(1)
diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c
index 042ddbc178..cd30e14986 100644
--- a/Zend/zend_generators.c
+++ b/Zend/zend_generators.c
@@ -97,16 +97,18 @@ ZEND_API zend_execute_data* zend_generator_freeze_call_stack(zend_execute_data *
/* }}} */
static void zend_generator_cleanup_unfinished_execution(
- zend_generator *generator, uint32_t catch_op_num) /* {{{ */
+ zend_generator *generator, zend_execute_data *execute_data, uint32_t catch_op_num) /* {{{ */
{
- zend_execute_data *execute_data = generator->execute_data;
-
if (execute_data->opline != execute_data->func->op_array.opcodes) {
/* -1 required because we want the last run opcode, not the next to-be-run one. */
uint32_t op_num = execute_data->opline - execute_data->func->op_array.opcodes - 1;
if (UNEXPECTED(generator->frozen_call_stack)) {
+ /* Temporarily restore generator->execute_data if it has been NULLed out already. */
+ zend_execute_data *save_ex = generator->execute_data;
+ generator->execute_data = execute_data;
zend_generator_restore_call_stack(generator);
+ generator->execute_data = save_ex;
}
zend_cleanup_unfinished_execution(execute_data, op_num, catch_op_num);
}
@@ -117,6 +119,9 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
{
if (EXPECTED(generator->execute_data)) {
zend_execute_data *execute_data = generator->execute_data;
+ /* Null out execute_data early, to prevent double frees if GC runs while we're
+ * already cleaning up execute_data. */
+ generator->execute_data = NULL;
if (EX_CALL_INFO() & ZEND_CALL_HAS_SYMBOL_TABLE) {
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
return;
}
- zend_vm_stack_free_extra_args(generator->execute_data);
+ zend_vm_stack_free_extra_args(execute_data);
/* Some cleanups are only necessary if the generator was closed
* before it could finish execution (reach a return statement). */
if (UNEXPECTED(!finished_execution)) {
- zend_generator_cleanup_unfinished_execution(generator, 0);
+ zend_generator_cleanup_unfinished_execution(generator, execute_data, 0);
}
/* Free closure object */
@@ -154,8 +159,7 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished
generator->gc_buffer = NULL;
}
- efree(generator->execute_data);
- generator->execute_data = NULL;
+ efree(execute_data);
}
}
/* }}} */
@@ -216,7 +220,7 @@ static void zend_generator_dtor_storage(zend_object *object) /* {{{ */
if (finally_op_num) {
zval *fast_call;
- zend_generator_cleanup_unfinished_execution(generator, finally_op_num);
+ zend_generator_cleanup_unfinished_execution(generator, ex, finally_op_num);
fast_call = ZEND_CALL_VAR(ex, ex->func->op_array.opcodes[finally_op_end].op1.var);
Z_OBJ_P(fast_call) = EG(exception);