fork: fail truthfully when a fork continuation overruns its save buffer#845
Open
brandonpayton wants to merge 1 commit into
Open
fork: fail truthfully when a fork continuation overruns its save buffer#845brandonpayton wants to merge 1 commit into
brandonpayton wants to merge 1 commit into
Conversation
The wpk_fork unwind writes the saved call stack into a fixed FORK_SAVE_BUFFER_SIZE (16 KiB) buffer that sits immediately below the syscall channel (forkBufAddr = channelOffset - FORK_BUF_SIZE). The instrumented unwind carries no bounds check of its own — crates/fork-instrument/src/runtime.rs documents the requirement `frames_start_offset + Sigma(frame) <= buffer_size` but never enforces it. So a fork() from a call stack too deep or wide to fit silently overruns the buffer into the channel, corrupting the syscall channel and later surfacing as an unexplained trap or a fork child that never makes progress — never as the real cause. Detect the overrun host-side, right after the unwind completes and before SYS_FORK is sent: current_pos (the write cursor the instrumentation keeps at the buffer base, which frames never clobber) exceeding the buffer size means the unwind wrote past it into the channel. When it does, fail the fork with a clear diagnostic naming the byte overage and the platform limit, instead of forking on a corrupted channel. Applied to both the main-process and thread fork paths, which Node and browser hosts share. Validated in a real browser by temporarily shrinking the buffer: a fork that overruns now reports "fork() continuation save buffer overflow ..." and dies cleanly, while forks that fit still succeed (no false positives). Adds a unit test for the detection arithmetic (wasm32 + wasm64); the existing fork/spawn/thread suites pass unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The
wpk_forkunwind saves the call stack into a fixedFORK_SAVE_BUFFER_SIZE(16 KiB) buffer that sits immediately below the syscall channel (forkBufAddr = channelOffset - FORK_BUF_SIZE). The instrumented unwind has no bounds check of its own —crates/fork-instrument/src/runtime.rsdocuments the requirementframes_start_offset + Σframe ≤ buffer_sizebut never enforces it.So a
fork()from a call stack too deep or wide to fit silently overruns the buffer into the channel, corrupting the syscall channel. Today that surfaces only later and elsewhere — an unexplained wasm trap, or a fork child whose channel never works so its worker "never starts" — never as the real cause.This PR detects the overrun host-side, right after the unwind completes and before
SYS_FORKis sent, in both the main-process and thread fork paths (shared by the Node and browser hosts).current_pos— the write cursor the instrumentation keeps at the buffer base, which frames grow away from and never clobber — exceeding the buffer size means the unwind wrote past it. When it does, the fork fails with a clear diagnostic instead of proceeding on a corrupted channel:This is a truthful-failure hardening (per the platform's "truthful failure over convenient illusion" contract): it turns silent corruption into a diagnosable failure at the fork point. It does not raise the limit — a deeper/wider fork still fails, just visibly and for the right reason.
Validation
exit=-1), while forks that fit still succeed (depth 2/5/10 → 6/6 children). Reported byte count scales exactly with stack depth.host/test/fork-save-buffer-overrun.test.ts) pins the detection arithmetic for the boundary, overrun, and wasm32/wasm64 cases.nested-fork-repro,fork-from-thread,fork-dlopen-replay-e2e,centralized-spawn,spawn-host-parity,pthread) pass unchanged — no false positives on normal forks.Scope / notes
host/src/worker-main.ts); no ABI change, no re-instrumentation, no kernel change.crates/fork-instrument), a larger, ABI-adjacent change; this PR is the safe, self-contained first step.🤖 Generated with Claude Code