Conversation
…table is truncated Linux maps each PT_LOAD's full p_filesz at execve without checking that the file is that long, so a compiled executable whose download or copy was cut short still starts, and the first read of the embedded module graph (the trailer, in StandaloneModuleGraph::from_executable) dies with SIGBUS and a crash report. Before reading the payload's length header and its trailer, probe the page each one lives on with madvise(MADV_POPULATE_READ). It faults the page in exactly like the read that follows would, but a page the file does not back comes back as EFAULT, which is turned into a real error message and exit code 1. The executable is never opened or stat'd, and kernels without MADV_POPULATE_READ (< 5.14) return EINVAL and keep the previous behaviour. Because truncation removes a suffix of the file, the trailer page being present implies everything before it is too. macOS, Windows and FreeBSD reject a truncated image at exec time, so the probes are Linux/Android only.
|
Warning Review limit reached
Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Status: ready for review. Reproduced on bun 1.4.0 canary (Linux x64): Supersedes #35134 (the |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Because it changes the startup path of every compiled Linux binary and picks a specific approach (MADV_POPULATE_READ probing) over alternatives that were previously declined (#29665, #35134), a maintainer sign-off on the mechanism would still be worthwhile.
What was reviewed:
get_data()refactor intoget_blob()+get_payload()is behavior-preserving;len >= 8is guaranteed beforelen - 1, andwrapping_addhandles a corrupted length by landing on an unmapped page (ENOMEM, ignored).- Only
EFAULTtriggers exit;EINVALon pre-5.14 kernels and every other errno fall through to the existing unchecked read, so old kernels keep today's behavior. - Plain
bunreturns before probing (vaddr == 0); FreeBSD build compiles to plainget_data(); execute-only binaries stay working since nothing opens the file. - Test correctly locates the payload via section headers (avoids loading the ~1 GB debug binary), gates on kernel 5.14+, disables LSAN for the truncated runs, and copies before truncating to dodge ETXTBSY.
Extended reasoning...
Overview
This PR converts a SIGBUS crash into a clean error message when a bun build --compile executable on Linux has been truncated (e.g., interrupted download). It refactors elf::get_data() in src/standalone_graph/StandaloneModuleGraph.rs into get_blob() + get_payload(), adds get_data_or_exit_if_truncated() (Linux/Android: probes the length-header page and the trailer page with madvise(MADV_POPULATE_READ) and exits with an error:/note: message on EFAULT; FreeBSD: identical to get_data()), and switches from_executable to call it. A new Linux-only test in test/bundler/bun-build-compile.test.ts compiles a binary, locates the .bun section via ELF headers, and asserts the exact error output for two truncation points plus success on the intact binary.
Security risks
None identified. The change reads no user-controlled input beyond what from_executable already reads (the mapped payload). madvise is called on an address derived from the process's own segment mapping; the only new filesystem access is self_exe_path() (readlink of /proc/self/exe) on the error path only. The wrapping_add on the untrusted len means a garbage length lands on an unmapped page and madvise returns ENOMEM, which is ignored — same behavior as today.
Level of scrutiny
High. This code runs unconditionally at startup of every compiled standalone binary on Linux/Android, before any user code. A false positive would break every compiled app. The mitigations look sound — plain bun returns before the first probe, only EFAULT triggers exit, and the existing compile test suite (execute-only, large-payload, patchelf, WSL1-shape) all run through the two probes and would catch a false positive — but a maintainer should confirm the MADV_POPULATE_READ approach is the one they want, given that two prior PRs (#29665, #35134) took different approaches and one was explicitly declined.
Other factors
The PR description is unusually thorough and pre-emptively addresses the review criteria: it explains why stat/open were avoided (prior maintainer feedback + execute-only binaries), why only two pages need probing (trailer is last, header is first), why FreeBSD/macOS/Windows are excluded (their loaders reject short images at exec), and documents USE_SYSTEM_BUN=1 failure, ASAN behavior, and clippy across all four cfg targets. The test follows harness conventions (tempDir, bunEnv spread, concurrent pipe drain, combined-object assertion, skipIf with a stated reason, per-test timeout with the same justification as the neighboring #29963 test). The refactor of get_data() preserves the write-provenance comment and the payload_len < 8 guard. I found nothing to flag; deferring only because the mechanism choice is a design decision with history in this repo.
|
Updated 7:00 PM PT - Aug 13th, 2026
✅ @robobun, your commit b08eb4b8a0fb7faee2a08d10af91f844caaf4d1b passed in 🧪 To try this PR locally: bunx bun-pr 38251That installs a local version of the PR into your bun-38251 --bun |
|
Reached the same root cause from the newer Sentry groups (BUN-4E9B and BUN-4D8P: the same crash under the Rust symbol names, 132 events, plus the older BUN-3PTA, BUN-3Q6K and BUN-3RHW groups). Notes for whoever picks this up:
|
… in 1.3.14) (#42894) ### Problem - On Linux, `Bun.build({ compile })` in a compiled app reports success, but the new executable crashes at startup: `panic(main thread): Segmentation fault at address 0x993CFF8` in `StandaloneModuleGraph::from_executable`. So do `BUN_BE_BUN=1 ./app build --compile` and `--compile-executable-path ./app`. - Regression: 1.3.11 works, 1.3.12 and 1.3.13 refuse (`NoGnuStackSegment`), 1.3.14 and later write the corrupt executable. chenxin-yan/crust#370 works around it. - Cause: `write_bun_section` (`src/exe_format/elf.rs:363`) finds `BUN_COMPILED` through the `.bun` section header. In a compiled executable that header names the payload. The new address lands on the old payload's length. `BUN_COMPILED` keeps the old address. ### Fix - The writer recognizes a payload by its layout and writes the new one at the same address, which `BUN_COMPILED` already holds. - It refuses any other non-zero `.bun` section: `BunSectionAlreadyWritten`, exit 1. That includes the corrupt executables from 1.3.14 to 1.4.x. - Correct because the output is byte-identical to a compile into a plain bun, which the tests assert. - Verified: `test/bundler/compile-elf-segment-layout.test.ts`. 17 of the 19 new tests fail on 1.4.3-canary.1+09bb54630. 18 of them run on every host (checked on Windows x64). Self-reviewed: two findings, both fixed (Notes). ### Background - `BUN_COMPILED` is an 8-byte variable in the `.bun` section of every bun. It holds 0 in a plain bun, and the address of the payload (`[u64 length][module graph]`) in a compiled executable. - `bun build --compile` copies an executable, appends the payload past everything it maps, and points the `.bun` section header at the payload. - A compiled executable that compiles copies itself. `BUN_BE_BUN=1` makes it act as the `bun` CLI. <details><summary>Notes</summary> **Repro** (Linux x64, `1.4.3-canary.1+09bb54630`): ```sh echo 'console.log("first");' > a.ts echo 'console.log("second");' > b.ts bun build --compile a.ts --outfile ./first BUN_BE_BUN=1 ./first build --compile b.ts --outfile ./second # exit 0 ./second # panic(main thread): Segmentation fault at address 0x993CFF8, exit 139 ``` A debug build reports a READ SEGV in `memcmp`, under `StandaloneModuleGraph::from_executable` (`src/standalone_graph/StandaloneModuleGraph.rs:2908`, the trailer compare). The bytes of `./second`: | file offset | what | value | | --- | --- | --- | | `0x47894f8` | `BUN_COMPILED` | `0x4c9e000`, the address of the first payload | | `0x49d1000` | length of the first payload | `0x4c9f000`, the address of the second payload | | `0x49d2000` | length of the second payload | `0xad` | The fault address is `0x4c9e000 + 8 + 0x4c9f000 - 16 = 0x993CFF8`. **Release window.** I ran the three routes on the release binaries: 1.3.11 produces working executables (it appended the graph to the end of the file). 1.3.12 and 1.3.13 print `Error writing .bun section to ELF: error.NoGnuStackSegment` and exit 1. 1.3.14 (the writer that grows the `PT_LOAD`, #29963) and the canary exit 0 and write the corrupt executable. **How a payload is recognized.** If the first word of `.bun` is 0, `.bun` is `BUN_COMPILED` and the payload is appended as before. A linked `.bun` always holds 0, and `to_executable` never writes an empty payload. If the first word is not 0, all of these must hold: the section is `[length][bytes]`, it starts a page, it starts at the first page boundary past the linked sections, its file offset agrees with the segment, the file backs all of the segment, the segment is the highest `PT_LOAD`, and the payload ends it. Then the payload is replaced. Anything else is refused. The test has one row for each condition. Each row changes a compiled executable so that only that condition fails. I checked with a standalone build of the writer that removing any one condition makes its row fail. **Executables that are already corrupt.** What 1.3.14 to 1.4.x wrote into a compiled executable has the second payload one or more pages past the sections. The writer refuses it (checked with executables made by 1.3.14 and by the canary). It does not repair it: nothing in the file says where `BUN_COMPILED` is. **Stripped executables.** GNU `strip` and `objcopy` rewrite a compiled executable from its sections. After a compile, no section header names the bytes of `BUN_COMPILED`, so these tools write 0 there. Such an executable already runs as a plain bun, with or without this PR. Its `PT_LOAD` no longer ends on a page, so the writer refuses it. That is the right result: a replaced payload would never load. `llvm-strip` keeps the bytes, and the writer accepts its output (the output runs). **A tool compiled by an affected bun stays affected.** The writer is part of the compiled tool. A CLI that was compiled by 1.3.14 to 1.4.x, and that compiles with itself, writes corrupt executables until the CLI is compiled again with a bun that has this fix. **Other platforms.** This PR changes only the ELF writer. - Windows refuses the same command: `Error adding Bun section to PE file: SectionExists`, exit 1 (run on `1.4.3-canary.1+c8b9b5853`). - Mach-O, writer level only (`--target=bun-darwin-x64` into a hand-built template, run on Linux, not executed on macOS): a payload that is not smaller replaces the old one, and the output is byte-identical to a direct compile. A smaller payload is refused: `Error writing standalone module graph: InvalidObject`, exit 1. This PR does not add shrinking to the Mach-O writer. **Overlap with open PRs.** - #37261 (and the same hunk in #37225) also lets `write_bun_section` replace a payload. It finds `BUN_COMPILED` through a trailer that it adds to each payload, and it sets `sh_size` to the aligned size. Executables compiled by 1.3.14 to 1.4.x have no trailer, so they take its append path, which is this bug. The two changes conflict in `write_bun_section`, and each refuses or misreads the payload format of the other. If this PR lands first, #37261 needs a rebase of its `elf.rs` hunk. Which recognizer stays is a maintainer decision. - #33621, #38936 and #35751 edit the same function. The conflicts are textual. - #31787, #41360, #37639 and #38251 harden the reader against a corrupt or truncated payload. This PR does not change the reader. **Self-review.** The first version of this change accepted an executable that the old writer had already corrupted: the compile exited 0 and the output crashed. It is refused now. Its tests covered one of the layout conditions. Now there is one row for each. **Checks.** - Real bun (Linux x64, debug build): `Bun.build({ compile })` in a compiled app, `BUN_BE_BUN=1`, and `--compile-executable-path` all produce executables that run. `first` -> `second` -> `third` chains, a payload that grows, a payload that shrinks, and `--bytecode` work. `cmp` reports no difference from a direct compile (same entry point and outfile name), for the release and the debug build. - Small C programs with the same `.bun` declaration, linked by lld as non-PIE and PIE, with and without `-z relro`, for x64 and aarch64: chains are byte-identical to direct compiles, and the x64 programs run and find the new payload, a zero `.bss`, and nothing of the old payload. - The Android builds of bun (PIE, x64 and aarch64): chains are byte-identical to direct compiles. - Payload sizes around a page boundary (`page - 9`, `page - 8`, `page - 7`, `2 * page - 8`, 1 byte), every old and new pair, for 4 KB and 64 KB pages: byte-identical to direct. - Suites on the debug build: `compile-elf-segment-layout`, `bun-build-compile`, `bundler_compile`, `compile-argv`, `compile-asset-bunfs`, `standalone`, and `test/internal/source-lints/`. In `bun-build-compile.test.ts`, four bytecode tests time out on the debug build with and without this change. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/compile-elf-segment-layout.test.ts <!-- robobun:evidence:end -->
Problem
bun build --compileexecutable whose file on disk is shorter than it should be (download or copy cut short) still starts, then dies before any user code runs withpanic(main thread): Bus error at address 0x...inStandaloneModuleGraph::from_executable, followed by the "Bun has crashed" banner and a bun.report link.write_bun_section(src/exe_format/elf.rs) grows the RWPT_LOADso the kernel maps the payload at exec. Linux maps the segment's fullp_fileszwithout checking that the file is that long, so a short file execs fine, and the first read of a page the file does not back raises SIGBUS. That read is the trailer compare infrom_executable(src/standalone_graph/StandaloneModuleGraph.rs, Linux branch), or the length header inelf::get_dataif the cut landed right at the start of the payload.Fix
from_executablenow goes throughelf::get_data_or_exit_if_truncated(), which callsmadvise(MADV_POPULATE_READ)on the page holding the length header before reading it, and on the page holding the last byte of the payload (the trailer) beforefrom_executablereads that.EFAULTprints an error and exits 1; every other result (EINVALon kernels older than 5.14,ENOMEM, ...) falls through to the same unchecked read as before.hint_source_pages_dont_needkeeps using the plainget_data().self_exe_path, called on the error path only):MADV_POPULATE_READfaults the page in exactly the way the read that follows would, and reports the condition that would have been a SIGBUS asEFAULT. Since the page is then already mapped, the read after it takes no fault: on a complete binary the two probes replace two page faults, and a plainbunreturns before probing (BUN_COMPILED.size == 0).from_bytesand the runtime read later is too; the header probe covers the one page before that. Together they cover every cut point inside the payload. A cut before the payload lands in bun's own.data/.bssand fails in ld.so or libc beforemain, where there is no crash handler to misattribute it.bun(checked: such a binary answers--version, before and after this PR). What an invalid trailer should do is the corruption question Don't crash at startup when a standalone executable's embedded graph is corrupted #31787 is about, not truncation.EFAULTis fatal, and it is the one errormadvisedocuments for this exact condition. Anything that does not implement the advice (kernels before 5.14, emulation layers) returnsEINVAL, the documented error for unknown advice, and keeps today's behaviour. bun's allocator and JSC already callmadviseat startup, so no sandbox sees a new syscall.stat("/proc/self/exe")plus a program header walk and was declined for exactly that ("we should not need to stat or read the executable"). Not touching the file is also what keeps execute-only (chmod 111) binaries working, which the mmap-at-exec design exists for; page faults do not check file permissions. It is also why the message carries no byte counts.get_data_or_exit_if_truncatedisget_data().ENOMEM, which is ignored.test/bundler/bun-build-compile.test.ts> "truncated compiled binary exits with an error instead of SIGBUS": compiles an app with a 1 MiB payload, finds the payload through the.bunsection header (under debug+ASAN hundreds of MB of debug info follow it, so cutting from the end of the file would miss), runs the intact binary, then runs copies cut 64 KiB into the payload (trailer probe) and at the payload start (header probe), asserting the exact stderr, empty stdout and exit code 1.BUSreport). Copies rather than truncating in place: the file that was just executed staysETXTBSYfor a few tens of ms after the child is reaped. The cut binaries run withdetect_leaks=0because LSAN's exit scan of the process would itself SIGBUS on the unbacked pages, which is what CI'sASAN_OPTIONSwould otherwise hit; verified with CI's options locally.bun-build-compile.test.ts,compile-argv.test.ts,compile-asset-bunfs.test.tsandbundler_compile.test.tspass on the debug build (the latter exceptcompile/HelloWorldWithProcessVersionsBun, which fails on main with any debug build and is fixed by test(bundler): fix compile/HelloWorldWithProcessVersionsBun on debug builds #37373). Every compiled binary those tests run goes through the two probes, so they also cover the no-false-positive side, including the execute-only cases.cargo clippy -p bun_standalone_graphis clean for linux-gnu, linux-musl, android and freebsd targets./proc/self/exeapproach from compile: fail gracefully instead of SIGBUS on truncated standalone binary #29665 (run frommain()). Complementary to Don't crash at startup when a standalone executable's embedded graph is corrupted #31787, which validates the stored length and vaddr of a full-length file and does not detect truncation.Background
bun build --compileappends[u64 payload_len][payload]at a page-aligned offset inside the template's RWPT_LOAD, extends that segment'sp_filesz/p_memszover it, and writes the payload's virtual address intoBUN_COMPILED, the 8-byte variable in the original.bunsection (0 in a plainbun). At startupelf::get_datadereferences that address; nothing is read from the file system. The payload ends with theOffsetsstruct and the 16-byteTRAILER, whichfrom_executablechecks before anything else.MADV_POPULATE_READ(Linux 5.14, 2021): prefault a range for reading. ReturnsEFAULTwhen a page in the range would raise SIGBUS on access,EINVALon kernels that do not know the advice,ENOMEMwhen the range is not mapped.Repro and before/after output
Before (bun 1.4.0 canary):
0x4AD50D7= payload vaddr0x49D5000+ 8 + payload_len - 16, i.e. the trailer compare. Truncating to exactly the payload start faults at0x49D5000, the length header.After: