node:fs: snapshot path buffers backed by resizable ArrayBuffers for async ops

Closed
RO
2mo ago
+136-8

Crash

Async node:fs operations that take a buffer-typed path (Uint8Array, DataView, or ArrayBuffer) capture a raw ptr/len into the backing store on the JS thread, then read it later on a work-pool thread. The pin taken at parse time (#31221) blocks transfer()/detach, but ArrayBuffer.prototype.resize never consults the pin count: shrinking a resizable ArrayBuffer decommits its tail pages (JSC::ArrayBuffer::resize -> OSAllocator::protect(start, len, false, false)), so the pool thread's read of the stale snapshot hits no-access pages.

panic: Segmentation fault at address 0x7C00B3000000

Repro (crashes current canary, verified on Linux; the decommit and stale read are platform-independent; several pool threads fault at once):

const fs = require("fs");
const rab = new ArrayBuffer(128 * 1024, { maxByteLength: 128 * 1024 });
const srcPath = Buffer.from("/tmp/does-not-exist");
new Uint8Array(rab).set(srcPath);
const view = new Uint8Array(rab, 0, srcPath.length);

const all = [];
for (let i = 0; i < 512; i++) all.push(fs.promises.rename(view, "/tmp/dest").catch(() => {}));
rab.resize(0); // decommits the pages the queued tasks read the path from
await Promise.all(all);

Found while auditing the async fs path-buffer lifetime for the Windows rename crash fleet (Sentry BUN-2V6E). Shipped 1.3.x builds predate the pin, so they are additionally exposed to plain transfer() detach; on current main the resize hole was the remaining gap. Whether BUN-2V6E itself is this exact mechanism is not proven (its fault address pattern differs), but this is the one memory-unsafe path the audit found, and it is user-reachable.

Fix

Copy the path bytes at parse time when the backing store is a non-shared resizable ArrayBuffer (snapshot_resizable_path_buffer in src/runtime/node/types.rs, applied to both the typed-array and ArrayBuffer arms of PathLike::from_js_with_allocator). Paths are bounded by MAX_PATH_BYTES, and Node also copies buffer paths at call time, so the copy both matches Node semantics and is cheap. Fixed-length buffers stay on the existing zero-copy pinned path, and growable SharedArrayBuffers (which can never shrink and keep a stable data pointer) stay zero-copy as well.

Supporting changes in src/jsc/node_path.rs, since PathLike::Buffer can now own its allocation:

  • PathLike::Drop frees the owned snapshot (MarkedArrayBuffer::destroy, a no-op for JS-owned backings)
  • PathLike::Clone dupes an owned payload instead of borrowing it, mirroring the owned-String arm

Scope: this covers path arguments (PathLike), which all async fs ops parse through the same two arms. Data arguments (StringOrBuffer) have the same bug class and are being fixed separately in #31645; fs.read/fs.write data buffers are zero-copy by API contract and are not changed.

Verification

test/js/node/fs/fs.test.ts gains a test that exercises both parse arms plus call-time snapshot semantics (a rename issued before resize(0) must still act on the original path). It fails on the unfixed build (child segfaults, exit 132) and passes with the fix; the existing pin tests ("keeps a buffer path argument attached") still pass.