Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
28 changes: 18 additions & 10 deletions src/jsc/CppTask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -47,14 +50,17 @@ 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 VM that created this task. Only safe to dereference on that VM's JS
/// thread; worker VMs are freed by `terminate()`.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn get_vm(&self) -> Option<bun_ptr::BackRef<VirtualMachine>> {
NonNull::new(Bun__EventLoopTaskNoContext__createdInBunVm(self)).map(bun_ptr::BackRef::from)
}

/// The creating context's `ScriptExecutionContextIdentifier`, for the
/// pool-thread checked unref in [`ConcurrentCppTask::run_owned`].
Comment thread
robobun marked this conversation as resolved.
Outdated
pub fn context_identifier(&self) -> u32 {
Bun__EventLoopTaskNoContext__contextIdentifier(self)
}
}

/// A task created from C++ code that runs inside the workpool, usually via ScriptExecutionContext.
Expand All @@ -73,22 +79,24 @@ 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 unref: the creating VM may be a worker freed by terminate()
// while `run` ran; the contexts-map lock serializes with markTerminating().
Comment thread
robobun marked this conversation as resolved.
Outdated
ScriptExecutionContext__unrefEventLoopConcurrently(context_id);
}
}

#[unsafe(no_mangle)]
pub(crate) extern "C" fn ConcurrentCppTask__createAndRun(cpp_task: *mut EventLoopTaskNoContext) {
crate::mark_binding!();
// `EventLoopTaskNoContext` is an `opaque_ffi!` ZST handle; `opaque_ref` is
// the centralised non-null deref proof. C++ just handed it over.
// the centralised non-null deref proof. Runs on the creating VM's JS
// thread (only caller is `PhonyWorkQueue::dispatch`), so dereferencing
// the captured VM here is safe; `run_owned`'s unref is the checked one.
Comment thread
robobun marked this conversation as resolved.
Outdated
if let Some(vm) = EventLoopTaskNoContext::opaque_ref(cpp_task).get_vm() {
vm.event_loop_shared().ref_concurrently();
}
Expand Down
5 changes: 5 additions & 0 deletions src/jsc/bindings/EventLoopTaskNoContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions src/jsc/bindings/EventLoopTaskNoContext.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include "ZigGlobalObject.h"
#include "ScriptExecutionContext.h"
#include "root.h"

namespace Bun {
Expand All @@ -12,6 +13,7 @@ class EventLoopTaskNoContext {
public:
EventLoopTaskNoContext(JSC::JSGlobalObject* globalObject, Function<void()>&& task)
: m_createdInBunVm(defaultGlobalObject(globalObject)->bunVM())
, m_contextIdentifier(defaultGlobalObject(globalObject)->scriptExecutionContext()->identifier())
, m_task(WTF::move(task))
{
}
Expand All @@ -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<void()> 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
12 changes: 12 additions & 0 deletions src/jsc/bindings/ScriptExecutionContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Comment thread
robobun marked this conversation as resolved.
extern "C" void ScriptExecutionContext__unrefEventLoopConcurrently(ScriptExecutionContextIdentifier id)
{
Locker locker { allScriptExecutionContextsMapLock };
auto* context = allScriptExecutionContextsMap().get(id);
if (!context || context->isTerminating())
Comment thread
robobun marked this conversation as resolved.
return;
context->unrefEventLoop();
}

} // namespace WebCore
49 changes: 49 additions & 0 deletions test/js/web/workers/worker-terminate-lifetime.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,55 @@ 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.
for (let i = 0; i < 4; i++) (async () => {
const k = await s.importKey("raw", new TextEncoder().encode("pw-material-key"), "PBKDF2", false, ["deriveBits"]);
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 => w.once("message", res));
// Let the pool lanes fill before terminating.
await Bun.sleep(60 + ((r * 41) % 220));
await w.terminate();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
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-
Expand Down
Loading