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..ae9ac0bd 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); @@ -823,28 +822,16 @@ 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 */ - 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)); - i++; - } - bool moveHeaderTableToTheEnd = rdi(hdr()->e_shoff) < pht_size; + /* 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); + off_t neededSpace = phtSize + shtSize; - /* Compute the total space needed for the replaced sections */ - off_t neededSpace = 0; 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 +840,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; + Elf_Addr phdrAddress = 0; auto& lastSeg = phdrs.back(); - /* Try to extend the last segment to include replaced sections */ + + /* 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; + + phdrAddress = rdi(lastSeg.p_vaddr) + newSz - neededSpace; } } - if (needNewSegment) { + if (phdrAddress == 0) { + debug("allocating new PT_LOAD segment for pht\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 +877,37 @@ void ElfFile::rewriteSectionsLibrary() wri(phdr.p_flags, PF_R | PF_W); wri(phdr.p_align, alignStartPage); assert(startPage % alignStartPage == startOffset % alignStartPage); + + phdrAddress = 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; - } + 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)); + rewriteHeaders(phdrAddress); } static bool noSort = false; @@ -1035,32 +1021,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/tests/grow-file.sh b/tests/grow-file.sh index ec5957d0..c5f5695f 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 +echo "$(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/no-rpath-pie-powerpc.sh b/tests/no-rpath-pie-powerpc.sh index c2b50fe5..099aac81 100755 --- a/tests/no-rpath-pie-powerpc.sh +++ b/tests/no-rpath-pie-powerpc.sh @@ -37,9 +37,9 @@ if [ "$(echo "$readelfData" | grep -c "PHDR")" != 1 ]; then fi virtAddr=$(echo "$readelfData" | grep "PHDR" | awk '{print $3}') -if [ "$virtAddr" != "0x00000034" ]; then +if [ "$virtAddr" != "0x01030000" ] && [ "$virtAddr" != "0x01040000" ]; then # Triggered if the virtual address is the incorrect endianness - echo "Unexpected virt addr, expected [0x00000034] got [$virtAddr]" + echo "Unexpected virt addr, got [$virtAddr]" exit 1 fi @@ -52,4 +52,3 @@ echo "$readelfData" | grep "LOAD" | while read -r line ; do exit 1 fi done - 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