diff --git a/NEWS b/NEWS index 52dda8a9daa5..20defdfaecbd 100644 --- a/NEWS +++ b/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) diff --git a/Zend/tests/fibers/fiber-callable-object-outlives-caller.phpt b/Zend/tests/fibers/fiber-callable-object-outlives-caller.phpt new file mode 100644 index 000000000000..849bc7b6daa4 --- /dev/null +++ b/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-- +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 diff --git a/Zend/zend_fibers.c b/Zend/zend_fibers.c index d571a622e476..bfda9154647f 100644 --- a/Zend/zend_fibers.c +++ b/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)