Local patches for patchelf 0.9 #2
Merged
Conversation
This file contains 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
They all have that page size, and the host-detected one might be different.
… and not overwriting NOBITS entries.
The current approach to changing sections in ET_DYN executables is to move the INTERP section to the end of the file. +This means changing PT_PHDR to add an extra PT_LOAD section so that the new section is mmaped into memory by the elf loader in the kernel. In order to extend PHDR, this means moving it to the end of the file. Its documented in BUGS there is a kernel 'bug' which means that if you have holes in memory between the base load address and the PT_LOAD segment that contains PHDR, it will pass an incorrect PHDR address to ld.so and fail to load the binary, segfaulting. To avoid this, the code currently inserts space into the binary to ensure that when loaded into memory there are no holes between the PT_LOAD sections. This inflates the binaries by many MBs in some cases. Whilst we could make them sparse, there is a second issue which is that strip can fail to process these binaries: $ strip fixincl Not enough room for program headers, try linking with -N [.note.ABI-tag]: Bad value This turns out to be due to libbfd not liking the relocated PHDR section either (NixOS#10). Instead this patch implements a different approach, leaving PHDR where it is but extending it in place to allow addition of a new PT_LOAD section. This overwrites sections in the binary but those get moved to the end of the file in the new PT_LOAD section. This is based on patches linked from the above github issue, however whilst the idea was good, the implementation wasn't correct and they've been rewritten here. Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> (cherry picked from commit c4deb5e) Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>
startPage is adjusted unconditionally for all executables. This results in incorrect addresses assigned to INTERP and LOAD program headers, which breaks patched executable. Adjusting startPage variable only when startOffset > startPage should fix this. This change is related to the issue NixOS#10 Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
sergiusens
pushed a commit
that referenced
this pull request
Jan 23, 2023
Otherwise, patchelf segfaults when it encounters DT_NEEDED in the read
garbage. Corresponding backtrace is:
#0 0x00007ffff7c275f7 in __strlen_avx2 () from /nix/store/cvr0kjg2q7z2wwhjblx6c73rv422k8cm-glibc-2.33-47/lib/libc.so.6
#1 0x00007ffff7f2d448 in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) () from /nix/store/lg104nh0szci8slz5z6494m457jm5y3p-gcc-10.3.0-lib/lib/libstdc++.so.6
#2 0x000000000040fe0f in ElfFile<Elf64_Ehdr, Elf64_Phdr, Elf64_Shdr, unsigned long, unsigned long, Elf64_Dyn, Elf64_Sym, Elf64_Verneed, unsigned short>::modifyRPath (this=0x7fffffffbaa0,
op=ElfFile<Elf64_Ehdr, Elf64_Phdr, Elf64_Shdr, unsigned long, unsigned long, Elf64_Dyn, Elf64_Sym, Elf64_Verneed, unsigned short>::rpPrint, allowedRpathPrefixes=std::vector of length 0, capacity 0, newRPath="") at patchelf.cc:1351
#3 0x00000000004061c3 in patchElf2<ElfFile<Elf64_Ehdr, Elf64_Phdr, Elf64_Shdr, unsigned long, unsigned long, Elf64_Dyn, Elf64_Sym, Elf64_Verneed, unsigned short> > (elfFile=..., fileContents=std::shared_ptr<std::vector<unsigned char, std::allocator<unsigned char> >> (use count 3, weak count 0) = {...},
fileName="libsystemd.debug") at patchelf.cc:1805
NixOS#4 0x0000000000404774 in patchElf () at patchelf.cc:1848
NixOS#5 0x000000000040551c in mainWrapped (argc=3, argv=0x7fffffffc148) at patchelf.cc:2003
NixOS#6 0x0000000000405913 in main (argc=3, argv=0x7fffffffc148) at patchelf.cc:2011
NOBIT sections are included in the section headers table but occupy no
actual space in the file. .dynamic sections of this types are created,
for example, by `strip --only-keep-debug`.
I'm not sure whether calling error() would be more appropriate than
ignoring this situation with debug/return. I chose ignoring it,
because error() caused autoPatchelfHook to fail with my package. Also
the rest of modifyRPath method simply calls debug/return in similar
situations.
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.
Includes the following patches:
stripNixOS/patchelf#10)cannot find sectionNixOS/patchelf#66 by ignoring the first section header when sorting