From 0cbdfe8cb10164ed09e3ecb3e7713cc60430f993 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+edmonddantes@users.noreply.github.com> Date: Sat, 4 Jul 2026 17:14:44 +0000 Subject: [PATCH] fix(thread): deep-copy nested closures per worker to stop a cross-thread run_time_cache UAF (#176) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A closure transferred to a worker thread got a per-worker copy of its top-level op_array, but its nested closures (op_array.dynamic_func_defs) stayed shared with the persistent snapshot: the load-path memcpy only reset the top-level run_time_cache. Instantiated on more than one worker, zend_create_closure writes a per-thread arena run_time_cache pointer into that shared op_array — read from another worker, or after a retired worker's arena is freed (a long-lived coroutine spawned in a ThreadPool bootloader across reload()) — a cross-thread use-after-free that SEGVs. async_thread_create_closure now self-contains the whole op_array via op_array_to_emalloc whenever it carries nested definitions, so every nested def becomes worker-local (immutable cleared, fresh run_time_cache). op_array_to_emalloc additionally duplicates the nested static_variables template — the snapshot stores it as a persistent immutable array (refcount 2), which destroy_op_array cannot free — into a worker-local mutable copy. The transfer path (closure_transfer_obj) now only detaches the top-level when there are no nested defs, since the load path already did the full copy. Closures without nested definitions keep the cheap borrow and are unaffected. Adds tests/thread_pool/078 (bootloader nested-closure coroutine across reload) and 079 (submitted tasks declaring nested closures per worker); both SEGV without the fix and pass clean under ASAN. --- CHANGELOG.md | 1 + .../078-reload_nested_closure_uaf.phpt | 39 ++++++++++++++ .../079-submit_nested_closure_per_worker.phpt | 51 +++++++++++++++++++ thread.c | 20 ++++++-- 4 files changed, 108 insertions(+), 3 deletions(-) create mode 100644 tests/thread_pool/078-reload_nested_closure_uaf.phpt create mode 100644 tests/thread_pool/079-submit_nested_closure_per_worker.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index 32e102fd..076d8391 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/tests/thread_pool/078-reload_nested_closure_uaf.phpt b/tests/thread_pool/078-reload_nested_closure_uaf.phpt new file mode 100644 index 00000000..8986fc91 --- /dev/null +++ b/tests/thread_pool/078-reload_nested_closure_uaf.phpt @@ -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-- + +--FILE-- +reload(); + delay(80); +} + +var_dump(await($pool->submit(fn () => 42)) === 42); +$pool->close(); + +echo "done\n"; +?> +--EXPECT-- +bool(true) +done diff --git a/tests/thread_pool/079-submit_nested_closure_per_worker.phpt b/tests/thread_pool/079-submit_nested_closure_per_worker.phpt new file mode 100644 index 00000000..9db80246 --- /dev/null +++ b/tests/thread_pool/079-submit_nested_closure_per_worker.phpt @@ -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-- + +--FILE-- + $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 diff --git a/thread.c b/thread.c index 62bb7ffe..aceef37b 100644 --- a/thread.c +++ b/thread.c @@ -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 @@ -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); + } } /** @@ -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