From d925e8c91d06a7099cf03b39ec481f5f43107506 Mon Sep 17 00:00:00 2001 From: Breno Rodrigues Guimaraes Date: Sat, 18 Feb 2023 20:56:04 -0300 Subject: [PATCH 1/5] 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. --- src/patchelf.cc | 48 +++++++++++++++++++++++++++++++-------- tests/Makefile.am | 1 + tests/repeated-updates.sh | 39 +++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 10 deletions(-) create mode 100755 tests/repeated-updates.sh diff --git a/src/patchelf.cc b/src/patchelf.cc index 2bb84eb7..af00172c 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -755,11 +755,16 @@ void ElfFile::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::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 219f238d..a81a1f6d 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 00000000..0a575742 --- /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 From 313c6115b7988f65d2823d37b330a91b39c8f415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Breno=20Rodrigues=20Guimar=C3=A3es?= Date: Sun, 19 Feb 2023 09:27:44 -0300 Subject: [PATCH 2/5] Update src/patchelf.cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörg Thalheim --- src/patchelf.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/patchelf.cc b/src/patchelf.cc index af00172c..f194cadd 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -799,7 +799,8 @@ void ElfFile::rewriteSectionsLibrary() bool needNewSegment = true; auto& lastSeg = phdrs.back(); /* Try to extend the last segment to include replaced sections */ - if (rdi(lastSeg.p_type) == PT_LOAD && + if (phdrs.size() && + 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()); From 2ccf3c385ebc904b3cf2fa115827c82dc019300b Mon Sep 17 00:00:00 2001 From: Breno Rodrigues Guimaraes Date: Mon, 20 Feb 2023 16:55:47 -0300 Subject: [PATCH 3/5] to trigger ci --- src/patchelf.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/patchelf.cc b/src/patchelf.cc index f194cadd..3e632e51 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -799,7 +799,7 @@ void ElfFile::rewriteSectionsLibrary() bool needNewSegment = true; auto& lastSeg = phdrs.back(); /* Try to extend the last segment to include replaced sections */ - if (phdrs.size() && + if (!phdrs.empty() && rdi(lastSeg.p_type) == PT_LOAD && rdi(lastSeg.p_flags) == (PF_R | PF_W) && rdi(lastSeg.p_align) == getPageSize()) { From afca68f86aa021c5cb991bb810db71eb6af89155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Breno=20Rodrigues=20Guimar=C3=A3es?= Date: Thu, 23 Feb 2023 17:35:25 -0300 Subject: [PATCH 4/5] Update tests/repeated-updates.sh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörg Thalheim --- tests/repeated-updates.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/repeated-updates.sh b/tests/repeated-updates.sh index 0a575742..d21f45ac 100755 --- a/tests/repeated-updates.sh +++ b/tests/repeated-updates.sh @@ -33,7 +33,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} > ${load_segments_before} + 2 )) +if [ ${load_segments_after} -gt ${load_segments_before} + 2 ] then exit 1 fi From eb9ea0094bd7e2a278c0d57cb5d8b032a13fc5e4 Mon Sep 17 00:00:00 2001 From: Breno Rodrigues Guimaraes Date: Thu, 23 Feb 2023 22:15:12 -0300 Subject: [PATCH 5/5] Shellcheck fixes --- tests/repeated-updates.sh | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/repeated-updates.sh b/tests/repeated-updates.sh index d21f45ac..669b11d5 100755 --- a/tests/repeated-updates.sh +++ b/tests/repeated-updates.sh @@ -1,16 +1,16 @@ #! /bin/sh -e -SCRATCH=scratch/$(basename $0 .sh) +SCRATCH=scratch/$(basename "$0" .sh) PATCHELF=$(readlink -f "../src/patchelf") -rm -rf ${SCRATCH} -mkdir -p ${SCRATCH} +rm -rf "${SCRATCH}" +mkdir -p "${SCRATCH}" -cp simple ${SCRATCH}/ -cp libfoo.so ${SCRATCH}/ -cp libbar.so ${SCRATCH}/ +cp simple "${SCRATCH}/" +cp libfoo.so "${SCRATCH}/" +cp libbar.so "${SCRATCH}/" -cd ${SCRATCH} +cd "${SCRATCH}" ${PATCHELF} --add-needed ./libbar.so simple @@ -20,7 +20,7 @@ ${PATCHELF} --add-needed ./libbar.so simple ############################################################################### load_segments_before=$(readelf -W -l libbar.so | grep -c LOAD) -for i in $(seq 1 100) +for _ in $(seq 1 100) do ${PATCHELF} --set-soname ./libbar.so libbar.so ${PATCHELF} --set-soname libbar.so libbar.so @@ -33,7 +33,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 + 2)) ] then exit 1 fi