Fiber: hold the object a callable resolved to in the creating frame (#24134)

Inside a method of A, new Fiber([A::class, 'm']) or new Fiber("A::m")
resolves the callable to the method's $this. Fiber::__construct() kept
that object in fci.object and fci_cache.object without a reference, so
once the A died before start(), the fiber called A::m() on freed memory.

The fiber now holds fci.object for every callable, releases it with the
callable (after the call, or when the fiber is freed unstarted) and
reports it to the GC, so a fiber stored in a property of that object is
a collectable cycle.

A __call trampoline is not covered: it is resolved again at start(),
against the frame that calls start().

Author: EdmondDantes

NEWS

@@ -121,6 +121,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)
   . Fixed bug GH-24139 (NULL pointer dereference in php_ini.c when

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)
@@ -812,7 +844,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);
@@ -823,7 +855,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) {
@@ -891,8 +923,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)