diff --git a/src/jsc/CppTask.rs b/src/jsc/CppTask.rs index 130402469c25..7ffebd175119 100644 --- a/src/jsc/CppTask.rs +++ b/src/jsc/CppTask.rs @@ -10,6 +10,9 @@ unsafe extern "C" { safe fn Bun__EventLoopTaskNoContext__createdInBunVm( task: &EventLoopTaskNoContext, ) -> *mut VirtualMachine; + safe fn Bun__EventLoopTaskNoContext__contextIdentifier(task: &EventLoopTaskNoContext) -> u32; + // safe: u32 in; resolves under the contexts-map lock, no-op if gone/terminating. + safe fn ScriptExecutionContext__unrefEventLoopConcurrently(id: u32); } bun_opaque::opaque_ffi! { @@ -47,14 +50,15 @@ impl EventLoopTaskNoContext { unsafe { Bun__EventLoopTaskNoContext__performTask(this) } } - /// Get the VM that created this task. `VirtualMachine` is process-lifetime - /// (PORTING.md §Global mutable state), so a [`BackRef`] is the right - /// non-owning handle: callers project `&VirtualMachine` via `Deref` and - /// route mutation through the VM's safe interior accessors (e.g. - /// `event_loop_shared()`). + /// The creating VM. Only safe to dereference on that VM's JS thread. pub fn get_vm(&self) -> Option> { NonNull::new(Bun__EventLoopTaskNoContext__createdInBunVm(self)).map(bun_ptr::BackRef::from) } + + /// The creating context's identifier, for the checked pool-thread unref. + pub fn context_identifier(&self) -> u32 { + Bun__EventLoopTaskNoContext__contextIdentifier(self) + } } /// A task created from C++ code that runs inside the workpool, usually via ScriptExecutionContext. @@ -73,14 +77,13 @@ 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_identifier(); 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(); - } + // Checked: the creating VM may be a worker freed by terminate() while `run` ran. + ScriptExecutionContext__unrefEventLoopConcurrently(context_id); } } @@ -90,6 +93,7 @@ pub(crate) extern "C" fn ConcurrentCppTask__createAndRun(cpp_task: *mut EventLoo // `EventLoopTaskNoContext` is an `opaque_ffi!` ZST handle; `opaque_ref` is // the centralised non-null deref proof. C++ just handed it over. if let Some(vm) = EventLoopTaskNoContext::opaque_ref(cpp_task).get_vm() { + // Runs on the creating VM's JS thread (from `PhonyWorkQueue::dispatch`). vm.event_loop_shared().ref_concurrently(); } WorkPool::schedule_new(ConcurrentCppTask { diff --git a/src/jsc/bindings/EventLoopTaskNoContext.cpp b/src/jsc/bindings/EventLoopTaskNoContext.cpp index 6f1f3d367ad9..003bc1bcc719 100644 --- a/src/jsc/bindings/EventLoopTaskNoContext.cpp +++ b/src/jsc/bindings/EventLoopTaskNoContext.cpp @@ -12,4 +12,9 @@ extern "C" void* Bun__EventLoopTaskNoContext__createdInBunVm(const EventLoopTask return task->createdInBunVm(); } +extern "C" WebCore::ScriptExecutionContextIdentifier Bun__EventLoopTaskNoContext__contextIdentifier(const EventLoopTaskNoContext* task) +{ + return task->contextIdentifier(); +} + } // namespace Bun diff --git a/src/jsc/bindings/EventLoopTaskNoContext.h b/src/jsc/bindings/EventLoopTaskNoContext.h index fede33f2603c..b372cc07fc7a 100644 --- a/src/jsc/bindings/EventLoopTaskNoContext.h +++ b/src/jsc/bindings/EventLoopTaskNoContext.h @@ -1,6 +1,7 @@ #pragma once #include "ZigGlobalObject.h" +#include "ScriptExecutionContext.h" #include "root.h" namespace Bun { @@ -12,6 +13,7 @@ class EventLoopTaskNoContext { public: EventLoopTaskNoContext(JSC::JSGlobalObject* globalObject, Function&& task) : m_createdInBunVm(defaultGlobalObject(globalObject)->bunVM()) + , m_contextIdentifier(defaultGlobalObject(globalObject)->scriptExecutionContext()->identifier()) , m_task(WTF::move(task)) { } @@ -23,13 +25,17 @@ class EventLoopTaskNoContext { } void* createdInBunVm() const { return m_createdInBunVm; } + WebCore::ScriptExecutionContextIdentifier contextIdentifier() const { return m_contextIdentifier; } private: void* m_createdInBunVm; + // For ConcurrentCppTask's checked pool-thread unref. + WebCore::ScriptExecutionContextIdentifier m_contextIdentifier; Function m_task; }; extern "C" void Bun__EventLoopTaskNoContext__performTask(EventLoopTaskNoContext* task); extern "C" void* Bun__EventLoopTaskNoContext__createdInBunVm(const EventLoopTaskNoContext* task); +extern "C" WebCore::ScriptExecutionContextIdentifier Bun__EventLoopTaskNoContext__contextIdentifier(const EventLoopTaskNoContext* task); } // namespace Bun diff --git a/src/jsc/bindings/ScriptExecutionContext.cpp b/src/jsc/bindings/ScriptExecutionContext.cpp index 60f4ca58b9ef..005c1cbb2609 100644 --- a/src/jsc/bindings/ScriptExecutionContext.cpp +++ b/src/jsc/bindings/ScriptExecutionContext.cpp @@ -308,4 +308,16 @@ extern "C" void ScriptExecutionContext__markTerminating(JSC::JSGlobalObject* glo context->markTerminating(); } +// Checked unref for ConcurrentCppTask's pool-thread completion: the map lock +// serializes with markTerminating() (called before the worker VM is freed), so +// a terminated worker's VM is never dereferenced. Same fence as postTaskTo(). +extern "C" void ScriptExecutionContext__unrefEventLoopConcurrently(ScriptExecutionContextIdentifier id) +{ + Locker locker { allScriptExecutionContextsMapLock }; + auto* context = allScriptExecutionContextsMap().get(id); + if (!context || context->isTerminating()) + return; + context->unrefEventLoop(); +} + } // namespace WebCore diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 9d476d1f3d53..d7eccaeb0511 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -121,6 +121,61 @@ test( timeout, ); +// Regression: ConcurrentCppTask::run_owned (the work-pool wrapper for async +// WebCrypto ops) dereferenced the raw bunVM pointer captured by the C++ +// EventLoopTaskNoContext to call unref_concurrently() after the crypto body +// ran. When the creating VM was a worker freed by terminate() while the +// crypto op was still running on the pool, that read the freed VM +// allocation. The body itself already posts back via postTaskTo(contextId) +// and so was safe; only the trailing unref was unfenced. +test.skipIf(!isASAN)( + "terminate() while crypto.subtle async ops are in flight does not UAF in ConcurrentCppTask", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("node:worker_threads"); + const src = \` + const { parentPort } = require("node:worker_threads"); + const s = crypto.subtle; + // Four lanes of PBKDF2 deriveBits on the work pool. The 200k-iteration + // SHA-512 body is long enough that terminate() reliably lands while + // at least one ConcurrentCppTask is still in run_owned. + const k = await s.importKey("raw", new TextEncoder().encode("pw-material-key"), "PBKDF2", false, ["deriveBits"]); + for (let i = 0; i < 4; i++) (async () => { + for (;;) try { await s.deriveBits({ name: "PBKDF2", salt: new Uint8Array(16), iterations: 200000, hash: "SHA-512" }, k, 512); } catch {} + })(); + 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", code => rej(new Error("worker exited early: " + code))); + }); + // The deriveBits tasks are on the work-pool queue as of "up"; give the + // pool threads a moment to enter the PBKDF2 body so terminate() frees + // the VM mid-op (the condition the fence must handle). + await Bun.sleep(60 + ((r * 41) % 220)); + await w.terminate(); + } + console.log("ok"); + `, + ], + env: bunEnv, + 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, +); + // Regression: the per-VM c-ares channel was destroyed in deinit_runtime_state // (RuntimeState drop) AFTER JSC teardown and RareData.file_polls drop. // ares_destroy() synchronously fires EDESTRUCTION query callbacks and socket-