Fiber: hold the object a callable resolved to in the creating frame

NEWS

@@ -32,6 +32,9 @@ PHP                                                                        NEWS
     the end of their mapping. (Ilia Alshanetsky)
   . Fixed bug GH-24050 (GC frees an object still held by a resurrected closure
     when (object) and use share an array). (David Carlier)
+  . Fixed use-after-free when a Fiber callable resolves to $this of the method
+    creating the Fiber ([A::class, 'm'] or "A::m") and that object dies before
+    the Fiber runs. (Edmond Dantes)
   . Fixed exception thrown by destructor during GC in a Fiber not being rethrown
     into the frame that triggered the GC. (Nicolas Grekas)
 

Zend/tests/fibers/fiber-callable-object-outlives-caller.phpt

@@ -0,0 +1,85 @@
+--TEST--
+Fiber holds the object a callable resolved to in the creating method ($this for [A::class, 'm'] or "A::m")
+--FILE--
+<?php
+class A {
+    public ?Fiber $fiber = null;
+
+    public function __construct(public string $name) {}
+
+    public function m() {
+        echo "m on {$this->name}\n";
+    }
+
+    public function waits() {
+        Fiber::suspend();
+        echo "resumed on {$this->name}\n";
+    }
+
+    public function make(string $method) {
+        return new Fiber([A::class, $method]);
+    }
+
+    public function makeFromString() {
+        return new Fiber("A::m");
+    }
+
+    public function keep() {
+        $this->fiber = new Fiber([A::class, 'm']);
+    }
+
+    public function __destruct() {
+        echo "released {$this->name}\n";
+    }
+}
+
+echo "-- started after its creator's last reference went\n";
+$fiber = (new A("a1"))->make('m');
+$fiber->start();
+echo "after start\n";
+
+echo "-- a string callable\n";
+$fiber = (new A("a2"))->makeFromString();
+$fiber->start();
+echo "after start\n";
+
+echo "-- never started, released with the fiber\n";
+$fiber = (new A("a3"))->make('m');
+unset($fiber);
+echo "after unset\n";
+
+echo "-- suspended while its object has no other reference\n";
+$a = new A("a4");
+$fiber = $a->make('waits');
+$fiber->start();
+unset($a);
+$fiber->resume();
+echo "after resume\n";
+
+echo "-- a cycle through the fiber is collected\n";
+$a = new A("a5");
+$a->keep();
+unset($a);
+var_dump(gc_collect_cycles());
+echo "end\n";
+?>
+--EXPECT--
+-- started after its creator's last reference went
+m on a1
+released a1
+after start
+-- a string callable
+m on a2
+released a2
+after start
+-- never started, released with the fiber
+released a3
+after unset
+-- suspended while its object has no other reference
+resumed on a4
+released a4
+after resume
+-- a cycle through the fiber is collected
+released a5
+int(2)
+end

Zend/zend_fibers.c

@@ -564,6 +564,39 @@ static void zend_fiber_cleanup(zend_fiber_context *context)
 	fiber->caller = NULL;
 }
 
+static void zend_fiber_addref_callable(zend_fiber *fiber)
+{
+	// Keep a reference to closures or callable objects while the fiber is running.
+	Z_TRY_ADDREF(fiber->fci.function_name);
+
+	/* For [A::class, 'm'] or "A::m" inside a method of A, fci.object is the caller's $this. */
+	if (fiber->fci.object != NULL) {
+		GC_ADDREF(fiber->fci.object);
+	}
+}
+
+static void zend_fiber_get_gc_callable(zend_fiber *fiber, zend_get_gc_buffer *buf)
+{
+	zend_get_gc_buffer_add_zval(buf, &fiber->fci.function_name);
+
+	if (fiber->fci.object != NULL) {
+		zend_get_gc_buffer_add_obj(buf, fiber->fci.object);
+	}
+}
+
+static void zend_fiber_release_callable(zend_fiber *fiber)
+{
+	zval_ptr_dtor(&fiber->fci.function_name);
+	ZVAL_UNDEF(&fiber->fci.function_name);
+	fiber->fci_cache = empty_fcall_info_cache;
+
+	if (fiber->fci.object != NULL) {
+		zend_object *object = fiber->fci.object;
+		fiber->fci.object = NULL;
+		OBJ_RELEASE(object);
+	}
+}
+
 static ZEND_STACK_ALIGNED void zend_fiber_execute(zend_fiber_transfer *transfer)
 {
 	ZEND_ASSERT(Z_TYPE(transfer->value) == IS_NULL && "Initial transfer value to fiber context must be NULL");
@@ -609,8 +642,7 @@ static ZEND_STACK_ALIGNED void zend_fiber_execute(zend_fiber_transfer *transfer)
 		zend_call_function(&fiber->fci, &fiber->fci_cache);
 
 		/* Cleanup callback and unset field to prevent GC / duplicate dtor issues. */
-		zval_ptr_dtor(&fiber->fci.function_name);
-		ZVAL_UNDEF(&fiber->fci.function_name);
+		zend_fiber_release_callable(fiber);
 
 		if (EG(exception)) {
 			if (!(fiber->flags & ZEND_FIBER_FLAG_DESTROYED)
@@ -809,7 +841,7 @@ static void zend_fiber_object_free(zend_object *object)
 {
 	zend_fiber *fiber = (zend_fiber *) object;
 
-	zval_ptr_dtor(&fiber->fci.function_name);
+	zend_fiber_release_callable(fiber);
 	zval_ptr_dtor(&fiber->result);
 
 	zend_object_std_dtor(&fiber->std);
@@ -820,7 +852,7 @@ static HashTable *zend_fiber_object_gc(zend_object *object, zval **table, int *n
 	zend_fiber *fiber = (zend_fiber *) object;
 	zend_get_gc_buffer *buf = zend_get_gc_buffer_create();
 
-	zend_get_gc_buffer_add_zval(buf, &fiber->fci.function_name);
+	zend_fiber_get_gc_callable(fiber, buf);
 	zend_get_gc_buffer_add_zval(buf, &fiber->result);
 
 	if (fiber->context.status != ZEND_FIBER_STATUS_SUSPENDED || fiber->caller != NULL) {
@@ -888,8 +920,7 @@ ZEND_METHOD(Fiber, __construct)
 	fiber->fci = fci;
 	fiber->fci_cache = fcc;
 
-	// Keep a reference to closures or callable objects while the fiber is running.
-	Z_TRY_ADDREF(fiber->fci.function_name);
+	zend_fiber_addref_callable(fiber);
 }
 
 ZEND_METHOD(Fiber, start)