-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Lower the .bun section alignment on ELF so PT_LOAD segments do not overlap #40756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8544d7e
Lower the .bun section alignment on ELF so PT_LOAD segments do not ov…
robobun 61b223e
[autofix.ci] apply automated fixes
autofix-ci[bot] b935220
Shorten the BLOB_HEADER_ALIGNMENT comments
robobun 6e62802
Drain the build subprocess stdout in the ELF layout test
robobun bae35b0
[autofix.ci] apply automated fixes
autofix-ci[bot] e2e36cb
Share the ELF program header reader between tests and cover FreeBSD
robobun 3781acd
[autofix.ci] apply automated fixes
autofix-ci[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| // The `.bun` section (the standalone module graph header) used to be declared | ||
| // with 16KB alignment on ELF. That raised the RW PT_LOAD's p_align to 0x4000 | ||
| // while the other segments stayed at 0x1000, and lld assigned the RW p_vaddr | ||
| // without keeping round_down(p_vaddr, 0x4000) clear of the previous segment's | ||
| // pages. The kernel ignores p_align at execve, so the plain binary ran, but a | ||
| // loader that honors p_align (UPX's decompression stub) mapped the RW segment | ||
| // over the tail of the R E segment and the embedded module source read back | ||
| // corrupted: `SyntaxError: Invalid character: '\0'`. | ||
| // | ||
| // These tests assert the strict-p_align non-overlap invariant on the bun | ||
| // binary itself and on a `--compile` output (the file UPX processes): | ||
| // for each PT_LOAD pair sorted by vaddr, | ||
| // round_down(next.p_vaddr, next.align) >= round_up(prev.p_vaddr + prev.p_memsz, prev.align) | ||
| // where align = max(p_align, page size). | ||
| // | ||
| // https://github.com/oven-sh/bun/issues/40752 | ||
|
|
||
| import { expect, test } from "bun:test"; | ||
| import type { Elf64ProgramHeader } from "harness"; | ||
| import { bunEnv, bunExe, isFreeBSD, isLinux, readElf64ProgramHeaders, tempDir } from "harness"; | ||
| import { join } from "node:path"; | ||
|
|
||
| type LoadSegment = Pick<Elf64ProgramHeader, "vaddr" | "memsz" | "align">; | ||
|
|
||
| /** PT_LOAD program headers of an ELF64 file. */ | ||
| function readLoadSegments(path: string): LoadSegment[] { | ||
| return readElf64ProgramHeaders(path).filter(ph => ph.type === 1 /* PT_LOAD */); | ||
| } | ||
|
|
||
| /** | ||
| * Mapped range of a PT_LOAD under strict p_align semantics: what a loader | ||
| * that honors p_align (like UPX's stub) maps for the segment. | ||
| */ | ||
| function strictRange({ vaddr, memsz, align }: LoadSegment): [bigint, bigint] { | ||
| const a = align > 0x1000n ? align : 0x1000n; // mapping granularity is at least a page | ||
| const start = vaddr & ~(a - 1n); | ||
| const end = (vaddr + memsz + a - 1n) & ~(a - 1n); | ||
| return [start, end]; | ||
| } | ||
|
|
||
| function expectNoOverlap(path: string) { | ||
| const loads = readLoadSegments(path).sort((a, b) => (a.vaddr < b.vaddr ? -1 : 1)); | ||
| expect(loads.length).toBeGreaterThan(1); | ||
| for (let i = 1; i < loads.length; i++) { | ||
| const [, prevEnd] = strictRange(loads[i - 1]); | ||
| const [nextStart] = strictRange(loads[i]); | ||
| if (nextStart < prevEnd) { | ||
| const fmt = (s: LoadSegment) => | ||
| `vaddr=0x${s.vaddr.toString(16)} memsz=0x${s.memsz.toString(16)} align=0x${s.align.toString(16)}`; | ||
| throw new Error( | ||
| `PT_LOAD segments overlap under strict p_align semantics by 0x${(prevEnd - nextStart).toString(16)} bytes:\n` + | ||
| ` ${fmt(loads[i - 1])}\n ${fmt(loads[i])}`, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| test.skipIf(!(isLinux || isFreeBSD))("bun binary has no PT_LOAD overlap under strict p_align", () => { | ||
| expectNoOverlap(bunExe()); | ||
| }); | ||
|
|
||
| test.skipIf(!(isLinux || isFreeBSD))( | ||
| "compiled executable has no PT_LOAD overlap under strict p_align", | ||
| async () => { | ||
| using dir = tempDir("elf-segment-layout", { | ||
| "index.ts": `console.log("hello from compiled");`, | ||
| }); | ||
| const cwd = String(dir); | ||
| const out = join(cwd, "app"); | ||
|
|
||
| await using build = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "--compile", join(cwd, "index.ts"), "--outfile", out], | ||
| env: bunEnv, | ||
| cwd, | ||
| stderr: "pipe", | ||
| stdout: "pipe", | ||
| }); | ||
| const [, buildErr, buildExit] = await Promise.all([build.stdout.text(), build.stderr.text(), build.exited]); | ||
| expect(buildErr).not.toContain("error:"); | ||
| expect(buildExit).toBe(0); | ||
|
|
||
| expectNoOverlap(out); | ||
|
|
||
| await using run = Bun.spawn({ cmd: [out], env: bunEnv, cwd, stderr: "pipe", stdout: "pipe" }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([run.stdout.text(), run.stderr.text(), run.exited]); | ||
| expect(stderr).toBe(""); | ||
| expect(stdout).toBe("hello from compiled\n"); | ||
| expect(exitCode).toBe(0); | ||
| }, | ||
| 180_000, | ||
| ); | ||
|
robobun marked this conversation as resolved.
|
||
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
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
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.
Uh oh!
There was an error while loading. Please reload this page.