Avoid overlapping program header table with section header table #457

This patch checks if the section header table is placed right after the
program header table such that it would overlap when we add a new entry
in the program header table.
If that is the case, move the section header table to the end of the file.

Moreover, there is no need to add a new PT_LOAD segment everytime.
Check if the last segment is already a PT_LOAD with the same characteristics
and adjacent. Extend it in this case.

3 files changed, 78 insertions, 10 deletions

diff --git a/src/patchelf.cc b/src/patchelf.cc
index 2bb84eb..af00172 100644
--- a/src/patchelf.cc
+++ b/src/patchelf.cc
@@ -755,11 +755,16 @@ void ElfFile<ElfFileParamNames>::rewriteSectionsLibrary()
             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 */
     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());
@@ -789,24 +794,47 @@ void ElfFile<ElfFileParamNames>::rewriteSectionsLibrary()
         startPage = startOffset;
     }
 
-    /* Add a segment that maps the replaced sections into memory. */
     wri(hdr()->e_phoff, sizeof(Elf_Ehdr));
-    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);
-    wri(phdr.p_type, PT_LOAD);
-    wri(phdr.p_offset, startOffset);
-    wri(phdr.p_vaddr, wri(phdr.p_paddr, startPage));
-    wri(phdr.p_filesz, wri(phdr.p_memsz, neededSpace));
-    wri(phdr.p_flags, PF_R | PF_W);
-    wri(phdr.p_align, getPageSize());
 
+    bool needNewSegment = true;
+    auto& lastSeg = phdrs.back();
+    /* Try to extend the last segment to include replaced sections */
+    if (rdi(lastSeg.p_type) == PT_LOAD &&
+        rdi(lastSeg.p_flags) == (PF_R | PF_W) &&
+        rdi(lastSeg.p_align) == getPageSize()) {
+        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));
+            needNewSegment = false;
+        }
+    }
+
+    if (needNewSegment) {
+        /* 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.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_filesz, wri(phdr.p_memsz, neededSpace));
+        wri(phdr.p_flags, PF_R | PF_W);
+        wri(phdr.p_align, getPageSize());
+    }
 
     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;
+    }
+
     writeReplacedSections(curOff, startPage, startOffset);
     assert(curOff == startOffset + neededSpace);
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 219f238..a81a1f6 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -43,6 +43,7 @@ src_TESTS = \
   replace-needed.sh \
   replace-add-needed.sh \
   add-debug-tag.sh \
+  repeated-updates.sh \
   empty-note.sh
 
 build_TESTS = \
diff --git a/tests/repeated-updates.sh b/tests/repeated-updates.sh
new file mode 100755
index 0000000..0a57574
--- /dev/null
+++ b/tests/repeated-updates.sh
@@ -0,0 +1,39 @@
+#! /bin/sh -e
+
+SCRATCH=scratch/$(basename $0 .sh)
+PATCHELF=$(readlink -f "../src/patchelf")
+
+rm -rf ${SCRATCH}
+mkdir -p ${SCRATCH}
+
+cp simple ${SCRATCH}/
+cp libfoo.so ${SCRATCH}/
+cp libbar.so ${SCRATCH}/
+
+cd ${SCRATCH}
+
+${PATCHELF} --add-needed ./libbar.so simple
+
+###############################################################################
+# Test that repeatedly modifying a string inside a shared library does not
+# corrupt it due to the addition of multiple PT_LOAD entries
+###############################################################################
+load_segments_before=$(readelf -W -l libbar.so | grep -c LOAD)
+
+for i in $(seq 1 100)
+do
+    ${PATCHELF} --set-soname ./libbar.so libbar.so
+    ${PATCHELF} --set-soname libbar.so libbar.so
+    ./simple || exit 1
+done
+
+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} > ${load_segments_before} + 2 ))
+then
+    exit 1
+fi