Control: tags -1 patch fixed-upstream

Hi,

On 06/10/16 09:42, James Cowgill wrote:
> Control: forwarded -1 
> https://gitlab.kitware.com/cmake/cmake/merge_requests/147
> Control: tags -1 sid stretch
> 
> Hi,
> 
> I've submitted the above PR upstream which should fix this issue. The
> patch was a bit bigger than what I would have liked due to the cmELF
> class requiring some changes.

The PR was accepted and I've attached a patch which backports it to 3.6.

Thanks,
James
From c51099bb1018191964e5ee05e17f133233160806 Mon Sep 17 00:00:00 2001
From: James Cowgill <jcowg...@debian.org>
Date: Thu, 13 Oct 2016 11:00:00 +0100
Subject: [PATCH] MIPS: Handle DT_MIPS_RLD_MAP_REL tag when removing RPath from
 ELFs

This is a combination of 5 commits.

elf: remove tag switch from ELF_Dyn ByteSwap function
elf: add DynamicEntryList methods and rpath tag constants
cmSystemTools: rewrite RemoveRPath using DyanmicEntryList methods
cmSystemTools, elf: handle DT_MIPS_RLD_REL_MAP in RemoveRPath
elf: Remove GetDynamicEntryCount and ReadBytes methods

Backported from upstream 3.8.0 commit:
ea563a27a2042cfb3be33d0f74efecc7687b86bb
---
 Source/cmELF.cxx         | 225 ++++++++++++++++-------------------------------
 Source/cmELF.h           |  24 +++--
 Source/cmSystemTools.cxx |  72 ++++++++-------
 3 files changed, 135 insertions(+), 186 deletions(-)

diff --git a/Source/cmELF.cxx b/Source/cmELF.cxx
index 26f1a44..9fe8a43 100644
--- a/Source/cmELF.cxx
+++ b/Source/cmELF.cxx
@@ -134,18 +134,13 @@ public:
 
   // Forward to the per-class implementation.
   virtual unsigned int GetNumberOfSections() const = 0;
-  virtual unsigned int GetDynamicEntryCount() = 0;
   virtual unsigned long GetDynamicEntryPosition(int j) = 0;
+  virtual cmELF::DynamicEntryList GetDynamicEntries() = 0;
+  virtual std::vector<char> EncodeDynamicEntries(
+    const cmELF::DynamicEntryList&) = 0;
   virtual StringEntry const* GetDynamicSectionString(unsigned int tag) = 0;
   virtual void PrintInfo(std::ostream& os) const = 0;
 
-  bool ReadBytes(unsigned long pos, unsigned long size, char* buf)
-  {
-    this->Stream.seekg(pos);
-    this->Stream.read(buf, size);
-    return this->Stream ? true : false;
-  }
-
   // Lookup the SONAME in the DYNAMIC section.
   StringEntry const* GetSOName()
   {
@@ -246,10 +241,12 @@ public:
     return static_cast<unsigned int>(this->ELFHeader.e_shnum);
   }
 
-  // Get the file position and size of a dynamic section entry.
-  virtual unsigned int GetDynamicEntryCount();
+  // Get the file position of a dynamic section entry.
   virtual unsigned long GetDynamicEntryPosition(int j);
 
+  virtual cmELF::DynamicEntryList GetDynamicEntries();
+  virtual std::vector<char> EncodeDynamicEntries(const cmELF::DynamicEntryList&);
+
   // Lookup a string from the dynamic section with the given tag.
   virtual StringEntry const* GetDynamicSectionString(unsigned int tag);
 
@@ -289,6 +286,10 @@ public:
   }
 
 private:
+  // ByteSwap(ELF_Dyn) assumes d_val and d_ptr are the same size
+  typedef char dyn_size_assert
+    [sizeof(ELF_Dyn().d_un.d_val) == sizeof(ELF_Dyn().d_un.d_ptr) ? 1 : -1];
+
   void ByteSwap(ELF_Ehdr& elf_header)
   {
     cmELFByteSwap(elf_header.e_type);
@@ -323,121 +324,7 @@ private:
   void ByteSwap(ELF_Dyn& dyn)
   {
     cmELFByteSwap(dyn.d_tag);
-    switch (dyn.d_tag) {
-      case DT_NULL: /* dyn.d_un ignored */
-        break;
-      case DT_NEEDED:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_PLTRELSZ:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_PLTGOT:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_HASH:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_STRTAB:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_SYMTAB:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_RELA:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_RELASZ:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_RELAENT:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_STRSZ:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_SYMENT:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_INIT:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_FINI:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_SONAME:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_RPATH:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_SYMBOLIC: /* dyn.d_un ignored */
-        break;
-      case DT_REL:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_RELSZ:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_RELENT:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_PLTREL:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-      case DT_DEBUG:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-      case DT_TEXTREL: /* dyn.d_un ignored */
-        break;
-      case DT_JMPREL:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-#ifdef T_BIND_NOW
-      case T_BIND_NOW: /* dyn.d_un ignored */
-        break;
-#endif
-#ifdef DT_INIT_ARRAY
-      case DT_INIT_ARRAY:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-#endif
-#ifdef DT_FINI_ARRAY
-      case DT_FINI_ARRAY:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-#endif
-#ifdef DT_INIT_ARRAYSZ
-      case DT_INIT_ARRAYSZ:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-#endif
-#ifdef DT_FINI_ARRAYSZ
-      case DT_FINI_ARRAYSZ:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-#endif
-#ifdef DT_RUNPATH
-      case DT_RUNPATH:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-#endif
-#ifdef DT_FLAGS
-      case DT_FLAGS:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-#endif
-#ifdef DT_PREINIT_ARRAY
-      case DT_PREINIT_ARRAY:
-        cmELFByteSwap(dyn.d_un.d_ptr);
-        break;
-#endif
-#ifdef DT_PREINIT_ARRAYSZ
-      case DT_PREINIT_ARRAYSZ:
-        cmELFByteSwap(dyn.d_un.d_val);
-        break;
-#endif
-    }
+    cmELFByteSwap(dyn.d_un.d_val);
   }
 
   bool FileTypeValid(ELF_Half et)
@@ -635,30 +522,64 @@ bool cmELFInternalImpl<Types>::LoadDynamicSection()
 }
 
 template <class Types>
-unsigned int cmELFInternalImpl<Types>::GetDynamicEntryCount()
+unsigned long cmELFInternalImpl<Types>::GetDynamicEntryPosition(int j)
 {
   if (!this->LoadDynamicSection()) {
     return 0;
   }
-  for (unsigned int i = 0; i < this->DynamicSectionEntries.size(); ++i) {
-    if (this->DynamicSectionEntries[i].d_tag == DT_NULL) {
-      return i;
-    }
+  if (j < 0 || j >= static_cast<int>(this->DynamicSectionEntries.size())) {
+    return 0;
   }
-  return static_cast<unsigned int>(this->DynamicSectionEntries.size());
+  ELF_Shdr const& sec = this->SectionHeaders[this->DynamicSectionIndex];
+  return static_cast<unsigned long>(sec.sh_offset + sec.sh_entsize * j);
 }
 
 template <class Types>
-unsigned long cmELFInternalImpl<Types>::GetDynamicEntryPosition(int j)
+cmELF::DynamicEntryList cmELFInternalImpl<Types>::GetDynamicEntries()
 {
+  cmELF::DynamicEntryList result;
+
+  // Ensure entries have been read from file
   if (!this->LoadDynamicSection()) {
-    return 0;
+    return result;
   }
-  if (j < 0 || j >= static_cast<int>(this->DynamicSectionEntries.size())) {
-    return 0;
+
+  // Copy into public array
+  result.reserve(this->DynamicSectionEntries.size());
+  for (typename std::vector<ELF_Dyn>::iterator di =
+         this->DynamicSectionEntries.begin();
+       di != this->DynamicSectionEntries.end(); ++di) {
+    ELF_Dyn& dyn = *di;
+    result.push_back(
+      std::pair<unsigned long, unsigned long>(dyn.d_tag, dyn.d_un.d_val));
   }
-  ELF_Shdr const& sec = this->SectionHeaders[this->DynamicSectionIndex];
-  return static_cast<unsigned long>(sec.sh_offset + sec.sh_entsize * j);
+
+  return result;
+}
+
+template <class Types>
+std::vector<char> cmELFInternalImpl<Types>::EncodeDynamicEntries(
+  const cmELF::DynamicEntryList& entries)
+{
+  std::vector<char> result;
+  result.reserve(sizeof(ELF_Dyn) * entries.size());
+
+  for (cmELF::DynamicEntryList::const_iterator it = entries.begin();
+       it != entries.end(); it++) {
+    // Store the entry in an ELF_Dyn, byteswap it, then serialize to chars
+    ELF_Dyn dyn;
+    dyn.d_tag = static_cast<tagtype>(it->first);
+    dyn.d_un.d_val = static_cast<tagtype>(it->second);
+
+    if (this->NeedSwap) {
+      ByteSwap(dyn);
+    }
+
+    char* pdyn = reinterpret_cast<char*>(&dyn);
+    result.insert(result.end(), pdyn, pdyn + sizeof(ELF_Dyn));
+  }
+
+  return result;
 }
 
 template <class Types>
@@ -751,6 +672,15 @@ cmELF::StringEntry const* cmELFInternalImpl<Types>::GetDynamicSectionString(
 //============================================================================
 // External class implementation.
 
+const long cmELF::TagRPath = DT_RPATH;
+const long cmELF::TagRunPath = DT_RUNPATH;
+
+#ifdef DT_MIPS_RLD_MAP_REL
+const long cmELF::TagMipsRldMapRel = DT_MIPS_RLD_MAP_REL;
+#else
+const long cmELF::TagMipsRldMapRel = 0;
+#endif
+
 cmELF::cmELF(const char* fname)
   : Internal(0)
 {
@@ -836,31 +766,32 @@ unsigned int cmELF::GetNumberOfSections() const
   }
 }
 
-unsigned int cmELF::GetDynamicEntryCount() const
+unsigned long cmELF::GetDynamicEntryPosition(int index) const
 {
   if (this->Valid()) {
-    return this->Internal->GetDynamicEntryCount();
+    return this->Internal->GetDynamicEntryPosition(index);
   } else {
     return 0;
   }
 }
 
-unsigned long cmELF::GetDynamicEntryPosition(int index) const
+cmELF::DynamicEntryList cmELF::GetDynamicEntries() const
 {
   if (this->Valid()) {
-    return this->Internal->GetDynamicEntryPosition(index);
-  } else {
-    return 0;
+    return this->Internal->GetDynamicEntries();
   }
+
+  return cmELF::DynamicEntryList();
 }
 
-bool cmELF::ReadBytes(unsigned long pos, unsigned long size, char* buf) const
+std::vector<char> cmELF::EncodeDynamicEntries(
+  const cmELF::DynamicEntryList& dentries) const
 {
   if (this->Valid()) {
-    return this->Internal->ReadBytes(pos, size, buf);
-  } else {
-    return false;
+    return this->Internal->EncodeDynamicEntries(dentries);
   }
+
+  return std::vector<char>();
 }
 
 bool cmELF::GetSOName(std::string& soname)
diff --git a/Source/cmELF.h b/Source/cmELF.h
index 80832ad..2ddce63 100644
--- a/Source/cmELF.h
+++ b/Source/cmELF.h
@@ -12,6 +12,9 @@
 #ifndef cmELF_h
 #define cmELF_h
 
+#include <utility>
+#include <vector>
+
 #if !defined(CMAKE_USE_ELF_PARSER)
 #error "This file may be included only if CMAKE_USE_ELF_PARSER is enabled."
 #endif
@@ -65,22 +68,27 @@ public:
     int IndexInSection;
   };
 
+  /** Represent entire dynamic section header */
+  typedef std::vector<std::pair<long, unsigned long> > DynamicEntryList;
+
   /** Get the type of the file opened.  */
   FileType GetFileType() const;
 
   /** Get the number of ELF sections present.  */
   unsigned int GetNumberOfSections() const;
 
-  /** Get the number of DYNAMIC section entries before the first
-      DT_NULL.  Returns zero on error.  */
-  unsigned int GetDynamicEntryCount() const;
-
   /** Get the position of a DYNAMIC section header entry.  Returns
       zero on error.  */
   unsigned long GetDynamicEntryPosition(int index) const;
 
-  /** Read bytes from the file.  */
-  bool ReadBytes(unsigned long pos, unsigned long size, char* buf) const;
+  /** Get a copy of all the DYNAMIC section header entries.
+      Returns an empty vector on error */
+  DynamicEntryList GetDynamicEntries() const;
+
+  /** Encodes a DYNAMIC section header entry list into a char vector according
+      to the type of ELF file this is */
+  std::vector<char> EncodeDynamicEntries(
+    const DynamicEntryList& entries) const;
 
   /** Get the SONAME field if any.  */
   bool GetSOName(std::string& soname);
@@ -95,6 +103,10 @@ public:
   /** Print human-readable information about the ELF file.  */
   void PrintInfo(std::ostream& os) const;
 
+  /** Interesting dynamic tags.
+      If the tag is 0, it does not exist in the host ELF implementation */
+  static const long TagRPath, TagRunPath, TagMipsRldMapRel;
+
 private:
   friend class cmELFInternal;
   bool Valid() const;
diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index 7dece47..bf50ef3 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -2439,9 +2439,9 @@ bool cmSystemTools::RemoveRPath(std::string const& file, std::string* emsg,
       std::swap(se[0], se[1]);
     }
 
-    // Get the size of the dynamic section header.
-    unsigned int count = elf.GetDynamicEntryCount();
-    if (count == 0) {
+    // Obtain a copy of the dynamic entries
+    cmELF::DynamicEntryList dentries = elf.GetDynamicEntries();
+    if (dentries.empty()) {
       // This should happen only for invalid ELF files where a DT_NULL
       // appears before the end of the table.
       if (emsg) {
@@ -2457,40 +2457,46 @@ bool cmSystemTools::RemoveRPath(std::string const& file, std::string* emsg,
       zeroSize[i] = se[i]->Size;
     }
 
-    // Get the range of file positions corresponding to each entry and
-    // the rest of the table after them.
-    unsigned long entryBegin[3] = { 0, 0, 0 };
-    unsigned long entryEnd[2] = { 0, 0 };
-    for (int i = 0; i < se_count; ++i) {
-      entryBegin[i] = elf.GetDynamicEntryPosition(se[i]->IndexInSection);
-      entryEnd[i] = elf.GetDynamicEntryPosition(se[i]->IndexInSection + 1);
-    }
-    entryBegin[se_count] = elf.GetDynamicEntryPosition(count);
-
-    // The data are to be written over the old table entries starting at
-    // the first one being removed.
-    bytesBegin = entryBegin[0];
-    unsigned long bytesEnd = entryBegin[se_count];
+    // Get size of one DYNAMIC entry
+    unsigned long const sizeof_dentry =
+      elf.GetDynamicEntryPosition(1) - elf.GetDynamicEntryPosition(0);
 
-    // Allocate a buffer to hold the part of the file to be written.
-    // Initialize it with zeros.
-    bytes.resize(bytesEnd - bytesBegin, 0);
-
-    // Read the part of the DYNAMIC section header that will move.
-    // The remainder of the buffer will be left with zeros which
-    // represent a DT_NULL entry.
-    char* data = &bytes[0];
-    for (int i = 0; i < se_count; ++i) {
-      // Read data between the entries being removed.
-      unsigned long sz = entryBegin[i + 1] - entryEnd[i];
-      if (sz > 0 && !elf.ReadBytes(entryEnd[i], sz, data)) {
-        if (emsg) {
-          *emsg = "Failed to read DYNAMIC section header.";
+    // Adjust the entry list as necessary to remove the run path
+    unsigned long entriesErased = 0;
+    for (cmELF::DynamicEntryList::iterator it = dentries.begin();
+         it != dentries.end();) {
+      if (it->first == cmELF::TagRPath || it->first == cmELF::TagRunPath) {
+        it = dentries.erase(it);
+        entriesErased++;
+        continue;
+      } else {
+        if (cmELF::TagMipsRldMapRel != 0 &&
+            it->first == cmELF::TagMipsRldMapRel) {
+          // Background: debuggers need to know the "linker map" which contains
+          // the addresses each dynamic object is loaded at. Most arches use
+          // the DT_DEBUG tag which the dynamic linker writes to (directly) and
+          // contain the location of the linker map, however on MIPS the
+          // .dynamic section is always read-only so this is not possible. MIPS
+          // objects instead contain a DT_MIPS_RLD_MAP tag which contains the
+          // address where the dyanmic linker will write to (an indirect
+          // version of DT_DEBUG). Since this doesn't work when using PIE, a
+          // relative equivalent was created - DT_MIPS_RLD_MAP_REL. Since this
+          // version contains a relative offset, moving it changes the
+          // calculated address. This may cause the dyanmic linker to write
+          // into memory it should not be changing.
+          //
+          // To fix this, we adjust the value of DT_MIPS_RLD_MAP_REL here. If
+          // we move it up by n bytes, we add n bytes to the value of this tag.
+          it->second += entriesErased * sizeof_dentry;
         }
-        return false;
+
+        it++;
       }
-      data += sz;
     }
+
+    // Encode new entries list
+    bytes = elf.EncodeDynamicEntries(dentries);
+    bytesBegin = elf.GetDynamicEntryPosition(0);
   }
 
   // Open the file for update.
-- 
2.9.3

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to