PinnedArrayBuffer: copy a resizable ArrayBuffer whose bytes are read after the call
Merged
RO
2mo ago+217+217+012345678901234567890123456789-14-14-01234567890123456789
Problem
- A
PinnedArrayBufferborrow read after JS runs again crashes when a resizableArrayBuffershrinks in between:Bun.zstdCompress(panic: Segmentation fault),fs.promises.statwith a Buffer path (SEGVinPathLike::slice_z_with_force_copy,src/runtime/node/types.rs:945), and a sync fs call whose option getter callsresize(0)on the path's buffer. - Cause: a pin blocks a detach, not a shrink.
ArrayBuffer::resizeunmaps the trimmed pages, so the held(ptr, len)points at unmapped memory.
Fix
- Add
PinnedArrayBuffer::copy_if_resizableandroot_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), thePathLikefunnel (both arms) andCompressionStreamCoder::AsyncInput::new.fs.writekeeps the zero-copy borrow (write(2)returnsEFAULT).fs.readand shell redirects write into the buffer and keeproot. - Correct because only a resizable non-shared buffer can unmap under the borrow: a fixed-length buffer cannot shrink and a growable
SharedArrayBufferonly 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
PinnedArrayBufferis the Rust handle for a borrowed JS buffer:pinstops a detach,rootalso GC-roots the value for a job that outlives the call.PathLikeis a parsed path argument. ItsBufferarm holds aPinnedArrayBufferread by the fs call, the pool thread, or aBlobstore.- JSC answers a shrink of a resizable
ArrayBufferwithmprotect(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: ASANSEGV on unknown addressinMEM_read64(vendor/zstd/lib/common/mem.h:184) on thread T2.Bun.zstdDecompresswith a compressed input crashes the same way inMEM_read32. - The
PathLikecase:const p = fs.promises.stat(new Uint8Array(resizableAB)); resizableAB.resize(0);gives ASANSEGVinPathLike::slice_z_with_force_copyon thread T3 (the path is copied into a path buffer on the pool thread). - The sync case:
fs.writeFileSyncpins the path, then reads theflaggetter, then copies the path into a path buffer (write_file_with_path_buffer,node_fs.rs:7170). A getter that callsresize(0)gives ASANSEGVinslice_z_with_force_copyon the main thread.readFileSyncwith aDataViewpath and anencodinggetter, andmkdirSyncwith a rawArrayBufferpath and arecursivegetter, crash the same way. - The
Bun.filecase: the store keeps the pinned buffer (PathLike::thread_isolated_copy)..text()clones the stored path (PathLike::clone,node_path.rs:39,to_vecof 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)withBUN_DESTRUCT_VM_ON_EXIT=1(the ASAN lane) tripsvalidateIsNotSweepingat exit, because the store'sPinnedArrayBufferdrops during the shutdown sweep andunpindowncasts 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) stillrootand read or write the view in user space, so aresize(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.writeis not copied: its pool-side reader iswrite(2). The kernel returnsEFAULTfor an unmapped page, so the caller gets an error and the process does not crash. A copy at the funnel would run beforeoffsetandlengthare 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
PinnedArrayBufferand not at the call sites: thePinnedBufferandPathLike::Buffervariants 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 throughPathLike. - 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.zstdCompresson 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.scryptandnode:zlib. The KDFs are fixed by #40554 (they copy every input, because a caller may zeroize a secret after the call).node:zlibrejects 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
renamewith a shrink right after the call, 256 queued renames of a missing source asUint8Array,DataViewand rawArrayBuffer(allENOENT), the syncwriteFileSync/readFileSync/mkdirSyncgetter shapes, a growableSharedArrayBufferpath, andBun.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