Fix Windows use-after-free of the pipe reader buffer retained by in-flight libuv reads

Merged
RO
3mo ago
+267-61

What does this PR do?

Fixes a Windows use-after-free: tearing down a pipe reader while a cooked-mode console read is in flight frees the buffer libuv's worker thread is still writing into.

Problem

  • On Windows, WindowsBufferedReader serves every libuv read from _buffer's spare capacity. For a cooked-mode console read (the default for process.stdin / Bun.stdin.stream() on a console), libuv's uv__tty_queue_read_line stores that pointer and blocks a worker thread in ReadConsoleW; the worker converts the typed line into the buffer whenever ReadConsoleW returns (uv_utf16_to_wtf8 into read_line_buffer.base), including when the read was cancelled mid-flight: uv_read_stop cancels asynchronously by injecting a VK_RETURN, and after read_stop the buffer is never handed back through read_cb.
  • Reader teardown (deinit() / Drop / finish()'s shrink_to_fit) frees or moves _buffer regardless, so the worker's late write lands in freed mimalloc memory and corrupts whatever is allocated there next. The crash then surfaces arbitrarily far away, typically as a fault on the corrupted allocation's next owner.
  • This is a plausible mechanism for the Windows-dominant Sentry crash family around Heap::sweepArrayBuffers -> ~ArrayBufferContents -> mi_free (BUN-30T8 and siblings): the corrupted blocks are prime-size tenants for ArrayBuffer backing stores, and release-build _mi_ptr_page is unchecked only off macOS, matching the platform skew. No crash dump directly ties the family to this path, so that attribution is a hypothesis, not a claim.
  • Separately, deinit() emptied _buffer before calling close_impl, so the orphaned_read_buf parking that #37075 added for in-flight uv_fs_read received an empty Vec and the threadpool write still targeted a freed block on that path.

Fix

  • Own the console read buffer by what libuv ties the read's lifetime to: the handle. uv::Tty becomes a #[repr(C)] wrapper over uv_tty_t (the same shape as File over uv::fs_t) with a read_scratch: Vec<u8>; alloc_cb serves tty reads from it, and on_stream_read stages delivered chunks into _buffer so commit/callback behavior is byte-identical to a pipe chunk.
  • The scratch is freed with its handle: libuv only runs a handle's close callback once no requests are pending on it (the parked line read holds one), and the process-static stdin tty is never closed at all. Heap ttys drop the scratch with the Box in on_tty_close / the open_handles teardown path.
  • Pipe reads complete synchronously inside uv__pipe_read_data on the loop thread and keep using _buffer; uv_fs_read keeps main's orphaned_read_buf parking. HAS_INFLIGHT_READ semantics are untouched, so has_pending_read() behaves exactly as on main (no per-chunk buffer churn for FileReader over a pipe).
  • deinit() now frees _buffer after close_impl, so the orphaned_read_buf parking gets the real allocation.
  • The tty scratch gets the same ReadLimit clamp main applies to _buffer (#39296), so a sliced read over a tty is cut at the limit exactly as over a pipe. This is the only change the rebase onto the PipeReader rework (#38886) needed; the staging copy in on_stream_read lands in _buffer's spare capacity, which is what main's on_read / on_read_chunk commit and hand to the parent.

How did you verify your code works?

  • test/js/bun/terminal/terminal-spawn.test.ts "cancelling a parked console stdin read does not corrupt the heap" (Windows-only, ConPTY via Bun.Terminal so the child has a real console): the child proves the cooked-mode line-read machinery works with a parent round-trip, arms a read, cancels it while the worker is parked in ReadConsoleW (the pre-fix free point), immediately adopts the freed-size allocation with ArrayBuffer probes, and fails if the cancelled read's late write mutates one. The run is one-sidedly nondeterministic: a cycle where the worker had not yet entered ReadConsoleW is vacuous, never a false failure.
  • Windows x64 debug build on the rebased base (main at 771c7e67e0): with only the test applied, the test fails 7 of 8 runs, each with the child tripping mimalloc: error: corrupted free list entry of size 10240b (the debug size class of libuv's 8 KiB line-read buffer); with the fix, 12 of 12 runs pass, the whole file passes (14 pass, 4 platform skips), and terminal.test.ts, terminal-platform-gaps.test.ts, process-stdin.test.ts, stdin-fixtures.test.ts, spawn-streaming-stdin.test.ts, bun-stdin-slice.test.ts, tty.test.ts and the tty regression tests pass (133 pass, 0 fail). A raw-mode ConPTY child (libuv's per-key-event read path through the same staging code) also received its input intact.
  • cargo check --workspace passes for x86_64-pc-windows-msvc and aarch64-pc-windows-msvc.

Background

  • libuv on Windows gives every stream read its buffer via alloc_cb and normally returns it through read_cb in the same loop iteration. The exception is the cooked-mode console line read: it parks a worker thread in ReadConsoleW, retains the alloc_cb buffer across loop iterations, and a cancellation via uv_read_stop never produces a read_cb for the in-flight read, so no reader-side completion point exists to free that buffer safely. The handle is the only object whose lifetime libuv guarantees covers the read.
Superseded earlier versions of this PR

Earlier revisions also (a) parked in-flight uv_fs_read buffers on the File and consumed DEFER_DONE_CALLBACK on normal completion after a lost uv_cancel race (both landed on main in a different shape via #37075), (b) redefined HAS_INFLIGHT_READ as a retention flag with clears moved to the read callbacks (dropped: it flipped has_pending_read() inside the chunk callback, causing a free+malloc per chunk for FileReader over a pipe), and (c) leaked one buffer per torn-down stdin reader, later reclaimed at the next alloc_cb, guarded by a malloc_bins-based leak test (dropped: the counters read 0 on release builds, making the test vacuous on CI, and the handle-owned scratch removes the leak outright).

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/terminal/terminal-spawn.test.ts