From 702cb0444be3979fb5d6cdae14ce20e5a864107c Mon Sep 17 00:00:00 2001 From: Felipe Sateler Date: Mon, 29 Feb 2016 17:29:35 -0300 Subject: [PATCH 1/5] no-rpath-prebuild: force pagesize to 4096 on prebuilt binaries They all have that page size, and the host-detected one might be different. --- tests/no-rpath-prebuild.sh | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/no-rpath-prebuild.sh b/tests/no-rpath-prebuild.sh index d059ddac..aa27b7ad 100755 --- a/tests/no-rpath-prebuild.sh +++ b/tests/no-rpath-prebuild.sh @@ -1,6 +1,7 @@ #! /bin/sh -e set -x ARCH="$1" +PAGESIZE=4096 if [ -z "$ARCH" ]; then ARCH=$(basename $0 .sh | sed -e 's/.*-//') @@ -25,13 +26,13 @@ mkdir -p ${SCRATCH} cp $no_rpath_bin ${SCRATCH}/no-rpath -oldRPath=$(../src/patchelf --print-rpath ${SCRATCH}/no-rpath) +oldRPath=$(../src/patchelf --page-size ${PAGESIZE} --print-rpath ${SCRATCH}/no-rpath) if test -n "$oldRPath"; then exit 1; fi -../src/patchelf \ - --set-interpreter "$(../src/patchelf --print-interpreter ../src/patchelf)" \ +../src/patchelf --page-size ${PAGESIZE} \ + --set-interpreter "$(../src/patchelf --page-size ${PAGESIZE} --print-interpreter ../src/patchelf)" \ --set-rpath /foo:/bar:/xxxxxxxxxxxxxxx ${SCRATCH}/no-rpath -newRPath=$(../src/patchelf --print-rpath ${SCRATCH}/no-rpath) +newRPath=$(../src/patchelf --page-size ${PAGESIZE} --print-rpath ${SCRATCH}/no-rpath) if ! echo "$newRPath" | grep -q '/foo:/bar'; then echo "incomplete RPATH" exit 1 From 0f87d7f2d73c7542f78afba198900e4957688f38 Mon Sep 17 00:00:00 2001 From: Ezra Cooper Date: Thu, 21 Jun 2018 11:07:35 -0700 Subject: [PATCH 2/5] Fix issue #66 by ignoring the first section header when sorting, and not overwriting NOBITS entries. --- src/patchelf.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/patchelf.cc b/src/patchelf.cc index 136098fb..4bdf5a7b 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -366,7 +366,7 @@ void ElfFile::sortShdrs() /* Sort the sections by offset. */ CompShdr comp; comp.elfFile = this; - sort(shdrs.begin(), shdrs.end(), comp); + sort(shdrs.begin() + 1, shdrs.end(), comp); /* Restore the sh_link mappings. */ for (unsigned int i = 1; i < rdi(hdr->e_shnum); ++i) @@ -517,7 +517,8 @@ void ElfFile::writeReplacedSections(Elf_Off & curOff, { string sectionName = i->first; Elf_Shdr & shdr = findSection(sectionName); - memset(contents + rdi(shdr.sh_offset), 'X', rdi(shdr.sh_size)); + if (shdr.sh_type != SHT_NOBITS) + memset(contents + rdi(shdr.sh_offset), 'X', rdi(shdr.sh_size)); } for (ReplacedSections::iterator i = replacedSections.begin(); From b07897f3334c6cb5143afe3d1d1fb40f335f021b Mon Sep 17 00:00:00 2001 From: Richard Purdie Date: Tue, 7 Mar 2017 21:08:34 +0000 Subject: [PATCH 3/5] Avoid inflating file sizes needlessly and allow binaries to be stripped 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 (https://github.com/NixOS/patchelf/issues/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 (cherry picked from commit c4deb5e9e1ce9c98a48e0d5bb37d87739b8cfee4) Signed-off-by: Claudio Matsuoka --- src/patchelf.cc | 77 +++++++++++++++++++++++++++---------------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/src/patchelf.cc b/src/patchelf.cc index 4bdf5a7b..98a4af2f 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -146,6 +146,8 @@ class ElfFile string & replaceSection(const SectionName & sectionName, unsigned int size); + bool haveReplacedSection(const SectionName & sectionName); + void writeReplacedSections(Elf_Off & curOff, Elf_Addr startAddr, Elf_Off startOffset); @@ -483,6 +485,15 @@ unsigned int ElfFile::findSection3(const SectionName & sectio return 0; } +template +bool ElfFile::haveReplacedSection(const SectionName & sectionName) +{ + ReplacedSections::iterator i = replacedSections.find(sectionName); + + if (i != replacedSections.end()) + return true; + return false; +} template string & ElfFile::replaceSection(const SectionName & sectionName, @@ -582,52 +593,51 @@ void ElfFile::rewriteSectionsLibrary() debug("last page is 0x%llx\n", (unsigned long long) startPage); + /* Because we're adding a new section header, we're necessarily increasing + the size of the program header table. This can cause the first section + to overlap the program header table in memory; we need to shift the first + few segments to someplace else. */ + /* Some sections may already be replaced so account for that */ + unsigned int i = 1; + Elf_Addr pht_size = sizeof(Elf_Ehdr) + (phdrs.size() + 1)*sizeof(Elf_Phdr); + while( shdrs[i].sh_addr <= pht_size && i < rdi(hdr->e_shnum) ) { + if (not haveReplacedSection(getSectionName(shdrs[i]))) + replaceSection(getSectionName(shdrs[i]), shdrs[i].sh_size); + i++; + } - /* Compute the total space needed for the replaced sections and - the program headers. */ - off_t neededSpace = (phdrs.size() + 1) * sizeof(Elf_Phdr); - for (ReplacedSections::iterator i = replacedSections.begin(); - i != replacedSections.end(); ++i) - neededSpace += roundUp(i->second.size(), sectionAlignment); + /* Compute the total space needed for the replaced sections */ + off_t neededSpace = 0; + for (auto & i : replacedSections) + neededSpace += roundUp(i.second.size(), sectionAlignment); debug("needed space is %d\n", neededSpace); - size_t startOffset = roundUp(fileSize, getPageSize()); growFile(startOffset + neededSpace); - /* Even though this file is of type ET_DYN, it could actually be an executable. For instance, Gold produces executables marked - ET_DYN. In that case we can still hit the kernel bug that - necessitated rewriteSectionsExecutable(). However, such - executables also tend to start at virtual address 0, so + ET_DYN as does LD when linking with pie. If we move PT_PHDR, it + has to stay in the first PT_LOAD segment or any subsequent ones + if they're continuous in memory due to linux kernel constraints + (see BUGS). Since the end of the file would be after bss, we can't + move PHDR there, we therefore choose to leave PT_PHDR where it is but + move enough following sections such that we can add the extra PT_LOAD + section to it. This PT_LOAD segment ensures the sections at the end of + the file are mapped into memory for ld.so to process. + We can't use the approach in rewriteSectionsExecutable() + since DYN executables tend to start at virtual address 0, so rewriteSectionsExecutable() won't work because it doesn't have - any virtual address space to grow downwards into. As a - workaround, make sure that the virtual address of our new - PT_LOAD segment relative to the first PT_LOAD segment is equal - to its offset; otherwise we hit the kernel bug. This may - require creating a hole in the executable. The bigger the size - of the uninitialised data segment, the bigger the hole. */ + any virtual address space to grow downwards into. */ if (isExecutable) { if (startOffset >= startPage) { debug("shifting new PT_LOAD segment by %d bytes to work around a Linux kernel bug\n", startOffset - startPage); - } else { - size_t hole = startPage - startOffset; - /* Print a warning, because the hole could be very big. */ - fprintf(stderr, "warning: working around a Linux kernel bug by creating a hole of %zu bytes in ‘%s’\n", hole, fileName.c_str()); - assert(hole % getPageSize() == 0); - /* !!! We could create an actual hole in the file here, - but it's probably not worth the effort. */ - growFile(fileSize + hole); - startOffset += hole; } startPage = startOffset; } - - /* Add a segment that maps the replaced sections and program - headers into memory. */ + /* Add a segment that maps the replaced sections into memory. */ phdrs.resize(rdi(hdr->e_phnum) + 1); wri(hdr->e_phnum, rdi(hdr->e_phnum) + 1); Elf_Phdr & phdr = phdrs[rdi(hdr->e_phnum) - 1]; @@ -640,15 +650,12 @@ void ElfFile::rewriteSectionsLibrary() /* Write out the replaced sections. */ - Elf_Off curOff = startOffset + phdrs.size() * sizeof(Elf_Phdr); + Elf_Off curOff = startOffset; writeReplacedSections(curOff, startPage, startOffset); assert(curOff == startOffset + neededSpace); - - /* Move the program header to the start of the new area. */ - wri(hdr->e_phoff, startOffset); - - rewriteHeaders(startPage); + /* Write out the updated program and section headers */ + rewriteHeaders(hdr->e_phoff); } From 4a4e8d0045f70af62cc9f1660c45d37b0b9574dc Mon Sep 17 00:00:00 2001 From: Claudio Matsuoka Date: Thu, 28 Mar 2019 17:48:20 -0300 Subject: [PATCH 4/5] build: set c++ compiler standard to c++11 Signed-off-by: Claudio Matsuoka --- src/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Makefile.am b/src/Makefile.am index 65d0fa89..4e2a7f0a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,4 +1,4 @@ -AM_CXXFLAGS = -Wall +AM_CXXFLAGS = -Wall -std=c++11 bin_PROGRAMS = patchelf From 27c0ba1cc261f7990484bdb8552e63d8ccfb9308 Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Fri, 21 Jul 2017 12:33:53 +0300 Subject: [PATCH 5/5] fix adjusting startPage 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 --- src/patchelf.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/patchelf.cc b/src/patchelf.cc index 98a4af2f..dbe5ea70 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -630,10 +630,8 @@ void ElfFile::rewriteSectionsLibrary() since DYN executables tend to start at virtual address 0, so rewriteSectionsExecutable() won't work because it doesn't have any virtual address space to grow downwards into. */ - if (isExecutable) { - if (startOffset >= startPage) { - debug("shifting new PT_LOAD segment by %d bytes to work around a Linux kernel bug\n", startOffset - startPage); - } + if (isExecutable && startOffset > startPage) { + debug("shifting new PT_LOAD segment by %d bytes to work around a Linux kernel bug\n", startOffset - startPage); startPage = startOffset; }