From 5645179b78c5ca718091691a9f2cf58a88223c61 Mon Sep 17 00:00:00 2001 From: Patryk Wychowaniec Date: Sat, 28 Dec 2024 17:00:44 +0100 Subject: [PATCH] Allocate PHT & SHT at the end of the *.elf file --- BUGS | 6 -- src/patchelf.cc | 175 ++++++++++++++++++++++------------- src/patchelf.h | 3 +- tests/grow-file.sh | 5 +- tests/repeated-updates.sh | 2 +- tests/short-first-segment.sh | 7 +- 6 files changed, 122 insertions(+), 76 deletions(-) delete mode 100644 BUGS diff --git a/BUGS b/BUGS deleted file mode 100644 index 9e8913d2..00000000 --- a/BUGS +++ /dev/null @@ -1,6 +0,0 @@ -Bug in Linux kernel, in fs/binfmt_elf.c: - - NEW_AUX_ENT( 3, AT_PHDR, load_addr + exec->e_phoff); - -This is wrong since the actual virtual address of the program headers -may be somewhere else. diff --git a/src/patchelf.cc b/src/patchelf.cc index 35a5dc1c..cd14116f 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -555,8 +555,7 @@ void ElfFile::shiftFile(unsigned int extraPages, size_t start assert(splitIndex != -1); - /* Add a segment that maps the new program/section headers and - PT_INTERP segment into memory. Otherwise glibc will choke. */ + /* Add another PT_LOAD segment loading the data we've split above. */ phdrs.resize(rdi(hdr()->e_phnum) + 1); wri(hdr()->e_phnum, rdi(hdr()->e_phnum) + 1); Elf_Phdr & phdr = phdrs.at(rdi(hdr()->e_phnum) - 1); @@ -636,11 +635,19 @@ unsigned int ElfFile::getSectionIndex(const SectionName & sec } template -bool ElfFile::haveReplacedSection(const SectionName & sectionName) const +bool ElfFile::hasReplacedSection(const SectionName & sectionName) const { return replacedSections.count(sectionName); } +template +bool ElfFile::canReplaceSection(const SectionName & sectionName) const +{ + auto shdr = findSectionHeader(sectionName); + + return sectionName == ".interp" || rdi(shdr.sh_type) != SHT_PROGBITS; +} + template std::string & ElfFile::replaceSection(const SectionName & sectionName, unsigned int size) @@ -823,28 +830,60 @@ void ElfFile::rewriteSectionsLibrary() unsigned int num_notes = std::count_if(shdrs.begin(), shdrs.end(), [this](Elf_Shdr shdr) { return rdi(shdr.sh_type) == SHT_NOTE; }); - /* 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 */ + /* Compute the total space needed for the replaced sections, pessimistically + assuming we're going to need one more to account for new PT_LOAD covering + relocated PHDR */ + off_t phtSize = roundUp((phdrs.size() + num_notes + 1) * sizeof(Elf_Phdr), sectionAlignment); + off_t shtSize = roundUp(rdi(hdr()->e_shnum) * rdi(hdr()->e_shentsize), sectionAlignment); + + /* Check if we can keep PHT at the beginning of the file. + + We'd like to do that, because it preverves compatibility with older + kernels¹ - but if the PHT has grown too much, we have no other option but + to move it at the end of the file. + + ¹ older kernels had a bug that prevented them from loading ELFs with + PHDRs not located at the beginning of the file; it was fixed over + 0da1d5002745cdc721bc018b582a8a9704d56c42 (2022-03-02) */ + bool relocatePht = false; unsigned int i = 1; - Elf_Addr pht_size = sizeof(Elf_Ehdr) + (phdrs.size() + num_notes + 1)*sizeof(Elf_Phdr); - while( i < rdi(hdr()->e_shnum) && rdi(shdrs.at(i).sh_offset) <= pht_size ) { - if (not haveReplacedSection(getSectionName(shdrs.at(i)))) - replaceSection(getSectionName(shdrs.at(i)), rdi(shdrs.at(i).sh_size)); + + while (i < rdi(hdr()->e_shnum) && ((off_t) rdi(shdrs.at(i).sh_offset)) <= phtSize) { + const auto & sectionName = getSectionName(shdrs.at(i)); + + if (!hasReplacedSection(sectionName) && !canReplaceSection(sectionName)) { + relocatePht = true; + break; + } + i++; } - bool moveHeaderTableToTheEnd = rdi(hdr()->e_shoff) < pht_size; - /* Compute the total space needed for the replaced sections */ - off_t neededSpace = 0; + if (!relocatePht) { + unsigned int i = 1; + + while (i < rdi(hdr()->e_shnum) && ((off_t) rdi(shdrs.at(i).sh_offset)) <= phtSize) { + const auto & sectionName = getSectionName(shdrs.at(i)); + const auto sectionSize = rdi(shdrs.at(i).sh_size); + + if (!hasReplacedSection(sectionName)) { + replaceSection(sectionName, sectionSize); + } + + i++; + } + } + + /* Calculate how much space we'll need. */ + off_t neededSpace = shtSize; + + if (relocatePht) { + neededSpace += phtSize; + } + for (auto & s : replacedSections) neededSpace += roundUp(s.second.size(), sectionAlignment); - off_t headerTableSpace = roundUp(rdi(hdr()->e_shnum) * rdi(hdr()->e_shentsize), sectionAlignment); - if (moveHeaderTableToTheEnd) - neededSpace += headerTableSpace; debug("needed space is %d\n", neededSpace); Elf_Off startOffset = roundUp(fileContents->size(), alignStartPage); @@ -853,45 +892,32 @@ void ElfFile::rewriteSectionsLibrary() // section segment is strictly smaller than the file (and not same size). // By making it one byte larger, we don't break readelf. off_t binutilsQuirkPadding = 1; - fileContents->resize(startOffset + neededSpace + binutilsQuirkPadding, 0); - - /* Even though this file is of type ET_DYN, it could actually be - an executable. For instance, Gold produces executables marked - 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. */ - 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; - } - wri(hdr()->e_phoff, sizeof(Elf_Ehdr)); + fileContents->resize(startOffset + neededSpace + binutilsQuirkPadding, 0); - bool needNewSegment = true; auto& lastSeg = phdrs.back(); - /* Try to extend the last segment to include replaced sections */ + Elf_Addr lastSegAddr = 0; + + /* As an optimization, instead of allocating a new PT_LOAD segment, try + expanding the last one */ if (!phdrs.empty() && rdi(lastSeg.p_type) == PT_LOAD && rdi(lastSeg.p_flags) == (PF_R | PF_W) && rdi(lastSeg.p_align) == alignStartPage) { auto segEnd = roundUp(rdi(lastSeg.p_offset) + rdi(lastSeg.p_memsz), alignStartPage); + if (segEnd == startOffset) { auto newSz = startOffset + neededSpace - rdi(lastSeg.p_offset); + wri(lastSeg.p_filesz, wri(lastSeg.p_memsz, newSz)); - needNewSegment = false; + + lastSegAddr = rdi(lastSeg.p_vaddr) + newSz - neededSpace; } } - if (needNewSegment) { + if (lastSegAddr == 0) { + debug("allocating new PT_LOAD segment\n"); + /* 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); @@ -903,25 +929,43 @@ void ElfFile::rewriteSectionsLibrary() wri(phdr.p_flags, PF_R | PF_W); wri(phdr.p_align, alignStartPage); assert(startPage % alignStartPage == startOffset % alignStartPage); + + lastSegAddr = startPage; } normalizeNoteSegments(); - /* Write out the replaced sections. */ Elf_Off curOff = startOffset; - if (moveHeaderTableToTheEnd) { - debug("Moving the shtable to offset %d\n", curOff); - wri(hdr()->e_shoff, curOff); - curOff += headerTableSpace; + if (relocatePht) { + debug("rewriting pht from offset 0x%x to offset 0x%x (size %d)\n", + rdi(hdr()->e_phoff), curOff, phtSize); + + wri(hdr()->e_phoff, curOff); + curOff += phtSize; } + // --- + + debug("rewriting sht from offset 0x%x to offset 0x%x (size %d)\n", + rdi(hdr()->e_shoff), curOff, shtSize); + + wri(hdr()->e_shoff, curOff); + curOff += shtSize; + + // --- + + /* Write out the replaced sections. */ writeReplacedSections(curOff, startPage, startOffset); assert(curOff == startOffset + neededSpace); /* Write out the updated program and section headers */ - rewriteHeaders(firstPage + rdi(hdr()->e_phoff)); + if (relocatePht) { + rewriteHeaders(lastSegAddr); + } else { + rewriteHeaders(firstPage + rdi(hdr()->e_phoff)); + } } static bool noSort = false; @@ -1035,32 +1079,35 @@ void ElfFile::rewriteSectionsExecutable() firstPage -= neededPages * getPageSize(); startOffset += neededPages * getPageSize(); - } else { - Elf_Off rewrittenSectionsOffset = sizeof(Elf_Ehdr) + phdrs.size() * sizeof(Elf_Phdr); - for (auto& phdr : phdrs) - if (rdi(phdr.p_type) == PT_LOAD && - rdi(phdr.p_offset) <= rewrittenSectionsOffset && - rdi(phdr.p_offset) + rdi(phdr.p_filesz) > rewrittenSectionsOffset && - rdi(phdr.p_filesz) < neededSpace) - { - wri(phdr.p_filesz, neededSpace); - wri(phdr.p_memsz, neededSpace); - break; - } } + Elf_Off curOff = sizeof(Elf_Ehdr) + phdrs.size() * sizeof(Elf_Phdr); + + /* Ensure PHDR is covered by a LOAD segment. + + Because PHDR is supposed to have been covered by such section before, in + here we assume that we don't have to create any new section, but rather + extend the existing one. */ + for (auto& phdr : phdrs) + if (rdi(phdr.p_type) == PT_LOAD && + rdi(phdr.p_offset) <= curOff && + rdi(phdr.p_offset) + rdi(phdr.p_filesz) > curOff && + rdi(phdr.p_filesz) < neededSpace) + { + wri(phdr.p_filesz, neededSpace); + wri(phdr.p_memsz, neededSpace); + break; + } /* Clear out the free space. */ - Elf_Off curOff = sizeof(Elf_Ehdr) + phdrs.size() * sizeof(Elf_Phdr); debug("clearing first %d bytes\n", startOffset - curOff); memset(fileContents->data() + curOff, 0, startOffset - curOff); - /* Write out the replaced sections. */ writeReplacedSections(curOff, firstPage, 0); assert(curOff == neededSpace); - + /* Write out the updated program and section headers */ rewriteHeaders(firstPage + rdi(hdr()->e_phoff)); } diff --git a/src/patchelf.h b/src/patchelf.h index 4e229d67..5ed1b585 100644 --- a/src/patchelf.h +++ b/src/patchelf.h @@ -120,7 +120,8 @@ class ElfFile std::string & replaceSection(const SectionName & sectionName, unsigned int size); - [[nodiscard]] bool haveReplacedSection(const SectionName & sectionName) const; + [[nodiscard]] bool hasReplacedSection(const SectionName & sectionName) const; + [[nodiscard]] bool canReplaceSection(const SectionName & sectionName) const; void writeReplacedSections(Elf_Off & curOff, Elf_Addr startAddr, Elf_Off startOffset); diff --git a/tests/grow-file.sh b/tests/grow-file.sh index ec5957d0..29bba590 100755 --- a/tests/grow-file.sh +++ b/tests/grow-file.sh @@ -7,10 +7,11 @@ mkdir -p "${SCRATCH}" cp simple-pie "${SCRATCH}/simple-pie" -# Add a 40MB rpath -tr -cd 'a-z0-9' < /dev/urandom | dd count=40 bs=1000000 > "${SCRATCH}/foo.bin" +# Add a large rpath +printf '=%.0s' $(seq 1 4096) > "${SCRATCH}/foo.bin" # Grow the file ../src/patchelf --add-rpath @"${SCRATCH}/foo.bin" "${SCRATCH}/simple-pie" + # Make sure we can still run it "${SCRATCH}/simple-pie" diff --git a/tests/repeated-updates.sh b/tests/repeated-updates.sh index da7715ab..ae99090c 100755 --- a/tests/repeated-updates.sh +++ b/tests/repeated-updates.sh @@ -34,7 +34,7 @@ load_segments_after=$(${READELF} -W -l libbar.so | grep -c LOAD) # To be even more strict, check that we don't add too many extra LOAD entries ############################################################################### echo "Segments before: ${load_segments_before} and after: ${load_segments_after}" -if [ "${load_segments_after}" -gt $((load_segments_before + 2)) ] +if [ "${load_segments_after}" -gt $((load_segments_before + 3)) ] then exit 1 fi diff --git a/tests/short-first-segment.sh b/tests/short-first-segment.sh index 7a11345f..d0918fb4 100755 --- a/tests/short-first-segment.sh +++ b/tests/short-first-segment.sh @@ -24,8 +24,11 @@ cd "${SCRATCH}" ldd "${EXEC_NAME}" -${PATCHELF} --add-rpath lalalalalalalala --output modified1 "${EXEC_NAME}" + +${PATCHELF} --set-rpath "$(printf '=%.0s' $(seq 1 4096))" --output modified1 "${EXEC_NAME}" +${PATCHELF} --add-rpath "$(printf '=%.0s' $(seq 1 4096))" modified1 + ldd modified1 -${PATCHELF} --add-needed "libXcursor.so.1" --output modified2 modified1 +${PATCHELF} --add-needed "libXcursor.so.1" --output modified2 modified1 ldd modified2