rewriteSectionsExecutable: fall back to tail-append on vaddr underrun#637
Open
JamieMagee wants to merge 1 commit intoNixOS:masterfrom
Open
rewriteSectionsExecutable: fall back to tail-append on vaddr underrun#637JamieMagee wants to merge 1 commit intoNixOS:masterfrom
JamieMagee wants to merge 1 commit intoNixOS:masterfrom
Conversation
When patching a non-PIE (ET_EXEC) binary in a way that grows the program header table, the existing code shifts every LOAD segment's p_vaddr down by one or more pages. This produces LOAD segments below the input's lowest p_vaddr. Many systems enforce vm.mmap_min_addr equal to the binary's lowest load address: riscv64-linux defaults non-PIE executables to TEXT_START_ADDR=0x10000 and Ubuntu sets vm.mmap_min_addr=0x10000; x86_64 PDEs link at 0x400000 and hardened distros raise the sysctl to match; the RISE RISC-V Runners GitHub Actions images default to 0x10000. On these systems the kernel rejects the fixed-address mmap at exec time and the patched binary fails to load. Fix it by detecting when the shift-down would underrun the input's lowest LOAD p_vaddr, and falling back to the same strategy rewriteSectionsLibrary already uses for shared objects: append the rewritten sections (and the relocated PHT) in a fresh PT_LOAD segment at the end of the file, leaving every existing LOAD's vaddr unchanged. Also force the tail-append path on inputs whose program header table has already been relocated (e_phoff != sizeof(Elf_Ehdr)), since the front-of-file rewrite can no longer produce a self- consistent layout once a previous patchelf invocation has moved the PHT to the tail. The tail-append logic was already present in rewriteSectionsLibrary; factor it out as appendReplacedSectionsAtEnd(relocatePht, originalPhdrVaddr) so both code paths share it. Adds a regression test (no-vaddr-underrun.sh) that asserts the post-patch lowest LOAD p_vaddr is not below the pre-patch value on a non-PIE x86_64 binary, and runs the patched binary to confirm the dynamic linker accepts the result. Verified manually: patching the upstream nixpkgs riscv64 bootstrap-tools gcc with a long rpath now preserves the 0x10000 load address (master shifts it down to 0xe000). Closes NixOS#622
13 tasks
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
1 participant
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.
Patching a non-PIE binary that grows the program header table currently shifts every LOAD segment's
p_vaddrdown. The new lowestp_vaddrends up below the input's, which the kernel rejects at exec time on systems wherevm.mmap_min_addrmatches the binary's load base. That default bites riscv64-linux/Ubuntu (both 0x10000), hardened x86_64 distros that raise the sysctl to 0x400000, and the RISE RISC-V Runners on GitHub Actions.When the shift-down would underrun, this PR instead appends the rewritten sections (and the relocated PHT) at the end of the file in a new PT_LOAD, the same thing
rewriteSectionsLibrarydoes for shared objects. Existing LOAD vaddrs are untouched.Inputs whose PHT has already been moved out of position (
e_phoff != sizeof(Elf_Ehdr)) take the same path. Once a previous patchelf run has relocated the PHT, the front-of-file rewrite can't produce a consistent layout.The tail-append logic was inline in
rewriteSectionsLibrary. pulled it intoappendReplacedSectionsAtEnd(relocatePht, originalPhdrVaddr)so both callers share it.Regression test:
tests/no-vaddr-underrun.shpatches a non-PIE x86_64 binary with a long rpath, asserts the lowest LOADp_vaddrdidn't drop, then actually runs the patched binary.Also tested on the upstream nixpkgs riscv64 bootstrap-tools gcc. Master shifts the load base from 0x10000 to 0xe000 (which is what broke loading on the RISE runners). With this PR it stays at 0x10000.
Closes #622