diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 5e8875598d3..039a7aaa1a3 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -1595,6 +1595,13 @@ impl VirtualMachine { .unwrap() .close_all_socket_groups(vm_ref); } + // Destroy the per-VM c-ares channel while JSC / `RareData.file_polls` + // / `runtime_state` are all still live — `ares_destroy()` re-enters + // them from its EDESTRUCTION and socket-state callbacks. Mirrors + // `WebWorker::shutdown`. + if let Some(hooks) = runtime_hooks() { + (hooks.close_dns_for_terminate)(); + } // The HTTP daemon thread holds a `Box` per // in-flight request; with the JS thread exiting those never reach @@ -1825,6 +1832,14 @@ pub struct RuntimeHooks { /// `vm` is the live per-thread VM; `runtime_state` must still be installed /// and the JSC heap must not have been swept yet. pub cancel_all_timers: unsafe fn(vm: *mut VirtualMachine), + /// Destroy the per-VM global DNS resolver's c-ares channel now, while JSC, + /// the event loop, `RareData.file_polls`, and `runtime_state` are all + /// live. `ares_destroy()` re-enters the resolver's socket-state and query + /// callbacks; deferring it to `deinit_runtime_state`'s `RuntimeState` drop + /// runs those callbacks against freed state. No-op when the resolver was + /// never lazily created. Called from `WebWorker::shutdown` / `global_exit` + /// right after `close_all_socket_groups`. + pub close_dns_for_terminate: fn(), } /// Canonical `EventLoopCtx` vtable for a `*mut VirtualMachine` owner — the JS diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 62a936a450c..ec51c28cb96 100644 --- a/src/jsc/web_worker.rs +++ b/src/jsc/web_worker.rs @@ -1285,6 +1285,19 @@ impl WebWorker { // is step 3 below). rare.close_all_socket_groups(unsafe { &*vm_ptr }); } + // Destroy the per-VM c-ares channel now: `ares_destroy()` fires + // every pending query callback with `ARES_EDESTRUCTION` and then + // the socket-state callback for each fd it closes, both of which + // dereference state (`JSGlobalObject`, `RareData.file_polls`, + // `runtime_state().timer`) that step 3/5 below free. Deferring it + // to `destroy()`'s `deinit_runtime_state` is a UAF. Must FOLLOW + // `close_all_socket_groups`: its on_close JS can call + // `dns.resolve*()`, and `Resolver::get_channel()` lazily re-inits + // on `channel == None` — running this earlier lets a re-created + // channel survive to `GlobalData::drop` (the original UAF). + if let Some(hooks) = runtime_hooks() { + (hooks.close_dns_for_terminate)(); + } // Stop cross-thread posters first: markTerminating() serializes // with postTaskTo() on the contexts-map lock, so after this call // every task another thread has already enqueued is visible to the diff --git a/src/runtime/dns_jsc/cares_jsc.rs b/src/runtime/dns_jsc/cares_jsc.rs index afcedbe6bf8..d804fcfc3a3 100644 --- a/src/runtime/dns_jsc/cares_jsc.rs +++ b/src/runtime/dns_jsc/cares_jsc.rs @@ -718,6 +718,16 @@ impl ErrorDeferred { } } + let vm = global_this.bun_vm(); + // Worker terminate's `close_dns_for_terminate` fires EDESTRUCTION with + // `is_shutting_down` already set; the task queue is about to be + // drained-without-run and ManagedTask has no cleanup here, so enqueuing + // would leak the `Context` and its `JSPromiseStrong` box. Drop now while + // JSC is still live so the Strong handle releases cleanly. + if vm.is_shutting_down() { + return; + } + let context = bun_core::heap::into_raw(Box::new(Context { deferred: self, global_this: bun_ptr::BackRef::new(global_this), @@ -725,9 +735,7 @@ impl ErrorDeferred { // TODO(@heimskr): new custom Task type // SAFETY: `bun_vm()` returns a non-null VM pointer (VM-owned for the lifetime of // the JSGlobalObject). - global_this - .bun_vm() - .as_mut() + vm.as_mut() .enqueue_task(bun_jsc::ManagedTask::ManagedTask::new( context, Context::callback, diff --git a/src/runtime/dns_jsc/dns.rs b/src/runtime/dns_jsc/dns.rs index 878ea8a2146..9ff56fe99ca 100644 --- a/src/runtime/dns_jsc/dns.rs +++ b/src/runtime/dns_jsc/dns.rs @@ -2138,6 +2138,27 @@ impl Drop for GlobalData { } } +impl Resolver { + /// Worker-terminate / main-VM-destruct hook: tear down the c-ares channel + /// while the JSC VM, `RareData.file_polls`, event loop, and `runtime_state` + /// are all still live. `ares_destroy()` synchronously fires every pending + /// query callback with `ARES_EDESTRUCTION` and then the socket-state + /// callback for each fd it closes; those callback chains dereference + /// `DNSLookup::global_this` (to enqueue the rejection task) and the hive + /// `FilePoll` (to unregister it from the loop). Running this after either + /// is freed is a UAF (Node `test-worker-dns-terminate.js`). + pub fn close_channel_for_terminate(&self) { + if let Some(channel) = self.channel.take() { + // SAFETY: `channel` is the live handle from `ares_init_options`, owned by this resolver. + unsafe { c_ares::Channel::destroy(channel) }; + } + // `GetAddrInfoRequest`'s EDESTRUCTION path does not call + // `request_completed()`, so the c-ares timeout timer (and its +1 ref on + // this resolver plus the uws active-handle bump) can still be linked. + self.remove_timer(); + } +} + // ────────────────────────────────────────────────────────────────────────── // internal — process-wide DNS cache used by usockets connect path // ────────────────────────────────────────────────────────────────────────── @@ -4199,9 +4220,11 @@ impl Resolver { let this = self.as_ctx_ptr(); scopeguard::defer! { // SAFETY: `this` is the heap allocation from `init`. This releases the - // ref taken by `add_timer`; all callers of `request_completed` (the only - // path here) hold an `IntrusiveRc`, so the timer ref is never - // the last and this `deref` cannot reach 0 while `&self` is live. + // ref taken by `add_timer`. Callers via `request_completed` hold an + // `IntrusiveRc`; the `close_channel_for_terminate` caller is + // the global resolver, which carries a permanent +1 pin from + // `global_resolver()`. Either way the timer ref is never the last and + // this `deref` cannot reach 0 while `&self` is live. unsafe { let uws_loop = (*this).vm().uws_loop(); let state = crate::jsc_hooks::runtime_state(); diff --git a/src/runtime/jsc_hooks.rs b/src/runtime/jsc_hooks.rs index dba1935522f..98b88bc1ee6 100644 --- a/src/runtime/jsc_hooks.rs +++ b/src/runtime/jsc_hooks.rs @@ -1486,6 +1486,7 @@ pub(crate) static __BUN_RUNTIME_HOOKS: RuntimeHooks = RuntimeHooks { terminate_all_workers_and_wait, retroactively_report_discovered_tests, cancel_all_timers, + close_dns_for_terminate, }; // ════════════════════════════════════════════════════════════════════════════ @@ -1619,6 +1620,22 @@ unsafe fn cancel_all_timers(vm: *mut VirtualMachine) { } } +/// `RuntimeHooks::close_dns_for_terminate` — destroy the per-VM global DNS +/// resolver's c-ares channel now so its `ARES_EDESTRUCTION` and socket-state +/// callbacks run while the JSC VM, `RareData.file_polls`, and `runtime_state` +/// are all still live. See `Resolver::close_channel_for_terminate`. +fn close_dns_for_terminate() { + let state = runtime_state(); + if state.is_null() { + return; + } + // SAFETY: `state` is the live per-thread `RuntimeState` box; shared borrow + // of the `OnceCell` only (the resolver's own state is interior-mutable). + if let Some(gd) = unsafe { &(*state).global_dns_data }.get() { + gd.resolver.close_channel_for_terminate(); + } +} + pub(crate) fn close_isolation_handles(vm: &mut VirtualMachine) { let state = runtime_state(); if state.is_null() { diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index b938d02fc47..9d476d1f3d5 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -1,5 +1,6 @@ import { expect, test } from "bun:test"; import { bunEnv, bunExe, isASAN, isDebug } from "harness"; +import { join } from "path"; // Worker VM startup/teardown is much slower under debug and/or ASAN; these // tests spawn many workers, so scale iteration counts and timeouts down. @@ -119,3 +120,59 @@ test( }, 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- +// state callbacks, which then dereferenced the freed JSGlobalObject and the +// freed FilePoll hive. ASAN-only: release builds read freed memory without +// crashing. Upstream Node test-worker-dns-terminate.js. +test.skipIf(!isASAN)( + "terminate() while dns.lookup() is in flight does not UAF on c-ares channel teardown", + async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const { Worker } = require("worker_threads"); + let done = 0; + for (let i = 0; i < 4; i++) { + const w = new Worker( + // Hermetic: point the global resolver's c-ares channel at a local + // UDP socket that never replies, so both queries are guaranteed + // in-flight (socket registered, no completion) when terminate() + // lands. dns.lookup() only respects setServers() where the c-ares + // backend is the default (Linux); elsewhere resolve4 alone still + // covers the socket-state and EDESTRUCTION paths hermetically. + 'const dgram = require("dgram");' + + 'const dns = require("dns");' + + 'const s = dgram.createSocket("udp4");' + + 's.bind(0, "127.0.0.1", () => {' + + ' dns.setServers(["127.0.0.1:" + s.address().port]);' + + ' if (process.platform === "linux") dns.lookup("example.org", () => {});' + + ' dns.resolve4("example.org", () => {});' + + ' require("worker_threads").parentPort.postMessage(0);' + + '});', + { eval: true }, + ); + w.on("message", () => w.terminate().then(() => { + if (++done === 4) console.log("ok"); + })); + } + `, + ], + env: { + ...bunEnv, + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"), + LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dirname, "../../../leaksan.supp")}`, + }, + 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, +);