Allocate PHT & SHT at the end of the *.elf file

Author: Patryk27

BUGS

@@ -555,8 +555,7 @@ void ElfFile<ElfFileParamNames>::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<ElfFileParamNames>::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<ElfFileParamNames>::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<ElfFileParamNames>::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<ElfFileParamNames>::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));
 }
 

src/patchelf.cc

@@ -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"

tests/grow-file.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
-

tests/no-rpath-pie-powerpc.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

tests/repeated-updates.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