Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

### Fixed
- **#176 Nested closures shared across worker threads → cross-thread `run_time_cache` use-after-free (SEGV).** A transferred closure's nested closures (`op_array.dynamic_func_defs`) stayed shared with the persistent snapshot, so instantiating one on multiple workers raced on its run-time cache (e.g. a coroutine spawned in a `ThreadPool` bootloader across `reload()`). The load path now deep-copies the op_array per worker when it carries nested definitions.
- **#173 Reactor: use-after-free when an IO request is disposed while its libuv operation is still in flight** (seen as an intermittent access violation on Windows process teardown in `io/042`). An awaiter that abandons its request — `stream_set_timeout()` + `fgets()` where the timer wins, or a cancelled coroutine parked on a write/flush/stat/sendfile — called `req->dispose(req)` while libuv still referenced the request, leaving several distinct UAFs:
- *Armed stream read* (the `io/042` crash): `libuv_io_req_dispose` freed the request but left `io->active_req` pointing at it with `uv_read_start` still armed. The subsequent `fclose()` took the no-subscribers path in `libuv_io_close` (the timeout had already cleaned the waker), so the stale pointer survived until the `uv_close` callback drained — often as late as request shutdown — where the `active_req` backstop in `libuv_io_event_dispose` called `dispose()` through freed memory. Dispose now stops the reader and detaches `io->active_req`. The same detach was added to `udp_req_dispose` for an abandoned `recvfrom` (armed `uv_udp_recv_start`).
- *In-flight `uv_write` / `uv_fs_read` / `uv_fs_write` / `uv_fs_fsync` / `uv_fs_fstat` / sendfile*: these cannot be cancelled synchronously, so dispose now defers the free with an `UV_IN_FLIGHT`/`DISPOSE_PENDING` rendezvous (mirroring the existing UDP-sendto and DNS ones) — the completion callback finishes the deferred dispose and skips the NOTIFY; a queued-but-not-started threadpool op is `uv_cancel`ed outright. `io_file_stat_cb` additionally skips writing the stat result into the vanished awaiter's buffer.
Expand Down
39 changes: 39 additions & 0 deletions tests/thread_pool/078-reload_nested_closure_uaf.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--TEST--
ThreadPool - reload() with a bootloader that spawns a nested closure coroutine (no cross-worker run_time_cache UAF, issue #176)
--SKIPIF--
<?php
if (!PHP_ZTS) die('skip ZTS required');
if (!class_exists('Async\ThreadPool')) die('skip ThreadPool not available');
?>
--FILE--
<?php

use Async\ThreadPool;
use function Async\delay;
use function Async\await;
use function Async\spawn;

// Bootloader spawns a nested closure as a long-lived coroutine on every worker.
// Before #176 the nested op_array was shared → cross-worker run_time_cache UAF.
$boot = function () {
spawn(function () {
static $ticks = 0;
while (true) { $ticks++; delay(50); }
});
};

$pool = new ThreadPool(4, 0, $boot);
delay(200);
for ($i = 0; $i < 6; $i++) {
$pool->reload();
delay(80);
}

var_dump(await($pool->submit(fn () => 42)) === 42);
$pool->close();

echo "done\n";
?>
--EXPECT--
bool(true)
done
51 changes: 51 additions & 0 deletions tests/thread_pool/079-submit_nested_closure_per_worker.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
--TEST--
ThreadPool - submit() tasks that declare nested closures run per-worker without a shared run_time_cache UAF (issue #176)
--SKIPIF--
<?php
if (!PHP_ZTS) die('skip ZTS required');
if (!class_exists('Async\ThreadPool')) die('skip ThreadPool not available');
?>
--FILE--
<?php

use Async\ThreadPool;
use function Async\delay;
use function Async\await;

$pool = new ThreadPool(6, 0, function () {});
delay(80);

// Each task declares its own nested closures across six workers + a reload.
// Fresh instances per task, so the static counter is always 1 → each returns 43.
$task = function () {
$mul = function ($x) { return $x * 2; };
$inc = fn ($y) => $y + 1;
$count = function () { static $n = 0; return ++$n; };
return $mul(20) + $inc(1) + $count();
};

$futures = [];
for ($i = 0; $i < 12; $i++) {
$futures[] = $pool->submit($task);
}

$pool->reload();
delay(60);

for ($i = 0; $i < 12; $i++) {
$futures[] = $pool->submit($task);
}

$ok = true;
foreach ($futures as $future) {
$ok = $ok && (await($future) === 43);
}
var_dump($ok);

$pool->close();

echo "done\n";
?>
--EXPECT--
bool(true)
done
20 changes: 17 additions & 3 deletions thread.c
Original file line number Diff line number Diff line change
Expand Up @@ -2324,6 +2324,12 @@ static void op_array_to_emalloc(zend_op_array *op_array)
ZSTR_VAL(op_array->doc_comment), ZSTR_LEN(op_array->doc_comment), 0);
}

/* Nested defs carry the snapshot's persistent immutable static_variables
* (refcount 2); dup into a worker-local array destroy_op_array can free. */
if (op_array->static_variables) {
op_array->static_variables = zend_array_dup(op_array->static_variables);
}

/* dynamic_func_defs — outer closures with nested function/closure
* definitions point into the snapshot's arena via dynamic_func_defs[i].
* Without an emalloc-side copy, async_thread_snapshot_destroy frees those
Expand Down Expand Up @@ -2475,6 +2481,12 @@ void async_thread_create_closure(
zend_array_destroy(loaded_vars);
}

/* Self-contain the op_array per worker when it has nested defs, else they
* stay shared with the snapshot and race on run_time_cache (#176). */
async_zend_closure_t *closure = (async_zend_closure_t *) Z_OBJ_P(closure_zv);
if (closure->func.op_array.num_dynamic_func_defs != 0) {
op_array_to_emalloc(&closure->func.op_array);
}
}

/**
Expand Down Expand Up @@ -3243,10 +3255,12 @@ static zend_object *closure_transfer_obj(
return fallback;
}

/* Copy op_array internals from persistent arena into emalloc
* so the closure is fully self-contained */
/* Detach the top-level from the snapshot arena. Nested-def closures
* were already fully copied by async_thread_create_closure (#176). */
async_zend_closure_t *closure = (async_zend_closure_t *) Z_OBJ(closure_zv);
op_array_to_emalloc(&closure->func.op_array);
if (closure->func.op_array.num_dynamic_func_defs == 0) {
op_array_to_emalloc(&closure->func.op_array);
}

async_thread_snapshot_destroy(snapshot);
/* Clear the slot so the persistent shell's later release path
Expand Down
Loading