node:fs: snapshot resizable-ArrayBuffer-backed path buffers so an option getter cannot resize(0) through the borrow

Closed
RO
2mo ago
+153-8

Repro

const fs = require("node:fs");
const pb = Buffer.from("/tmp/pathlike-resize-test.txt");
const p = new Uint8Array(new ArrayBuffer(pb.length, { maxByteLength: 1 << 16 }));
p.set(pb);
fs.writeFileSync(p, "data", { get flag() { p.buffer.resize(0); return "w"; } });

panic(main thread): Segmentation fault at address 0x728022000000

Every node:fs op that takes a buffer-typed path through PathLike::from_js_with_allocator and then evaluates a later argument is affected, on both paths:

  • sync: writeFileSync/appendFileSync (flag/mode/signal/flush), readFileSync (encoding/flag), mkdirSync (recursive/mode), readdirSync, rmSync, openSync, statSync (throwIfNoEntry), ...
  • async: any fs.promises.*/callback op, where JS can shrink the buffer between the call returning and the work-pool thread reading the path bytes.
  • Bun.file(Uint8Array) + .text()/.arrayBuffer(), which store the PathLike and read it later on a worker.

Cause

PathLike::from_js_with_allocator pins the backing JSC::ArrayBuffer via Buffer::from_js_pinned so transfer()/structuredClone/postMessage cannot detach it (#31221). The pin does not guard ArrayBuffer.prototype.resize: JSC::ArrayBuffer::resize answers a shrink by OSAllocator::protecting the trimmed pages PROT_NONE. The captured (ptr, len) still spans those pages, so the next slice_z → copy_from_slice (sync) or the work-pool thread's read (async) faults.

Node does not crash: it copies buffer paths into an internal string before the syscall.

Fix

When the path buffer is backed by a resizable non-shared ArrayBuffer, snapshot the bytes into an owned MarkedArrayBuffer at capture time (snapshot_resizable_path_buffer in src/runtime/node/types.rs, applied to both the Uint8Array/DataView and ArrayBuffer arms). Paths are already validated to fit in MAX_PATH_BYTES, so the copy is cheap. Fixed-length buffers stay on the zero-copy pinned path. Growable SharedArrayBuffers can only grow in place within the reserved max and keep a stable data pointer, so they stay borrowed.

Because a PathLike::Buffer can now own its allocation, src/jsc/node_path.rs gains:

  • Drop for PathLike frees an owned snapshot via MarkedArrayBuffer::destroy (idempotent; no-op for JS-owned backings).
  • Clone for PathLike dupes an owned payload instead of borrowing it, so the clone is independently droppable (mirroring the owned-String arm).

The owned snapshot carries value = JSValue::ZERO, so to_thread_safe/unprotect's protect()/unprotect() calls are no-ops for it, and the protect_eat() on the original JS value is skipped (nothing to root once the bytes are copied).

Semantics

Snapshotting at capture time means a resizable-backed path is read as it was when passed, so an option getter that overwrites the bytes in place (without resizing) no longer changes which path is used. That differs from both Node and Bun's non-resizable path (which borrow and see the overwrite). Matching Node's read-after-options order would require reordering path parsing in every args::*::from_js and re-reading the live byteLength from JSC after arbitrary JS ran; that is a much larger refactor than this crash fix and would need to cover every PathLike caller at once. The practical impact is limited to programs that deliberately rewrite the path buffer from inside an option getter. The existing pin-against-transfer() tests continue to pass unchanged (they use fixed-length buffers).

Verification

test/js/node/fs/fs.test.ts spawns a subprocess that exercises writeFileSync (Uint8Array path, flag getter), readFileSync (DataView path, encoding getter), mkdirSync (ArrayBuffer path, recursive getter), fs.promises.rename (async, post-call shrink), a growable SharedArrayBuffer path, and Bun.file(view).text() called twice (stores the owned PathLike, then clones it on the JS thread and again on the worker, covering the new Clone/Drop arms under ASAN). The subprocess segfaults on stock bun and exits 0 with this change. The existing pin tests ("keeps a ... attached") and the readFileSync/mkdirSync/statSync suites pass.

Related: #32189 covered the async side only; this change covers sync and async in one place. #35821 is the sibling fix for StringOrBuffer. #31221 added the pin that guards transfer() but not resize().

no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/fs/fs.test.ts