shell: clamp a `> ${buf}` redirect to the target buffer's live length

Closed
RO
19d ago
+282-7

Problem

  • Bun.$ crashes with panic(main thread): Segmentation fault when a > ${buf} target shrinks mid-command, or when its WebAssembly.Memory grows.
  • The shell pins the target but keeps its first pointer and length: BuiltinIO::write_no_io_to (src/runtime/shell/Builtin.rs:357), BufferedOutput::append and slice (src/runtime/shell/subproc.rs:1546, :1536). A pin stops only a transfer().

Fix

  • PinnedArrayBuffer::live_slice_mut() and live_slice() ask JSC for the view's current range before each use. A detached view reads as empty.
  • Both writers and the 2>&1 ${buf} tee use them, for every kind of buffer. All run on the JS thread, so the re-read cannot race a resize.
  • Verified: test/js/bun/shell/bunshell.test.ts, six new tests, each fails on 1.4.2 and on main. Also test/js/bun/shell/.

Background

  • PinnedArrayBuffer is the pin plus GC root the shell holds on a redirect target. resize() ignores the pin and makes the trimmed pages inaccessible. A bounds-checked wasm grow frees the old block and detaches the buffer, pinned or not.
  • Considered a re-read for resizable buffers only: it misses the wasm grow. A copy (#40641) hides the output from the caller.

Downsides

  • A write into a buffer target costs 62 more instructions (844 to 906 per 8 KiB chunk), 197 if resizable. Wall clock: under the noise floor. Binary: +512 bytes.
  • Behaviour changes: a target that grows mid-command fills to its new length. result.stdout of cmd 2>&1 ${buf} is the bytes written, not the whole target.
  • Not fixed: external command output that does not fit is dropped with exit 0, no message (#43196).
Notes

Cost for a caller that never changes the target. Release builds of main (e32be5c66b) and of that commit with this branch merged, linux x64. perf and valgrind were not usable on the machine (perf_event_open returns EPERM). The counts come from a gdb script that single-steps one call from its entry to its return. Every sample of a configuration gave the same count.

CallTargetmainthis PRdelta
BuiltinIO::write_no_io_to, one 8 KiB chunk of yesfixed-length Uint8Array, Buffer, view of a wasm memory844906+62
samelength-tracking view of a resizable ArrayBuffer8441041+197
Yes::write_no_io_loop, four chunks and the task enqueuefixed-length36223870+248
sameresizable36224410+788
BuiltinIO::write_no_io_to, echo hellofixed-length65127+62
sameresizable65262+197
PipeReader::on_read_chunk, 4096 bytes from an external commandfixed-length467522+55
sameresizable467657+190

The delta does not depend on the chunk size. It is the call into JSC plus the update of the cached range. echo > ${buf} pays it once per command.

Wall clock: one process runs yes > ${buf} 32 times into a 64 MiB target (262144 chunks). Runs alternate between the two builds on one pinned core, 25 rounds, 100 runs per target kind.

Targetmedian, mainmedian, this PRdeltaspread between two series of one build
fixed-length, wall786.4 ms747.5 ms-38.9 ms34.8 ms
fixed-length, user CPU403.6 ms405.8 ms+2.2 ms7.8 ms
resizable, wall1012.2 ms1013.1 ms+0.9 ms50.1 ms
resizable, user CPU423.0 ms428.7 ms+5.7 ms0.9 ms

The machine ran other jobs at the same time, so the spread is large. Each delta is of the same order as the spread. The wall clock and the CPU time show no cost beyond the one the instruction counts give: 62 or 197 instructions for each of the 262144 chunks.

Binary size (size on the stripped release binary): .text is 58300277 bytes on main and 58300789 with this PR, +512. .rodata, .data and .bss do not change. The file is 80995912 bytes in both builds.

What a program observes when it does not crash. The same two release builds.

Casemainthis PR
The target grows from 64 KiB to 1 MiB while yes writes65536 bytes written, yes: ENOSPC1048576 bytes written, yes: ENOSPC
The target shrinks from 1 MiB to 64 KiB while yes writessegfault65536 bytes written, yes: ENOSPC
A fixed-length view of 1 MiB whose buffer shrinks to 512 KiBsegfaultthe view is out of bounds and its length is 0, so nothing follows the 32768 bytes already written. yes: ENOSPC
sh -c 'printf OUT' 2>&1 ${buf}, 64-byte target filled with .result.stdout is 64 bytes, OUT and 61 dotsresult.stdout is OUT
An external command writes 12 bytes, the target shrinks to 6 after the first 6exit 0, empty stderr, 6 bytes keptthe same
An external command writes 12 bytes into a fixed-length target of 6exit 0, empty stderr, 6 bytes keptthe same

The six tests on a build without the fix. Each was run alone on released 1.4.2 and on a release build of main e32be5c66b. Four crash the test runner with panic(main thread): Segmentation fault (the two resize(0) cases, the 2>&1 ${buf} case, the shrink to a shorter length). The grow case fails its assertion. In the wasm case the spawned child crashes and the test fails. All six pass on the release build and on the debug ASAN build of main with this branch merged.

The branch against current main. A merge of this branch into main e32be5c66b is clean. Eight commits on main since the branch point touch the same files. On a debug ASAN build of the merge, bunshell.test.ts gives 438 pass and 4 fail. The four are 5 s timeouts on a loaded machine: long pipeline, pathological deep nesting with long chains, &> and &>> redirect, does not modify export env of parent. Three pass when run alone, 3 of 3. long pipeline also times out on the debug build of e32be5c66b without this branch, 3 of 3. On the release build of the merge the file gives 441 pass and 1 fail, the same long pipeline timeout. The other 42 files in test/js/bun/shell/ give 534 pass and 5 fail on both release builds. Three failures are the same on both: the two ls permission cases (the run was as root) and shell load > immediate exit, a 90 s timeout. The other two are 5 s timeouts, and the two builds differ only in which tests hit that timeout. fd leak > #11816, the pair that timed out with this branch, times out on the build of main too when run alone, 3 of 3.

Details moved from the earlier description. A target that shrank, or whose buffer is gone, stops the write: the builtin path reports ENOSPC, the subprocess path drops the rest. The binding reports vector() and byteLength(), which follow a resize and an auto-length view, and reports null and 0 for a detached view. Only the signaling mode of a wasm memory grows in place.


Earlier notes, unchanged:

Reproduction (released 1.4.3, SEGV 3 of 3 for each):

// builtin writer
const ab = new ArrayBuffer(1 << 24, { maxByteLength: 1 << 25 });
const u8 = new Uint8Array(ab);
const p = Bun.$`yes > ${u8}`.quiet().nothrow();
const done = p.then(r => r); // Bun.$ is lazy: this starts the command and takes the pin
await Bun.sleep(0);
ab.resize(0);
await done;

.then() matters. Without it the pin is taken after the resize, the captured length is 0, and every write is skipped.

Why the faulting address is inside the buffer: tryAllocateResizableMemory reserves maxByteLength rounded up to the page size and protects everything past the initial length. ArrayBuffer::resize to a smaller length calls OSAllocator::protect(memory + desiredSize, bytesToSubtract, false, false). The base pointer never moves, so the stale pointer still points at the reservation, and the write lands on a PROT_NONE page.

Test shape:

  • The builtin test uses yes, which writes four 8 KB chunks per event-loop turn. .then() runs the first batch synchronously, the await Promise.resolve() drains the microtask queue before the loop picks up the next batch, so the resize(0) is always between two batches.
  • The external-command test gates the child on a local Bun.serve: the child writes, fetches, then writes again. The parent resizes while the child waits, so the second chunk always arrives after the resize.

Suites run with the debug build: all of test/js/bun/shell/. The failures there (fd leak > memleak_*, shell load > immediate exit, the two ls permission cases) reproduce without this change. The leak tests pass under a release binary, so their 100 s timeouts are debug and ASAN build speed, and the ls permission cases need a non-root user.

The third site, found by review after the first two were fixed: BufferedOutput::slice() is the tee a command's close path runs, and it read the range captured at pin time. An earlier note here claimed no reader reaches it for a buffer target. That was wrong. 2>&1 ${buf} is the one spelling that sets DUPLICATE_OUT with a buffer target, which inverts redirects_elsewhere() for stderr and reaches the tee. It also copied the whole target instead of the written prefix: 5 bytes of output memcpy'd 8 MiB. The patched build still SEGV'd on that spelling until the last commit, which re-reads the range there (live_slice) and stops at the cursor.

refresh / live_extent / JSC__JSValue__arrayBufferExtent are meant to be the one place that asks JSC for a pinned value's current range. #42203 fixes the same hazard for fs.read into a wasm-memory view and adds its own write_back. Whichever lands second should call this primitive rather than add a second one.

The wasm face, and why it needs two env settings. Reproduction (released 1.4.3, crash 20 of 20):

// BUN_JSC_useWasmFastMemory=0 Malloc=1 bun wasmgrow.js
const mem = new WebAssembly.Memory({ initial: 128, maximum: 132 });
const target = new Uint8Array(mem.buffer);
const p = Bun.$`yes > ${target}`.quiet().nothrow();
const running = p.then(o => o);
await Promise.resolve();
mem.grow(4);
console.log((await running).exitCode);

BUN_JSC_useWasmFastMemory=0 picks BoundsChecking, which reallocates on a grow. The signaling mode grows in place, so the stale pointer stays valid there and the bug is invisible. Malloc=1 turns off the Gigacage: the freed block is then unmapped instead of returned to a cage free list that keeps it mapped. With the default settings the same script prints 1 and exits 0, which is why a fuzz pass over the release binary does not see this face.

The target view spans the whole memory on purpose. A 1 MiB view over an 8 MiB memory crashed only about half the time, because a stale write of 32 KB can land in a part of the freed hole that something else has taken. A view over the whole memory crashed 20 of 20.

Frames on the debug ASAN build, before the second commit (WRITE of size 8192, the first write after the grow):

__asan_memcpy
core::slice::copy_from_slice_impl::<u8>
<bun_runtime::shell::builtin::BuiltinIO>::write_no_io_to  src/runtime/shell/Builtin.rs:367
<bun_runtime::shell::builtins::yes::Yes>::write_no_io_loop  src/runtime/shell/builtin/yes.rs:105
<bun_runtime::shell::builtins::yes::YesTask>::run_from_main_thread  src/runtime/shell/builtin/yes.rs:245
bun_runtime::dispatch::run_task  src/runtime/dispatch.rs:333

yes writes four 8 KB chunks per event-loop turn, so 32768 bytes land before the grow() and the rest after it. That count is the same for a file and for -e.

The subprocess door has the same wasm face: sh -c 'sleep 0.3; head -c 200000 /dev/zero' > ${viewOverWasmMemory} with a grow() at 100 ms crashes 3 of 3 on 1.4.3. It is not a separate test, because both doors reach the target through live_slice_mut, and the subprocess door already has a resize test with a deterministic gate.

Checked the other two ways a pinned target can change under a deferred writer. A transfer() of a pinned buffer copies and leaves the source attached, so it is safe. A growable SharedArrayBuffer never detaches and keeps its reservation, and the re-read picks up the new length.


no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/shell/bunshell.test.ts