Repository navigation
fix(relinker): write Linux PT_LOAD headers in ascending address order - #1559
Conversation
149c766 to
aaf33ad
Compare
|
Rebased on #1586, which changes the same lines of |
|
While the upstream runs wait for approval (#1631), the same Build workflow ran on the fork for this head (
|
|
The |
The header-block PT_LOAD has the highest address but was the first loadable entry in the table. Linux before 5.18 sizes the ET_DYN mapping from the first and last PT_LOAD entries in table order, so the output failed to exec with SIGSEGV. Write PT_PHDR and PT_INTERP first, sort the kept PT_LOAD entries by address, and place the extra and header blocks after them.
aaf33ad to
1352b9b
Compare
Dextroid17
left a comment
Dextroid17
left a comment
There was a problem hiding this comment.
Verified on Linux, with the artifact and the kernel source both in hand.
- base
d70b8998(upstream/main) + head1352b9b3, merged clean. - Arm A:
linux_program_header_orderPassed (0.11 s). - Arm B (only
ProgramHeaderLayoutBuilder.{cpp,hpp}reverted tomain, the PR's test kept): fails —AssertionError: ('single-load', [6, 1, 1, 1, 2, 3]), i.e.PT_LOADsecond andPT_INTERPlast, exactly the ordering the test exists to reject.
Your kernel citation checks out — I read it rather than trusting it. fs/binfmt_elf.c v5.15, line 402:
static unsigned long total_mapping_size(const struct elf_phdr *cmds, int nr)
{
int i, first_idx = -1, last_idx = -1;
for (i = 0; i < nr; i++)
if (cmds[i].p_type == PT_LOAD) {
last_idx = i;
if (first_idx == -1) first_idx = i;
}
if (first_idx == -1) return 0;
return cmds[last_idx].p_vaddr + cmds[last_idx].p_memsz -
ELF_PAGESTART(cmds[first_idx].p_vaddr);
}first_idx/last_idx are chosen by table index, so the result genuinely depends on table order — the mechanism is real, not inferred. And v5.18 replaces it with min/max over all PT_LOAD entries (line 404 of that version), so the second half of the description is right too.
I then computed that function's value on the produced file (/bin/true through each binary — 15 headers, but the first and last PT_LOAD in table order are what matter):
main |
PR | |
|---|---|---|
| type sequence | PHDR, LOAD, PHDR, INTERP, LOAD x4 ... |
PHDR, INTERP, PHDR, INTERP, LOAD x4 ... |
PT_INTERP before every load |
no | yes |
PT_LOAD p_vaddr ascending |
no (0x11000 first, 0xf938 last) |
yes (0x0 -> 0x11000) |
<5.18 total_mapping_size |
(0xf938 + 0xd5d) - page_start(0x11000) = -2411 -> as u64 0xfffffffffffff695 |
(0x11000 + 0x350) - 0 = 0x11350 |
| >=5.18 min/max span | 0x11350 |
0x11350 |
So the underflow you describe is reproducible by evaluation, not just by argument: on main the kernel's own quantity goes negative, which is the elf_map failure. With the change both kernel eras agree on 0x11350, which is the tidiest statement of why the reorder is correct.
What I could not do: reproduce the SIGSEGV itself. This box runs 7.2.9, i.e. the >=5.18 form, so the failure mode is out of reach here and I am not claiming otherwise — the arithmetic above is the substitute, and it is the same quantity the pre-5.18 path computes.
Two notes. Pre-existing, not introduced here: the output carries two PT_PHDR and two PT_INTERP entries (one synthetic, one kept from the input) in both arms — harmless since both interpreter entries name the same path, but since this PR is the one rearranging that table it may be worth a line in the body. And for sequencing: #2274 edits the same lines of ProgramHeaderLayoutBuilder.cpp (keeping < and kSyntheticProgramHeaderCount = 4), so whichever of the two lands second will need a rebase.
Written with AI assistance.
|
One more instrument, since the table order is the whole subject of this PR. REA's pwntools-elf adapter, run on both artefacts (digest-anchored; same pinned toolchain as the CI adapter — pwntools 4.15.0 / pyelftools 0.33 / unicorn 2.1.2):
That is a third independent read of the same structure (the other two being Written with AI assistance. |
|
@Dextroid17 thank you for the arm A/B run and for evaluating Checking your kernel reading led me to a gap in my description, now corrected. Before 5.18,
No kernel goes from working to failing, but the description now says exactly that instead of claiming every 5.15 or 5.10. On the duplicate On #2274: it also edits these lines of Written with AI assistance. |
|
The corrected description is the right granularity: it names which kernels move and, more importantly, states that no kernel goes from working to failing. Splitting 5.15.33-5.17 and 5.10.110+ (WSL2's 5.15.167 included) from older 5.15/5.10 is a much stronger claim than "every 5.15 or 5.10", because the older pair failing in both directions is a real bound rather than a hedge. I cannot add a kernel-version data point myself — this box runs 7.2 — so I am taking the backport versions on your reading. On the On #2274: agreed, rebase whichever lands first, and I will re-check the ordering claim on whichever head survives. Written with AI assistance. |
|
Independent Windows and Arch WSL validation of head
Additional title observation: Demon's Souls stopped on an unmapped global during initialization with the main-generated Linux executable, whose first PT_LOAD was the high-address header block. After reconverting with this PR, it passes that access, reports the direct-memory budget, and maps its initial heaps. It then faults later with guest allocations placed at native high Linux addresses; I am investigating #861 next. This is startup progress, not a playable-title claim. The runtime libraries for this title observation were main No full runtime suite was run for this PR; these counts cover the complete relinker-only suites. Validation performed with OpenAI Codex assistance. |
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
|
|
Checked: merges cleanly onto current Note: #2274 edits the same part of Merging. |
fix(relinker): write Linux PT_LOAD headers in ascending address order
4 participants
What
LinuxElfPatcherwrote the program header table as PT_PHDR, the header-block PT_LOAD, the kept original headers, the extra-block PT_LOAD, PT_DYNAMIC and PT_INTERP. The header block is placed after the extra block, so it has the highest address of all loadable segments while being the first PT_LOAD entry in the table. The ELF gABI requires loadable segment entries to appear in ascendingp_vaddrorder, and PT_PHDR and PT_INTERP to precede every loadable entry.Linux before 5.18 sizes the ET_DYN mapping as
last PT_LOAD p_vaddr + p_memsz - page_start(first PT_LOAD p_vaddr)in table order (total_mapping_size,fs/binfmt_elf.cv5.15). For a relinked executable that difference underflows,elf_mapfails after the point of no return, and the process dies with SIGSEGV before the dynamic linker runs. On 5.18 and later the kernel takes the minimum and maximum over all PT_LOAD entries, so the file loads, but the first mapping still reserves the range starting at the header block instead of the lowest segment.Kernel coverage, from
fs/binfmt_elf.cof each release:mainThe last row lacks the backport of the 5.18 AT_PHDR fix, so
AT_PHDRis computed as the first PT_LOAD'sp_vaddr - p_offsetpluse_phoff. That points into the header block only while the header block is the first PT_LOAD, which is exactly the order the table-ordertotal_mapping_sizerejects. Supporting those kernels would need the program header table moved to a segment whose file-to-address delta matches the lowest PT_LOAD; that is a separate change. No kernel runsmainand fails with this pull request.ProgramHeaderLayoutBuilder::WriteLayoutnow writes PT_PHDR, PT_INTERP, the kept original headers with their PT_LOAD entries sorted byp_vaddr(the Windows image builder already sorts them inWindowsLoadImage), the extra-block PT_LOAD, the header-block PT_LOAD and PT_DYNAMIC. The guest module writer already orders its segments this way.Rebased on
mainafter #1586 merged: itsPT_GNU_EH_FRAMEslot check and warning are kept, and a droppedPT_GNU_EH_FRAMEis removed from the kept headers before they are laid out.Tested
WSL2 Ubuntu 24.04 (kernel 5.15.167.4-microsoft-standard-WSL2, glibc 2.39, GCC 13.3), Linux build with
-DBUILD_TESTING=ON.linux_entry_argvfails there: the output exits with SIGSEGV (-11) instead of SIGTRAP. Reordering the PT_LOAD entries of that output by hand makes it run, which isolated the cause.main: 514/514 withctest --test-dir build -j8(23 GPU tests skipped by the WSL Vulkan device), includinglinux_entry_argv,linux_eh_frame_headerand the new test.linux_program_header_ordertest: a single-segment image, a three-segment image (text, rodata, data with bss) with a kept PT_OS_PROCPARAM entry, and the same image with its PT_LOAD entries out of order in the input. It checks PT_PHDR first, PT_INTERP before the first PT_LOAD, strictly ascending PT_LOAD addresses, the preserved non-load entry, that the 5.15 table-order span equals the address span, that PT_PHDR is covered by a PT_LOAD, and on Linux x86-64 it executes the outputs and expects SIGTRAP from the entry.No title tested.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)