diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index e64145537196..8b0e2f3ab4ac 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -273,6 +273,9 @@ pub struct VirtualMachine { pub(crate) macro_event_loop: EventLoop, pub regular_event_loop: EventLoop, pub event_loop: *mut EventLoop, // BORROW_FIELD — points at sibling regular_event_loop/macro_event_loop + /// See [`crate::any_task_job::AnyTaskGate`]. `Option` only for + /// `alloc_zeroed` validity; always `Some` between `init()` and `destroy()`. + pub any_task_gate: Option>, pub(crate) ref_strings: crate::ref_string::Map, pub(crate) ref_strings_mutex: bun_threading::Mutex, @@ -752,6 +755,14 @@ impl VirtualMachine { unsafe { &*self.event_loop } } + #[inline] + pub fn any_task_gate(&self) -> &std::sync::Arc { + debug_assert!(self.any_task_gate.is_some()); + // SAFETY: `Some` between `init()` and `destroy()`; every caller is + // between the two. + unsafe { self.any_task_gate.as_ref().unwrap_unchecked() } + } + /// Alias for [`Self::event_loop_mut`]. Kept for callers migrated on the /// `runtime-hostfn-safe` branch; both names funnel into the single audited /// `unsafe` deref above. @@ -1555,6 +1566,8 @@ impl VirtualMachine { // drain below) or observes the flag under m_lock and drops. // destructOnExit sets it again (idempotently). Bun__JSCTaskScheduler__markShuttingDown(self.global()); + // Same fence for work-pool `AnyTaskJob`s; see `AnyTaskGate`. + self.any_task_gate().close(); // Every worker has now posted its close task to our concurrent // queue (OUTSTANDING is decremented after dispatchExit). Drop @@ -2136,6 +2149,7 @@ impl VirtualMachine { (*regular).virtual_machine = NonNull::new(vm); let _ = (*regular).tasks.ensure_unused_capacity(64); addr_of_mut!((*vm).event_loop).write(regular); + addr_of_mut!((*vm).any_task_gate).write(Some(crate::any_task_job::AnyTaskGate::new())); // `source_mappings.map` is a sibling-field backref onto // `saved_source_map_table`. @@ -4475,6 +4489,8 @@ impl VirtualMachine { // so reclaim it here or every Worker leaks it. drop(core::mem::take(&mut self.preload)); + drop(self.any_task_gate.take()); + // SAFETY: this VM is raw-`dealloc`'d (no field `Drop` runs), so // `transpiler` is never auto-dropped after `deinit` clears its fields. unsafe { self.transpiler.deinit() }; diff --git a/src/jsc/any_task_job.rs b/src/jsc/any_task_job.rs index 870232d0fae8..ab5aa97e6952 100644 --- a/src/jsc/any_task_job.rs +++ b/src/jsc/any_task_job.rs @@ -9,14 +9,54 @@ use core::ffi::c_void; use core::ptr::NonNull; +use std::sync::Arc; use bun_event_loop::AnyTask::AnyTask; use bun_io::KeepAlive; +use bun_threading::RwLock; use bun_threading::work_pool::{IntrusiveWorkTask as _, Task as WorkPoolTask, WorkPool}; use crate::event_loop::ConcurrentTask; use crate::{JSGlobalObject, JsResult, VirtualMachineRef as VirtualMachine}; +/// Close-once fence between a VM and every [`AnyTaskJob`] it schedules. +/// [`AnyTaskJob::run_task`] runs `ctx.run()` and the completion enqueue under +/// a read lock (both touch the JSC heap / VM box; `Heap::lastChanceToFinalize` +/// frees `ArrayBuffer` inputs regardless of `protect()`). Worker shutdown and +/// `global_exit` call [`Self::close`] (write lock, waits out readers) before +/// JSC teardown and the VM free. Same fence-then-drain shape as +/// `ScriptExecutionContext::markTerminating`. +pub struct AnyTaskGate { + closed: RwLock, +} + +impl AnyTaskGate { + pub fn new() -> Arc { + Arc::new(Self { + closed: RwLock::new(false), + }) + } + + /// Mark the owning VM as going away. Takes the write lock, so it returns + /// only after every concurrent [`Self::run_gated`] reader has released. + pub fn close(&self) { + *self.closed.write() = true; + } + + /// Run `body` under a read lock if the gate is open, else skip it. The + /// lock held across `body` orders it entirely before [`Self::close`] + /// returns. Returns `false` when closed (body not run). + fn run_gated(&self, body: impl FnOnce()) -> bool { + let guard = self.closed.read(); + if *guard { + return false; + } + body(); + drop(guard); + true + } +} + /// Per-job payload trait. Implementors own the off-thread work body and the /// JS-thread completion; the surrounding heap/queue/keep-alive plumbing is /// supplied by [`AnyTaskJob`]. @@ -49,6 +89,9 @@ pub trait AnyTaskJobCtx: Sized { /// e.g. a `JSPromiseStrong` field after scheduling. pub struct AnyTaskJob { vm: bun_ptr::BackRef, + /// [`Self::run_task`] only dereferences `vm`/JSC-heap buffers under this + /// gate's read lock. See [`AnyTaskGate`]. + gate: Arc, task: WorkPoolTask, any_task: AnyTask, poll: KeepAlive, @@ -72,9 +115,11 @@ impl AnyTaskJob { /// (running `Drop for C`). The returned pointer is owned by the caller /// until handed to [`Self::schedule`]. pub fn create(global: &JSGlobalObject, ctx: C) -> JsResult<*mut Self> { - let vm = bun_ptr::BackRef::new(global.bun_vm()); + let vm_ref = global.bun_vm(); + let vm = bun_ptr::BackRef::new(vm_ref); let job = bun_core::heap::into_raw(Box::new(Self { vm, + gate: Arc::clone(vm_ref.any_task_gate()), task: WorkPoolTask { node: Default::default(), callback: Self::run_task, @@ -137,14 +182,36 @@ impl AnyTaskJob { fn run_task(task: *mut WorkPoolTask) { // SAFETY: only reachable via the `WorkPoolTask::callback` slot wired // in `create`; `task` points to `Self.task` and the job is live until - // `run_from_js` reclaims it. - let job = unsafe { &mut *Self::from_task_ptr(task) }; - let vm = job.vm; - job.ctx.run(vm.global); - // `ConcurrentTask::create` heap-allocates a fresh task; the queue takes - // ownership of it. - vm.event_loop_shared() - .enqueue_task_concurrent(ConcurrentTask::create(job.any_task.task())); + // `run_from_js` reclaims it (or is leaked below). + let this = unsafe { Self::from_task_ptr(task) }; + // SAFETY: `gate` was written in `create`; this thread exclusively owns + // `*this` until the enqueue below hands it to the JS thread. Cloned so + // the read lock can be held while `this` is reborrowed `&mut` inside. + let gate = Arc::clone(unsafe { &(*this).gate }); + let ran = gate.run_gated(|| { + // SAFETY: gate open ⇒ worker shutdown hasn't passed `close()` ⇒ + // the owning VM, its JSC heap (which `ctx.run` may read/write via + // `ArrayBuffer`-backed inputs/outputs), and its embedded event + // loop are all still live. + let job = unsafe { &mut *this }; + let vm = job.vm; + job.ctx.run(vm.global); + // `ConcurrentTask::create` heap-allocates; the queue takes + // ownership. + vm.event_loop_shared() + .enqueue_task_concurrent(ConcurrentTask::create(job.any_task.task())); + }); + drop(gate); + if ran { + return; + } + // Gate closed (worker terminated before this task was picked up). + // SAFETY: sole owner of `*this` (`run_gated` did not touch it); `gate` + // is never read again (the box is leaked below). + unsafe { core::ptr::drop_in_place(core::ptr::addr_of_mut!((*this).gate)) }; + // `poll`/`ctx` are intentionally leaked: both would touch the freed + // VM/JSC heap on Drop. Same fate as tasks stranded on a terminated + // worker's never-drained concurrent queue. } /// `AnyTask` callback — runs ON the JS thread. Reclaims the heap diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 6e67c931d47c..a8b8b9e034b0 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1302,6 +1302,8 @@ impl WebWorker { // or observes m_isShuttingDown under m_lock and drops. Idempotent; // teardownJSCVM sets it again. Bun__JSCTaskScheduler__markShuttingDown(vm.global()); + // Same fence for work-pool `AnyTaskJob`s; see `AnyTaskGate`. + vm.any_task_gate().close(); // Reclaim queued CppTasks (the per-worker stdio/messaging // MessagePort drain tasks that can be in self.tasks mid-tick when // terminate() lands, and any Worker dispatchExit close task from a diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 6b9f7c1b3366..34f855f272c9 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -455,3 +455,64 @@ test.skipIf(!isDebug)( }, 120_000, ); + +// Regression: worker.terminate() while an async crypto.pbkdf2()/scrypt() was +// running on the shared work pool freed the worker VM (and its JSC heap) out +// from under the pool thread. AnyTaskJob::run_task both (a) handed the +// caller's ArrayBuffer-backed password/salt to BoringSSL (freed by +// Heap::lastChanceToFinalize) and (b) dereferenced the VM box to post the +// completion (freed by the worker's dealloc). Release builds segfaulted the +// whole process; ASAN reported the UAF on a "Bun Pool" thread inside +// PKCS5_PBKDF2_HMAC / EVP_PBE_scrypt or VirtualMachine::event_loop_shared. +test( + "terminate() while async crypto.pbkdf2()/scrypt() with Buffer inputs is running on the work pool does not UAF", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const src = + 'const { parentPort } = require("node:worker_threads");' + + 'const crypto = require("node:crypto");' + + // Salt length NOT a multiple of 64: SHA-256's block function is + // uninstrumented assembly, only the <64-byte tail goes through + // memcpy where ASAN can observe the freed read. + 'const salt = Buffer.alloc((4 << 20) - 17, 0xaa);' + + 'const pw = salt.subarray(0, 1 << 20);' + + // Heavy enough params that each job is still inside BoringSSL on a + // pool thread when terminate() lands (so the worker event loop never + // reaches the JS-thread completion), but bounded so the fix's + // close() wait stays well under the test timeout. + 'for (let k = 0; k < 2; k++) crypto.pbkdf2(pw, salt, 200000, 64, "sha256", () => {});' + + 'for (let k = 0; k < 2; k++) crypto.scrypt(salt, pw, 32, { N: 1 << 14, r: 8, p: 1, maxmem: 128 << 20 }, () => {});' + + 'parentPort.postMessage("up");'; + for (let r = 0; r < ${rounds}; r++) { + const w = new Worker(src, { eval: true }); + await new Promise((res, rej) => { + w.once("message", res); + w.once("error", rej); + w.once("exit", (c) => rej(new Error("worker exited " + c + " before ready"))); + }); + w.on("error", () => {}); + await w.terminate(); + } + console.log("ok"); + `, + ], + // A job still on the pool when the gate closes is leaked by design (its + // ctx holds Strong/JSPromiseStrong handles into the dead VM's JSC heap, + // and poll.unref would touch the freed event loop). Bounded per + // terminated worker; opt this subprocess out of LSan so that stranded + // accounting is not asserted as a regression. + env: { ...bunEnv, ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=0"].filter(Boolean).join(":") }, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "ok\n", stderr: "", exitCode: 0 }); + }, + timeout, +);