PinnedArrayBuffer: copy a resizable ArrayBuffer whose bytes are read after the call

Merged
RO
2mo ago
+217-14

Problem

  • A PinnedArrayBuffer borrow read after JS runs again crashes when a resizable ArrayBuffer shrinks in between: Bun.zstdCompress (panic: Segmentation fault), fs.promises.stat with a Buffer path (SEGV in PathLike::slice_z_with_force_copy, src/runtime/node/types.rs:945), and a sync fs call whose option getter calls resize(0) on the path's buffer.
  • Cause: a pin blocks a detach, not a shrink. ArrayBuffer::resize unmaps the trimmed pages, so the held (ptr, len) points at unmapped memory.

Fix

  • Add PinnedArrayBuffer::copy_if_resizable and root_read_only (src/jsc/array_buffer.rs): a resizable non-shared buffer gets a copy of its current bytes, and the view points at the copy until the handle drops.
  • Used where the bytes are read in user space after the call: StringOrBuffer::from_js_async (zstd), the PathLike funnel (both arms) and CompressionStreamCoder::AsyncInput::new. fs.write keeps the zero-copy borrow (write(2) returns EFAULT). fs.read and shell redirects write into the buffer and keep root.
  • Correct because only a resizable non-shared buffer can unmap under the borrow: a fixed-length buffer cannot shrink and a growable SharedArrayBuffer only grows in place. Node copies a Buffer path at call time too.
  • Verified: test/js/bun/util/zstd.test.ts (three tests), test/js/node/fs/fs.test.ts (two tests). Four of five fail on the unfixed build. Other suites in Notes.

Background

  • PinnedArrayBuffer is the Rust handle for a borrowed JS buffer: pin stops a detach, root also GC-roots the value for a job that outlives the call.
  • PathLike is a parsed path argument. Its Buffer arm holds a PinnedArrayBuffer read by the fs call, the pool thread, or a Blob store.
  • JSC answers a shrink of a resizable ArrayBuffer with mprotect(PROT_NONE) on the trimmed tail, so a stale pointer faults.
Notes

Repro on main (834ad12725):

for (let i = 0; i < 20; i++) {
  const ab = new ArrayBuffer(256 * 1024, { maxByteLength: 1 << 21 });
  new Uint8Array(ab).fill(0x41);
  const p = Bun.zstdCompress(new Uint8Array(ab));
  ab.resize(0);
  await p;
}
  • Release: panic: Segmentation fault at address 0x26447780001. Debug: ASAN SEGV on unknown address in MEM_read64 (vendor/zstd/lib/common/mem.h:184) on thread T2. Bun.zstdDecompress with a compressed input crashes the same way in MEM_read32.
  • The PathLike case: const p = fs.promises.stat(new Uint8Array(resizableAB)); resizableAB.resize(0); gives ASAN SEGV in PathLike::slice_z_with_force_copy on thread T3 (the path is copied into a path buffer on the pool thread).
  • The sync case: fs.writeFileSync pins the path, then reads the flag getter, then copies the path into a path buffer (write_file_with_path_buffer, node_fs.rs:7170). A getter that calls resize(0) gives ASAN SEGV in slice_z_with_force_copy on the main thread. readFileSync with a DataView path and an encoding getter, and mkdirSync with a raw ArrayBuffer path and a recursive getter, crash the same way.
  • The Bun.file case: the store keeps the pinned buffer (PathLike::thread_isolated_copy). .text() clones the stored path (PathLike::clone, node_path.rs:39, to_vec of the slice) on the JS thread and faults after a shrink. The sync-arm copy fixes it, but the test does not assert it: Bun.file(anyBuffer) with BUN_DESTRUCT_VM_ON_EXIT=1 (the ASAN lane) trips validateIsNotSweeping at exit, because the store's PinnedArrayBuffer drops during the shutdown sweep and unpin downcasts the cell there. That is on main for every Buffer path since #40511 and is separate from this change.
  • Out of scope: the shell's ArrayBuffer redirects (> ${buf}, < ${buf}, src/runtime/shell/Builtin.rs, Cmd.rs) still root and read or write the view in user space, so a resize(0) while the command runs is still unsafe there. A redirect target has to receive the output, so a copy is not the fix for that site.
  • Why fs.write is not copied: its pool-side reader is write(2). The kernel returns EFAULT for an unmapped page, so the caller gets an error and the process does not crash. A copy at the funnel would run before offset and length are parsed, so a small write from a large resizable buffer would copy the whole view. Node documents that the buffer must not change until the callback runs.
  • Why the copy lives inside PinnedArrayBuffer and not at the call sites: the PinnedBuffer and PathLike::Buffer variants drive argument dispatch and carry the JS value. A copy that keeps the variant, the pin and the root changes nothing for those readers. The first version of this PR copied at the zstd site only; the review found the same crash through PathLike.
  • Why not copy every input: the pinned borrow is the zero-copy path for the common fixed-length Buffer. An empty resizable buffer is not copied either: it has nothing to read, and a later grow keeps the pointer valid.
  • The copy is freed with the job: 2000 rounds of Bun.zstdCompress on a 256 KiB resizable input leave RSS flat on the debug build.
  • #34751 reported the zstd crash together with the same shape in crypto.pbkdf2/crypto.scrypt and node:zlib. The KDFs are fixed by #40554 (they copy every input, because a caller may zeroize a secret after the call). node:zlib rejects a resizable input since #36165. #34751 was closed because it was built on the plumbing #40511 replaced. #31645 proposed a funnel-level copy on that older plumbing.
  • Supersedes #32189 and #35840, which made the same path copy on the plumbing #40511 replaced. Their cases pass on this branch: a rename with a shrink right after the call, 256 queued renames of a missing source as Uint8Array, DataView and raw ArrayBuffer (all ENOENT), the sync writeFileSync/readFileSync/mkdirSync getter shapes, a growable SharedArrayBuffer path, and Bun.file(view).text() read twice.
  • Other suites run: test/js/node/fs/fs.test.ts (562 pass), test/js/node/fs/promises.test.js, test/js/bun/shell/bunshell.test.ts, test/js/web/streams/compression.test.ts, test/js/node/zlib/zlib.test.js, test/js/node/crypto/pbkdf2.test.ts, test/js/node/crypto/scrypt.test.ts, test/js/bun/util/bun-file.test.ts, test/js/bun/util/bun-file-read.test.ts, test/js/web/fetch/blob.test.ts.

no test proof · iteration 4 · 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