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.

diff --git a/NEWS b/NEWS
index ac9a2c27167cd975feb7700790ddb682fcef88d7..1d5c6d3d7c33c4c2bb820718751f6029e1f3deb8 100644 (file)
--- 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 (file)
index 0000000..367a44e
--- /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 042ddbc1784bf54af287aba4072ca73b7adc0a85..cd30e149867757ed72514b94fb9da3971326dd88 100644 (file)
--- 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);