From 85b83d010d024cf438106a23d932509cebb37162 Mon Sep 17 00:00:00 2001 From: David Gow Date: Mon, 11 Apr 2022 14:06:31 +0800 Subject: [PATCH 1/3] Cleanup .gnu.version_r after removing symbol version If --clear-symbol-version is used to remove all occurrances of a symbol version, it nevertheless remains in the .gnu.version_r section as being required by a particular dependency. Instead, after removing symbol versions, loop over the .gnu.version and gnu.version_r tables to find any unused version, and unlink them from the linked-lists in the .gnu.version_r section. --- src/patchelf.cc | 75 +++++++++++++++++++++++++++++++++++++++++++++++-- src/patchelf.h | 6 ++-- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/src/patchelf.cc b/src/patchelf.cc index 49accae1..ca20730e 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -1709,6 +1709,73 @@ void ElfFile::addDebugTag() changed = true; } +/* Remove any unused dependency symbol versions from .gnu.version_r */ +template +void ElfFile::cleanDependencySymbolVersions() +{ + auto shdrVersym = findSectionHeader(".gnu.version"); + auto shdrVersymR = findSectionHeader(".gnu.version_r"); + + auto versyms = (Elf_Versym *)(fileContents->data() + rdi(shdrVersym.sh_offset)); + size_t count = rdi(shdrVersym.sh_size) / sizeof(Elf_Versym); + + /* Set of versions actually used. */ + std::set allVersions; + + for (size_t i = 0; i < count; i++) { + allVersions.insert(versyms[i]); + } + + /* Strings associated with .gnu_version_r section: used for debug only. */ + Elf_Shdr & shdrVersionRStrings = shdrs.at(rdi(shdrVersymR.sh_link)); + char * verStrTab = (char *) fileContents->data() + rdi(shdrVersionRStrings.sh_offset); + + + auto ver_r = (Elf_Verneed *)(fileContents->data() + rdi(shdrVersymR.sh_offset)); + while (true) { + auto prev = (Elf_Vernaux *)nullptr; + auto vern_aux = (Elf_Vernaux *)((char *)ver_r + rdi(ver_r->vn_aux)); + char * file = verStrTab + rdi(ver_r->vn_file); + for (size_t j = 0; j < ver_r->vn_cnt ; j++) { + char * ver_name = verStrTab + rdi(vern_aux->vna_name); + auto next = (Elf_Vernaux *)((char *)vern_aux + rdi(vern_aux->vna_next)); + + if (!allVersions.count(rdi(vern_aux->vna_other) & ~0x8000)) { + debug("Removing version identifier %d %s@%s\n", rdi(vern_aux->vna_other), file, ver_name); + /* Symbol version is no longer used, unlink it. */ + if (!prev) { + auto next_off = (intptr_t)(vern_aux) + rdi(vern_aux->vna_next) - (intptr_t)(ver_r); + wri(ver_r->vn_aux, next_off); + } else { + auto next_off = (intptr_t)(vern_aux) + rdi(vern_aux->vna_next) - (intptr_t)(prev); + wri(prev->vna_next, next_off); + } + wri(ver_r->vn_cnt, rdi(ver_r->vn_cnt) - 1); + } else { + prev = vern_aux; + } + + if (vern_aux == next) { + if (j != rdi(ver_r->vn_cnt)) { + debug("Section missing elements! Ended on element %d, expected %d\n", j, rdi(ver_r->vn_cnt)); + } + break; + } + vern_aux = next; + } + + /* If this was the last entry, we're done. */ + if (!rdi(ver_r->vn_next)) { + break; + } + + ver_r = (Elf_Verneed *) (((char *) ver_r) + rdi(ver_r->vn_next)); + } + + changed = true; + this->rewriteSections(); +} + template void ElfFile::clearSymbolVersions(const std::set & syms) { @@ -1734,6 +1801,10 @@ void ElfFile::clearSymbolVersions(const std::set wri(versyms[i], 1); } } + + /* Remove entries in the .gnu.versions_r table which are no;-longer required. */ + cleanDependencySymbolVersions(); + changed = true; this->rewriteSections(); } @@ -1817,9 +1888,9 @@ static void patchElf() const std::string & outputFileName2 = outputFileName.empty() ? fileName : outputFileName; if (getElfType(fileContents).is32Bit) - patchElf2(ElfFile(fileContents), fileContents, outputFileName2); + patchElf2(ElfFile(fileContents), fileContents, outputFileName2); else - patchElf2(ElfFile(fileContents), fileContents, outputFileName2); + patchElf2(ElfFile(fileContents), fileContents, outputFileName2); } } diff --git a/src/patchelf.h b/src/patchelf.h index 33ecfc3f..96b75a2c 100644 --- a/src/patchelf.h +++ b/src/patchelf.h @@ -1,7 +1,7 @@ using FileContents = std::shared_ptr>; -#define ElfFileParams class Elf_Ehdr, class Elf_Phdr, class Elf_Shdr, class Elf_Addr, class Elf_Off, class Elf_Dyn, class Elf_Sym, class Elf_Verneed, class Elf_Versym -#define ElfFileParamNames Elf_Ehdr, Elf_Phdr, Elf_Shdr, Elf_Addr, Elf_Off, Elf_Dyn, Elf_Sym, Elf_Verneed, Elf_Versym +#define ElfFileParams class Elf_Ehdr, class Elf_Phdr, class Elf_Shdr, class Elf_Addr, class Elf_Off, class Elf_Dyn, class Elf_Sym, class Elf_Verneed, class Elf_Vernaux, class Elf_Versym +#define ElfFileParamNames Elf_Ehdr, Elf_Phdr, Elf_Shdr, Elf_Addr, Elf_Off, Elf_Dyn, Elf_Sym, Elf_Verneed, Elf_Vernaux, Elf_Versym template class ElfFile @@ -133,6 +133,8 @@ class ElfFile void addDebugTag(); + void cleanDependencySymbolVersions(); + void clearSymbolVersions(const std::set & syms); private: From 7ff7a80eeebb7f8a9fddee2d309a8c6d61ec5edb Mon Sep 17 00:00:00 2001 From: David Gow Date: Mon, 11 Apr 2022 14:48:10 +0800 Subject: [PATCH 2/3] Add a test for --clear-symbol-version This test removes the __libc_start_main@GLIBC_2.34 symbol, which should be the only GLIBC_2.34 symbol in 'main' when built with recent (i.e., > 2.34) glibc. Because it is the only symbol, the entry in .gnu.version_r should also be removed. Because this is a symbol version test, it's unlikely to work on musl or anything else which doesn't use glibc symbol versions. In these cases (or in the case that __libc_start_main is now at a different version), the test should print a warning, but exit with "0" to report a pass. --- tests/Makefile.am | 3 ++- tests/clear-symver.sh | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100755 tests/clear-symver.sh diff --git a/tests/Makefile.am b/tests/Makefile.am index bbea1ba4..adbe3605 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -41,7 +41,8 @@ src_TESTS = \ phdr-corruption.sh \ replace-needed.sh \ replace-add-needed.sh \ - add-debug-tag.sh + add-debug-tag.sh \ + clear-symver.sh build_TESTS = \ $(no_rpath_arch_TESTS) diff --git a/tests/clear-symver.sh b/tests/clear-symver.sh new file mode 100755 index 00000000..123a6f90 --- /dev/null +++ b/tests/clear-symver.sh @@ -0,0 +1,33 @@ +#! /bin/sh -e +SCRATCH=scratch/$(basename $0 .sh) + +rm -rf ${SCRATCH} +mkdir -p ${SCRATCH} + +cp main ${SCRATCH}/ + +SYMBOL_TO_REMOVE=__libc_start_main +VERSION_TO_REMOVE=GLIBC_2.34a + +readelfData=$(readelf -V ${SCRATCH}/main 2>&1) + +if [ $(echo "$readelfData" | grep --count "$VERSION_TO_REMOVE") -lt 2 ]; then + # We expect this to appear at least twice: once for the symbol entry, + # and once for verneed entry. + echo "Warning: Couldn't find expected versioned symbol." + echo "This is probably because you're either not using glibc, or" + echo "${SYMBOL_TO_REMOVE} is no longer at version ${VERSION_TO_REMOVE}" + echo "Reporting a pass anyway, as the test result is invalid." + exit 0 +fi + +../src/patchelf --clear-symbol-version ${SYMBOL_TO_REMOVE} ${SCRATCH}/main + +readelfData=$(readelf -V ${SCRATCH}/main 2>&1) + +if [ $(echo "$readelfData" | grep --count "$VERSION_TO_REMOVE") -ne 0 ]; then + # We expect this to appear at least twice: once for the symbol entry, + # and once for verneed entry. + echo "The symbol version ${SYMBOL_TO_REMOVE} remained behind!" + exit 1 +fi From e1aa6d671c6e81172f7924b812ff8c8669e1a9e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Mon, 1 Aug 2022 10:13:00 +0200 Subject: [PATCH 3/3] add more explicit pointer checks when clearing .gnu.version --- src/patchelf.cc | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/patchelf.cc b/src/patchelf.cc index ca20730e..5ea82592 100644 --- a/src/patchelf.cc +++ b/src/patchelf.cc @@ -1716,7 +1716,8 @@ void ElfFile::cleanDependencySymbolVersions() auto shdrVersym = findSectionHeader(".gnu.version"); auto shdrVersymR = findSectionHeader(".gnu.version_r"); - auto versyms = (Elf_Versym *)(fileContents->data() + rdi(shdrVersym.sh_offset)); + auto versyms = reinterpret_cast(fileContents->data() + rdi(shdrVersym.sh_offset)); + checkPointer(fileContents, versyms, sizeof(Elf_Versym)); size_t count = rdi(shdrVersym.sh_size) / sizeof(Elf_Versym); /* Set of versions actually used. */ @@ -1728,17 +1729,24 @@ void ElfFile::cleanDependencySymbolVersions() /* Strings associated with .gnu_version_r section: used for debug only. */ Elf_Shdr & shdrVersionRStrings = shdrs.at(rdi(shdrVersymR.sh_link)); - char * verStrTab = (char *) fileContents->data() + rdi(shdrVersionRStrings.sh_offset); + char * verStrTab = reinterpret_cast(fileContents->data() + rdi(shdrVersionRStrings.sh_offset)); + auto ver_r = reinterpret_cast(fileContents->data() + rdi(shdrVersymR.sh_offset)); + checkPointer(fileContents, ver_r, sizeof(Elf_Verneed)); - auto ver_r = (Elf_Verneed *)(fileContents->data() + rdi(shdrVersymR.sh_offset)); while (true) { auto prev = (Elf_Vernaux *)nullptr; - auto vern_aux = (Elf_Vernaux *)((char *)ver_r + rdi(ver_r->vn_aux)); + auto vern_aux = reinterpret_cast((char *)ver_r + rdi(ver_r->vn_aux)); + checkPointer(fileContents, vern_aux, sizeof(Elf_Vernaux)); + char * file = verStrTab + rdi(ver_r->vn_file); for (size_t j = 0; j < ver_r->vn_cnt ; j++) { char * ver_name = verStrTab + rdi(vern_aux->vna_name); - auto next = (Elf_Vernaux *)((char *)vern_aux + rdi(vern_aux->vna_next)); + // FIXME: add proper check for null-terminated string + checkPointer(fileContents, ver_name, sizeof(char)); + + auto next = reinterpret_cast((char *)vern_aux + rdi(vern_aux->vna_next)); + checkPointer(fileContents, next, sizeof(Elf_Vernaux)); if (!allVersions.count(rdi(vern_aux->vna_other) & ~0x8000)) { debug("Removing version identifier %d %s@%s\n", rdi(vern_aux->vna_other), file, ver_name); @@ -1769,7 +1777,8 @@ void ElfFile::cleanDependencySymbolVersions() break; } - ver_r = (Elf_Verneed *) (((char *) ver_r) + rdi(ver_r->vn_next)); + ver_r = reinterpret_cast(((char *) ver_r) + rdi(ver_r->vn_next)); + checkPointer(fileContents, ver_r, sizeof(Elf_Verneed)); } changed = true;