worker: post every cross-thread completion by ScriptExecutionContext id so terminate() cannot UAF the freed VM

docs/ROOT-B-SHUTDOWN-FENCE.md

@@ -0,0 +1,141 @@
+# Root B: worker teardown vs. cross-thread completions
+
+## The failure class
+
+`WebWorker::shutdown` (`src/jsc/web_worker.rs:1216`) tears down a worker's
+`VirtualMachine` in five ordered steps. Step 2 sets `is_shutting_down`, drains
+timers, force-closes sockets and the c-ares channel, calls
+`ScriptExecutionContext::markTerminating()` (so C++ `postTaskTo` posters are
+fenced), and drains the concurrent queue via
+`release_queued_tasks_for_shutdown()`. Step 5 `std::alloc::dealloc`'s the raw
+`VirtualMachine` box (≈`web_worker.rs:1383`) and frees the uws loop.
+
+Nothing in that sequence cancels or awaits work already handed to a
+process-global thread (WorkPool, the HTTP thread, the bundle thread). Those
+jobs complete later and post back through a pointer captured at schedule time:
+a `BackRef<EventLoop>`, a `&'static VirtualMachine`, or a `*const
+JSGlobalObject`. Every one of those pointers is into the freed VM box or the
+freed JSC heap.
+
+All reproduced cross-thread UAFs in this class converge on
+**`EventLoop::enqueue_task_concurrent`** (`src/jsc/event_loop.rs:997`). Putting
+a check _inside_ the funnel does not help: `&self` there is already a pointer
+into the freed box. Several callers also "guard" with an off-thread
+`vm.is_shutting_down()` read; that read is itself a UAF face (the flag lives in
+the freed box).
+
+## Shutdown map (reference)
+
+| step | action                                                         | fences                        |
+| ---- | -------------------------------------------------------------- | ----------------------------- |
+| 1    | `self.vm = null` under `vm_lock`                               | parent-thread readers         |
+| 2a   | `is_shutting_down = true`, `on_exit`, drain timers/sockets/DNS | on-thread re-entry            |
+| 2b   | `ScriptExecutionContext::markTerminating()`                    | C++ `postTaskTo` posters      |
+| 2c   | `Bun__JSCTaskScheduler__markShuttingDown()`                    | `Atomics.notify` posters      |
+| 2d   | `release_queued_tasks_for_shutdown()`                          | tasks already queued          |
+| 3    | `WebWorker__teardownJSCVM`                                     | GC finalizers, JSC heap freed |
+| 4    | `WebWorker__dispatchExit`                                      | parent releases its ref       |
+| 5    | `vm.destroy()`; `dealloc(vm_ptr)`; free uws loop               | VM box freed                  |
+
+Rust-side cross-thread posters were not serialized with any of 2b/2c/2d.
+
+## The fence: enqueue by identifier
+
+Off-thread jobs now carry the worker's `ScriptExecutionContextIdentifier` (a
+`u32`) instead of a `BackRef<EventLoop>` / `&VirtualMachine`, and post through
+
+```rust
+ScriptExecutionContextIdentifier::post_concurrent_task(id, task) -> bool
+```
+
+backed by the same locked-registry + `isTerminating()` gate that
+`ScriptExecutionContext::postTaskTo` already uses:
+
+```cpp
+extern "C" bool ScriptExecutionContext__postConcurrentTask(Identifier id, void* task) {
+    Locker locker { allScriptExecutionContextsMapLock };
+    auto* ctx = allScriptExecutionContextsMap().get(id);
+    if (!ctx || ctx->isTerminating()) return false;
+    Bun__EventLoop__enqueueConcurrentTask(ctx->globalObject(), task);
+    return true;
+}
+```
+
+`markTerminating()` (shutdown step 2b) takes the same lock to set the flag, so
+every poster serializes into exactly one of two cases:
+
+1. The poster's whole critical section ran before `markTerminating()`: the task
+   is in the concurrent queue, and step 2d's drain observes and reclaims it.
+2. `markTerminating()` ran first: the poster sees `isTerminating()` and returns
+   `false` without touching the VM. The caller owns the task and runs its
+   abandon path.
+
+A `u32` identifier cannot dangle. The funnel's `bool` return replaces every
+stale off-thread `is_shutting_down()` read.
+
+Companion helpers on the same lock:
+
+- `ScriptExecutionContextIdentifier::is_alive()` — "should I even start?" check
+  for work bodies that write into JSC-heap buffers (e.g. `Scrypt`'s output
+  `ArrayBuffer`). A best-effort fast drop; the authoritative gate is
+  `post_concurrent_task`.
+- `ScriptExecutionContextIdentifier::unref_event_loop_concurrently()` — the
+  `concurrent_ref` decrement that `ConcurrentCppTask` (WebCrypto) needs after
+  its body ran on the pool thread, without dereferencing the VM.
+
+## Abandon path
+
+On `post_concurrent_task` → `false` the target VM and its JSC heap are gone (or
+about to be). The abandon path:
+
+- **must not** touch `Strong`/`Weak`/`JSPromiseStrong`/`JSGlobalObject`/
+  `VirtualMachine`/`KeepAlive::unref` — the HandleSet is freed, the loop is
+  freed;
+- **may** free any pure-Rust heap it owns (body buffers, `Vec`s, `Box<[u8]>`);
+- **must** free a freshly heap-allocated `ConcurrentTask` node (ownership was
+  not transferred);
+- **may** leak the job box when it holds JSC handles. Bounded: one per
+  terminated worker per in-flight op.
+
+## Coverage
+
+Three generic helpers carry most of the surface:
+
+| helper                     | users                                                       |
+| -------------------------- | ----------------------------------------------------------- |
+| `WorkTask<C>`              | `ReadFile`, `WriteFile`, `GetAddrInfoRequest`               |
+| `ConcurrentPromiseTask<C>` | `CopyFile`, `TransformTask`, `WalkTask`, `PipelineTask`     |
+| `AnyTaskJob<C>`            | `Pbkdf2Ctx`, `CryptoJob<Scrypt/…>`, `ZstdCtx`, `SecretsCtx` |
+| `ConcurrentCppTask`        | WebCrypto (`PhonyWorkQueue::dispatch`)                      |
+
+Direct callers converted alongside: `FetchTasklet`, `PasswordJob`,
+`CompressionStream` (zlib/brotli/zstd), `AsyncFSTask` / `NewAsyncCpTask` /
+`AsyncReaddirRecursiveTask`, `S3HttpSimpleTask` / `S3HttpDownloadStreamingTask`,
+`Archive::AsyncTask`, `JSBundleCompletionTask`, `TranspilerJob`.
+
+Explicitly **not** enqueue-shaped and left for their own fixes: nested-worker
+child-init reading a freed parent VM, `node:quic` finalizer ordering,
+`RedisClient::finalize` free-then-read, `Bun.SQL` handle crashes during
+terminating-VM JS execution, the `serve.listen`/JS-re-entry assert zone.
+
+## Reserve alternative (not taken)
+
+Refcount-deferred VM dealloc + a closed-flag at the funnel: each off-thread job
+takes an `Arc` clone of a per-VM gate and brackets its enqueue with a read
+lock; `shutdown` takes the write lock before dealloc. Same sweep, but:
+
+- `shutdown` then _waits_ on in-flight pool jobs (a slow argon2/RSA can stall
+  terminate for seconds);
+- more atomics on the hot enqueue path;
+- the gate must be `Arc`'d so it outlives the VM box anyway.
+
+The identifier route reuses an existing lock, adds no wait, and matches what
+the C++ side already does.
+
+## Post-fix gate
+
+`repro/rootB-verify/verify.mjs`: one worker per iteration arms one in-flight op
+of every cross-thread source above (self-contained, loopback-only, public API),
+the parent `terminate()`s mid-flight, ×100. PASS = rc 0, `ROOT-B VERIFY: PASS`,
+zero ASan/assert/panic. Baseline on current canary: SIGSEGV on teardown 1
+(release), heap-use-after-free on teardown 1-3 (debug+ASAN).

repro/rootB-verify/verify.mjs

@@ -0,0 +1,39 @@
+// Root-B post-fix gate: one worker per iteration arms one in-flight op of
+// every cross-thread completion source (public API, loopback-only), parent
+// terminates mid-flight, x ITERATIONS. PASS = rc 0 + the PASS line + zero
+// ASan/assert/panic. On stock canary this SIGSEGVs on the first teardown.
+//
+// Run under the debug+ASAN build so the fail-before is deterministic:
+//   bun bd repro/rootB-verify/verify.mjs
+
+import { Worker } from "node:worker_threads";
+
+const ITERATIONS = Number(process.env.ROOTB_ITER ?? 100);
+const body = new URL("./worker-body.mjs", import.meta.url);
+
+for (let i = 0; i < ITERATIONS; i++) {
+  const w = new Worker(body);
+  // Wait for the worker to finish arming (or give up after a bound).
+  const armed = await new Promise((resolve) => {
+    const t = setTimeout(() => resolve("timeout"), 5000);
+    w.once("message", (m) => {
+      clearTimeout(t);
+      resolve(m);
+    });
+    w.once("error", (e) => {
+      clearTimeout(t);
+      resolve(e);
+    });
+  });
+  if (armed instanceof Error) {
+    console.error(`iter ${i}: worker error before terminate:`, armed);
+    process.exit(1);
+  }
+  // Small jitter so terminate lands at varying points in the in-flight work.
+  const jitter = (i * 2654435761 >>> 0) % 5;
+  if (jitter) await new Promise((r) => setTimeout(r, jitter));
+  await w.terminate();
+  if (i % 10 === 9) console.log(`… ${i + 1}/${ITERATIONS} teardowns clean`);
+}
+
+console.log(`ROOT-B VERIFY: PASS (${ITERATIONS} teardowns)`);

repro/rootB-verify/worker-body.mjs

@@ -0,0 +1,104 @@
+// One worker instance: arm one in-flight op of every cross-thread completion
+// source, then sit. Parent terminates us mid-flight. Every op is self-contained
+// and loopback-only (no public internet). Catch-and-ignore everywhere: the
+// point is to have work airborne on a process-global thread when the VM dies,
+// not to observe the result.
+
+import { parentPort } from "node:worker_threads";
+import fs from "node:fs";
+import fsp from "node:fs/promises";
+import zlib from "node:zlib";
+import crypto from "node:crypto";
+import dns from "node:dns/promises";
+import os from "node:os";
+import path from "node:path";
+
+const sink = () => {};
+const swallow = (p) => Promise.resolve(p).then(sink, sink);
+
+const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "rootB-"));
+const tmpFile = path.join(tmp, "a.txt");
+fs.writeFileSync(tmpFile, "x".repeat(1 << 16));
+
+// fetch / HTMLRewriter / TLS (HTTP thread -> FetchTasklet)
+{
+  const srv = Bun.serve({
+    port: 0,
+    fetch: () => new Response("x".repeat(1 << 14)),
+  });
+  swallow(fetch(`http://127.0.0.1:${srv.port}/`).then((r) => r.text()));
+  swallow(
+    fetch(`http://127.0.0.1:${srv.port}/`).then((r) =>
+      new HTMLRewriter().on("*", { text() {} }).transform(r).text(),
+    ),
+  );
+}
+
+// Bun.file / Bun.write (WorkPool -> WorkTask<WriteFile>/<ReadFile>)
+swallow(Bun.write(path.join(tmp, "b.txt"), "y".repeat(1 << 16)));
+swallow(Bun.file(tmpFile).text());
+
+// node:fs promises (WorkPool -> AsyncFSTask)
+swallow(fsp.readFile(tmpFile));
+swallow(fsp.stat(tmpFile));
+swallow(fsp.readdir(tmp, { recursive: true }));
+
+// pbkdf2 / scrypt / generateKeyPair (WorkPool -> AnyTaskJob)
+crypto.pbkdf2("p", "s", 100000, 64, "sha512", sink);
+crypto.scrypt("p", "saltsalt", 64, sink);
+crypto.generateKeyPair("rsa", { modulusLength: 2048 }, sink);
+
+// Bun.password (WorkPool -> PasswordJob)
+swallow(Bun.password.hash("hunter2", { algorithm: "bcrypt", cost: 8 }));
+
+// zlib (WorkPool -> CompressionStream async_job_run)
+zlib.deflate(Buffer.alloc(1 << 18), sink);
+zlib.gzip(Buffer.alloc(1 << 18), sink);
+
+// S3 (HTTP thread -> S3HttpSimpleTask); loopback endpoint that never answers.
+{
+  const srv = Bun.serve({ port: 0, fetch: () => new Promise(sink) });
+  const s3 = new Bun.S3Client({
+    accessKeyId: "x",
+    secretAccessKey: "y",
+    endpoint: `http://127.0.0.1:${srv.port}`,
+    bucket: "b",
+  });
+  swallow(s3.file("k").text());
+}
+
+// Bun.build (BundleThread -> JSBundleCompletionTask)
+{
+  const entry = path.join(tmp, "entry.ts");
+  fs.writeFileSync(entry, `export const x: number = 1;\n`);
+  swallow(Bun.build({ entrypoints: [entry], target: "bun" }));
+}
+
+// Transpiler.transform (WorkPool -> ConcurrentPromiseTask<TransformTask>)
+swallow(new Bun.Transpiler({ loader: "tsx" }).transform("const x: number = 1;"));
+
+// dns.lookup (c-ares; clean-by-construction via close_dns_for_terminate)
+swallow(dns.lookup("localhost"));
+
+// WebCrypto (WorkPool -> ConcurrentCppTask)
+swallow(
+  crypto.subtle.digest("SHA-256", new Uint8Array(1 << 16)),
+);
+swallow(
+  crypto.subtle.generateKey({ name: "AES-GCM", length: 256 }, true, [
+    "encrypt",
+    "decrypt",
+  ]),
+);
+
+// Glob (WorkPool -> ConcurrentPromiseTask<WalkTask>)
+swallow(Array.fromAsync(new Bun.Glob("**/*").scan(tmp)));
+
+// zstd (WorkPool -> AnyTaskJob<ZstdCtx>)
+swallow(Bun.zstdCompress(Buffer.alloc(1 << 16)));
+
+// Signal parent that everything is airborne.
+parentPort.postMessage("armed");
+
+// Keep the loop alive so terminate() lands mid-flight.
+setInterval(sink, 1 << 30);

src/jsc/ConcurrentPromiseTask.rs

@@ -3,6 +3,7 @@ use bun_io::{self as Async, KeepAlive};
 use bun_threading::{IntrusiveWorkTask as _, WorkPoolTask, work_pool::WorkPool};
 
 use crate::event_loop::EventLoop;
+use crate::js_global_object::ScriptExecutionContextIdentifier;
 use crate::js_promise::{JSPromise, Strong as JSPromiseStrong};
 use crate::virtual_machine::VirtualMachine;
 use crate::{JSGlobalObject, JsTerminated};
@@ -31,9 +32,15 @@ pub struct ConcurrentPromiseTask<'a, Context: ConcurrentPromiseTaskContext> {
     // Owned here so dropping the task frees the context.
     pub ctx: Box<Context>,
     pub task: WorkPoolTask,
-    /// BACKREF — captured from the JS-thread VM at create time; the VM (and its
-    /// `EventLoop`) outlives every task scheduled on it.
+    /// BACKREF — captured from the JS-thread VM at create time. Only
+    /// dereferenced on the JS thread; the pool-thread completion goes through
+    /// [`Self::context_id`] so a worker VM freed by `terminate()` is never
+    /// touched.
     pub event_loop: BackRef<EventLoop>,
+    /// Stable `u32` id for the originating `ScriptExecutionContext`. The
+    /// pool-thread `on_finish` posts the completion via this id under the C++
+    /// contexts-map lock (serializing with worker `markTerminating()`).
+    pub context_id: ScriptExecutionContextIdentifier,
     pub promise: JSPromiseStrong,
     pub global_this: &'a JSGlobalObject,
     pub concurrent_task: ConcurrentTask,
@@ -62,6 +69,7 @@ impl<'a, Context: ConcurrentPromiseTaskContext> ConcurrentPromiseTask<'a, Contex
         let event_loop = BackRef::new(VirtualMachine::get().as_mut().event_loop_shared());
         let mut this = Box::new(Self {
             event_loop,
+            context_id: global_this.script_execution_context_identifier(),
             ctx: value,
             task: WorkPoolTask {
                 node: Default::default(),
@@ -108,15 +116,19 @@ impl<'a, Context: ConcurrentPromiseTaskContext> ConcurrentPromiseTask<'a, Contex
         // `this` while holding `&mut *this` is sound because `from` only stores
         // the pointer (does not dereference it).
         let this_ref = unsafe { &mut *this };
-        let event_loop = this_ref.event_loop;
+        let context_id = this_ref.context_id;
         let task = core::ptr::NonNull::from(
             this_ref
                 .concurrent_task
                 .from(this, AutoDeinit::ManualDeinit),
         );
-        // `task` is the live `concurrent_task` field of the heap-allocated
-        // job; the queue takes ownership of its intrusive `next` link.
-        event_loop.enqueue_task_concurrent(task);
+        // Post by stable context id: the enqueue dereferences the target VM
+        // only under the contexts-map lock (serializes with worker
+        // `markTerminating()`). When the context is gone the box is leaked
+        // (its `JSPromiseStrong` and `global_this` point into the dead JSC
+        // heap and cannot be released off-thread); `task` is a field of
+        // `*this`, so no separate free.
+        let _ = context_id.post_concurrent_task(task);
     }
 
     /// Frees the heap allocation backing this task.

src/jsc/CppTask.rs

@@ -1,5 +1,6 @@
 use core::ptr::NonNull;
 
+use crate::js_global_object::ScriptExecutionContextIdentifier;
 use crate::{JSGlobalObject, JsResult, VirtualMachineRef as VirtualMachine};
 use bun_event_loop::{TaskTag, Taskable, task_tag};
 use bun_threading::work_pool::{Task as WorkPoolTask, WorkPool};
@@ -10,6 +11,7 @@ unsafe extern "C" {
     safe fn Bun__EventLoopTaskNoContext__createdInBunVm(
         task: &EventLoopTaskNoContext,
     ) -> *mut VirtualMachine;
+    safe fn Bun__EventLoopTaskNoContext__contextId(task: &EventLoopTaskNoContext) -> u32;
 }
 
 bun_opaque::opaque_ffi! {
@@ -55,6 +57,14 @@ impl EventLoopTaskNoContext {
     pub fn get_vm(&self) -> Option<bun_ptr::BackRef<VirtualMachine>> {
         NonNull::new(Bun__EventLoopTaskNoContext__createdInBunVm(self)).map(bun_ptr::BackRef::from)
     }
+
+    /// Stable `u32` id of the `ScriptExecutionContext` that created this task,
+    /// captured on the JS thread at construction. Safe to use off-thread via
+    /// [`ScriptExecutionContextIdentifier::unref_event_loop_concurrently`] so
+    /// the VM is dereferenced only under the contexts-map lock.
+    pub fn context_id(&self) -> ScriptExecutionContextIdentifier {
+        ScriptExecutionContextIdentifier(Bun__EventLoopTaskNoContext__contextId(self))
+    }
 }
 
 /// A task created from C++ code that runs inside the workpool, usually via ScriptExecutionContext.
@@ -73,14 +83,16 @@ impl ConcurrentCppTask {
         let cpp_task = self.cpp_task;
         // `EventLoopTaskNoContext` is an `opaque_ffi!` ZST handle; `opaque_ref`
         // is the centralised non-null deref proof. Valid until `run` consumes it.
-        let maybe_vm = EventLoopTaskNoContext::opaque_ref(cpp_task).get_vm();
+        let context_id = EventLoopTaskNoContext::opaque_ref(cpp_task).context_id();
         drop(self);
         // SAFETY: `cpp_task` is the valid C++ handle stored by `ConcurrentCppTask__createAndRun`;
         // `opaque_ref` above proved it non-null and it has not yet been freed — `run` consumes it here.
         unsafe { EventLoopTaskNoContext::run(cpp_task) };
-        if let Some(vm) = maybe_vm {
-            vm.event_loop_shared().unref_concurrently();
-        }
+        // The task body (e.g. a WebCrypto crypto-thread lambda) posts its own
+        // result back via `ScriptExecutionContext::postTaskTo`, which already
+        // serializes with `markTerminating()`. The trailing `concurrent_ref`
+        // decrement dereferences the VM, so route it through the same lock.
+        context_id.unref_event_loop_concurrently();
     }
 }