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

Patryk27 · Patryk27 · commit da4b0b00ae73 · 2024-02-08T18:01:27.000+01:00
diff --git a/BUGS b/BUGS
diff --git a/src/patchelf.cc b/src/patchelf.cc
@@ -27,6 +27,7 @@
 #include <stdexcept>
 #include <string>
 #include <string_view>
+#include <type_traits>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
@@ -127,6 +128,14 @@ constexpr I ElfFile<ElfFileParamNames>::rdi(I i) const noexcept
     return r;
 }
 
+static void warn(const char * format, ...)
+{
+    va_list ap;
+    va_start(ap, format);
+    fprintf(stderr, "warning: ");
+    vfprintf(stderr, format, ap);
+    va_end(ap);
+}
 
 static void debug(const char * format, ...)
 {
@@ -554,8 +563,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);
@@ -801,49 +809,37 @@ void ElfFile<ElfFileParamNames>::rewriteSectionsLibrary()
        at the end of the file.  They're mapped into memory by a
        PT_LOAD segment located directly after the last virtual address
        page of other segments. */
-    Elf_Addr startPage = 0;
-    Elf_Addr firstPage = 0;
-    unsigned alignStartPage = getPageSize();
+    Elf_Addr firstAddr = 0;
+    Elf_Addr lastAddr = 0;
+    unsigned alignLastAddr = getPageSize();
+
     for (auto & phdr : phdrs) {
-        Elf_Addr thisPage = rdi(phdr.p_vaddr) + rdi(phdr.p_memsz);
-        if (thisPage > startPage) startPage = thisPage;
-        if (rdi(phdr.p_type) == PT_PHDR) firstPage = rdi(phdr.p_vaddr) - rdi(phdr.p_offset);
+        Elf_Addr thisAddr = rdi(phdr.p_vaddr) + rdi(phdr.p_memsz);
+        if (thisAddr > lastAddr) lastAddr = thisAddr;
+        if (rdi(phdr.p_type) == PT_PHDR) firstAddr = rdi(phdr.p_vaddr) - rdi(phdr.p_offset);
         unsigned thisAlign = rdi(phdr.p_align);
-        alignStartPage = std::max(alignStartPage, thisAlign);
+        alignLastAddr = std::max(alignLastAddr, thisAlign);
     }
 
-    startPage = roundUp(startPage, alignStartPage);
+    lastAddr = roundUp(lastAddr, alignLastAddr);
 
-    debug("last page is 0x%llx\n", (unsigned long long) startPage);
-    debug("first page is 0x%llx\n", (unsigned long long) firstPage);
+    debug("first address is 0x%llx\n", (unsigned long long) firstAddr);
+    debug("last address is 0x%llx\n", (unsigned long long) lastAddr);
 
     /* When normalizing note segments we will in the worst case be adding
        1 program header for each SHT_NOTE section. */
     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 `needNewSegment` */
+    Elf_Addr 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(), getPageSize());
@@ -852,37 +848,20 @@ 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;
     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 segment */
     if (!phdrs.empty() &&
         rdi(lastSeg.p_type) == PT_LOAD &&
         rdi(lastSeg.p_flags) == (PF_R | PF_W) &&
-        rdi(lastSeg.p_align) == alignStartPage) {
+        rdi(lastSeg.p_align) == alignLastAddr) {
         auto segEnd = roundUp(rdi(lastSeg.p_offset) + rdi(lastSeg.p_memsz), getPageSize());
+
         if (segEnd == startOffset) {
             auto newSz = startOffset + neededSpace - rdi(lastSeg.p_offset);
             wri(lastSeg.p_filesz, wri(lastSeg.p_memsz, newSz));
@@ -897,29 +876,39 @@ void ElfFile<ElfFileParamNames>::rewriteSectionsLibrary()
         Elf_Phdr & phdr = phdrs.at(rdi(hdr()->e_phnum) - 1);
         wri(phdr.p_type, PT_LOAD);
         wri(phdr.p_offset, startOffset);
-        wri(phdr.p_vaddr, wri(phdr.p_paddr, startPage));
+        wri(phdr.p_vaddr, wri(phdr.p_paddr, lastAddr));
         wri(phdr.p_filesz, wri(phdr.p_memsz, neededSpace));
         wri(phdr.p_flags, PF_R | PF_W);
-        wri(phdr.p_align, alignStartPage);
+        wri(phdr.p_align, alignLastAddr);
     }
 
     normalizeNoteSegments();
 
-
     /* Write out the replaced sections. */
-    Elf_Off curOff = startOffset;
+    Elf_Off currOffset = 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\n",
+        rdi(hdr()->e_phoff), currOffset);
 
-    writeReplacedSections(curOff, startPage, startOffset);
-    assert(curOff == startOffset + neededSpace);
+    wri(hdr()->e_phoff, currOffset);
+    currOffset += phtSize;
 
-    /* Write out the updated program and section headers */
-    rewriteHeaders(firstPage + rdi(hdr()->e_phoff));
+    // ---
+
+    debug("rewriting sht from offset 0x%x to offset 0x%x\n",
+        rdi(hdr()->e_phoff), currOffset);
+
+    wri(hdr()->e_shoff, currOffset);
+    currOffset += shtSize;
+
+    // ---
+
+    writeReplacedSections(currOffset, lastAddr, startOffset);
+    assert(currOffset == startOffset + neededSpace);
+
+    // ---
+
+    rewriteHeaders(lastAddr);
 }
 
 static bool noSort = false;
@@ -1033,31 +1022,33 @@ 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 currOffset = sizeof(Elf_Ehdr) + phdrs.size() * sizeof(Elf_Phdr);
 
-    /* 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);
+    /* 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) <= currOffset &&
+            rdi(phdr.p_offset) + rdi(phdr.p_filesz) > currOffset &&
+            rdi(phdr.p_filesz) < neededSpace)
+        {
+            wri(phdr.p_filesz, neededSpace);
+            wri(phdr.p_memsz, neededSpace);
+            break;
+        }
 
-    /* Write out the replaced sections. */
-    writeReplacedSections(curOff, firstPage, 0);
-    assert(curOff == neededSpace);
+    /* Clear out the free space. */
+    debug("clearing first %d bytes\n", startOffset - currOffset);
+    memset(fileContents->data() + currOffset, 0, startOffset - currOffset);
 
+    /* Write out the replaced sections. */
+    writeReplacedSections(currOffset, firstPage, 0);
+    assert(currOffset == neededSpace);
 
     rewriteHeaders(firstPage + rdi(hdr()->e_phoff));
 }
@@ -1152,7 +1143,7 @@ void ElfFile<ElfFileParamNames>::rewriteSections(bool force)
 
 
 template<ElfFileParams>
-void ElfFile<ElfFileParamNames>::rewriteHeaders(Elf_Addr phdrAddress)
+void ElfFile<ElfFileParamNames>::rewriteHeaders(Elf_Addr phdrAddr)
 {
     /* Rewrite the program header table. */
 
@@ -1161,7 +1152,7 @@ void ElfFile<ElfFileParamNames>::rewriteHeaders(Elf_Addr phdrAddress)
     for (auto & phdr : phdrs) {
         if (rdi(phdr.p_type) == PT_PHDR) {
             phdr.p_offset = hdr()->e_phoff;
-            wri(phdr.p_vaddr, wri(phdr.p_paddr, phdrAddress));
+            wri(phdr.p_vaddr, wri(phdr.p_paddr, phdrAddr));
             wri(phdr.p_filesz, wri(phdr.p_memsz, phdrs.size() * sizeof(Elf_Phdr)));
             break;
         }
@@ -1178,9 +1169,11 @@ void ElfFile<ElfFileParamNames>::rewriteHeaders(Elf_Addr phdrAddress)
     /* Rewrite the section header table.  For neatness, keep the
        sections sorted. */
     assert(rdi(hdr()->e_shnum) == shdrs.size());
+
     if (!noSort) {
         sortShdrs();
     }
+
     for (unsigned int i = 1; i < rdi(hdr()->e_shnum); ++i)
         * ((Elf_Shdr *) (fileContents->data() + rdi(hdr()->e_shoff)) + i) = shdrs.at(i);
 
@@ -1262,7 +1255,7 @@ void ElfFile<ElfFileParamNames>::rewriteHeaders(Elf_Addr phdrAddress)
                        is broken, and it's not our job to fix it; yet, we have
                        to find some location for dynamic loader to write the
                        debug pointer to; well, let's write it right here */
-                    fprintf(stderr, "warning: DT_MIPS_RLD_MAP_REL entry is present, but .rld_map section is not\n");
+                    warn("DT_MIPS_RLD_MAP_REL entry is present, but .rld_map section is not\n");
                     dyn->d_un.d_ptr = 0;
                 }
             }
@@ -1281,7 +1274,7 @@ void ElfFile<ElfFileParamNames>::rewriteHeaders(Elf_Addr phdrAddress)
             unsigned int shndx = rdi(sym->st_shndx);
             if (shndx != SHN_UNDEF && shndx < SHN_LORESERVE) {
                 if (shndx >= sectionsByOldIndex.size()) {
-                    fprintf(stderr, "warning: entry %d in symbol table refers to a non-existent section, skipping\n", shndx);
+                    warn("entry %d in symbol table refers to a non-existent section, skipping\n", shndx);
                     continue;
                 }
                 const std::string & section = sectionsByOldIndex.at(shndx);
diff --git 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" != "0x01040000" ]; then
   # Triggered if the virtual address is the incorrect endianness
-  echo "Unexpected virt addr, expected [0x00000034] got [$virtAddr]"
+  echo "Unexpected virt addr, expected [0x01040000] 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
@@ -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
@@ -12,7 +12,7 @@ if ! gzip --version >/dev/null; then
 fi
 
 if test "$(uname -i)" != x86_64 || test "$(uname)" != Linux; then
-    echo "skipping test: not supported on x86_64 Linux"
+    echo "skipping test: supported only on x86_64 Linux"
     exit 77
 fi
 
@@ -24,8 +24,10 @@ cd "${SCRATCH}"
 
 ldd "${EXEC_NAME}"
 
-${PATCHELF} --add-rpath lalalalalalalala --output modified1 "${EXEC_NAME}"
+${PATCHELF} --set-rpath $(tr -dc A-Za-z0-9 </dev/urandom | head -c 4096) --output modified1 "${EXEC_NAME}"
+${PATCHELF} --add-rpath $(tr -dc A-Za-z0-9 </dev/urandom | head -c 4096) modified1
+
 ldd modified1
 
-${PATCHELF}  --add-needed "libXcursor.so.1" --output modified2 modified1
+${PATCHELF} --add-needed "libXcursor.so.1" --output modified2 modified1
 ldd modified2
