[DynInst_API:] [PATCH] Elf_X: Fix the search for the build-id

[Josh Stone <jistone@xxxxxxxxxx>]

Hi,

Please consider the attached patch to correct the way that Elf_X
searches for the build-id in the mutatee binaries.  I tried to explain
it fully in the commit message, but I'm happy to answer questions or
modify it according to your review.

I discovered this issue when debugging an unrelated problem with my
mutator, and I noticed lines from strace like this:
> stat("/usr/lib/debug/.build-id/\4", 0x7fffc430eda0) = -1 ENOENT (No such file or directory)

That's because it was trying to use .note.gnu.build-id directly as a
string, but really that starts with an Elf_Nhdr.  The first field is
n_namesz, which is 04 00 00 00 in little endian for the name "GNU".  So
that explains how the path from strace shows a raw 4 and is then
truncated by the 0.

This link has more info about build-id if you're interested.  That was
written for Fedora, but AFAIK all that work was contributed to their
respective projects, so it should be implemented the same elsewhere.
https://fedoraproject.org/wiki/Releases/FeatureBuildId

Thanks,
Josh

>From b308f7186034b5cce8913f0cb40c519a121d9c58 Mon Sep 17 00:00:00 2001
From: Josh Stone <jistone@xxxxxxxxxx>
Date: Thu, 11 Oct 2012 17:43:28 -0700
Subject: [PATCH] Elf_X: Fix the search for the build-id

The code was previously acting as though section .note.gnu.build-id
could be read directly as an ASCII string, but this is incorrect.  There
is at least an Elf_Nhdr first, and the actual build-id is in raw bytes.
It's also not guaranteed that the build-id will be in that named
section, as it could be merged with other SHT_NOTE sections.

This patch first adds an Elf_X_Nhdr type to mirror the other Elf_X
abstractions.  For the build-id it walks over all SHT_NOTE sections
looking for type==NT_GNU_BUILD_ID and name=="GNU", and then converts
that to a hex string for the /usr/lib/debug/.build-id/... path.
---
 elf/h/Elf_X.h   |  30 +++++++++++++++++
 elf/src/Elf_X.C | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 125 insertions(+), 5 deletions(-)

diff --git a/elf/h/Elf_X.h b/elf/h/Elf_X.h
index 58b2463..8b3a596 100644
--- a/elf/h/Elf_X.h
+++ b/elf/h/Elf_X.h
@@ -53,6 +53,7 @@ class Elf_X_RegInfo;
 class Elf_32_RegInfo;
 class Elf_64_RegInfo;
 class Elf_X_Dyn;
+class Elf_X_Nhdr;
 
 
 // Wrappers to allow word-independant use of libelf routines.
@@ -205,6 +206,8 @@ class Elf_X_Shdr {
     unsigned wordSize() const;
     Elf_Scn *getScn() const;
 
+    Elf_X_Nhdr get_note() const;
+
   protected:
     Elf_Scn *scn;
     Elf_Data *data;
@@ -504,6 +507,33 @@ class Elf_X_Dyn {
     bool is64;
 };
 
+// ------------------------------------------------------------------------
+// Class Elf_X_Nhdr simulates the Elf(32|64)_Shdr structure.
+class Elf_X_Nhdr {
+    friend class Elf_X;
+
+  public:
+    Elf_X_Nhdr();
+    Elf_X_Nhdr(Elf_Data *data_, size_t offset);
+
+    // Read Interface
+    unsigned long n_namesz() const;
+    unsigned long n_descsz() const;
+    unsigned long n_type() const;
+
+    // Meta-Info Interface
+    bool isValid() const;
+
+    const char* get_name() const;
+    const void* get_desc() const;
+
+    Elf_X_Nhdr next() const;
+
+  protected:
+    Elf_Data *data;
+    Elf32_Nhdr *nhdr;
+};
+
 }
 
 #endif
diff --git a/elf/src/Elf_X.C b/elf/src/Elf_X.C
index e36bd7b..418cc16 100644
--- a/elf/src/Elf_X.C
+++ b/elf/src/Elf_X.C
@@ -43,6 +43,8 @@
 #include "common/h/headers.h"
 #include "elf/h/Elf_X.h"
 #include <iostream>
+#include <iomanip>
+#include <sstream>
 
 using namespace std;
 using boost::crc_32_type;
@@ -701,6 +703,13 @@ Elf_Scn *Elf_X_Shdr::getScn() const
     return scn;
 }
 
+Elf_X_Nhdr Elf_X_Shdr::get_note() const
+{
+    if (sh_type() != SHT_NOTE)
+        return Elf_X_Nhdr();
+    return Elf_X_Nhdr(data, 0);
+}
+
 // ------------------------------------------------------------------------
 // Class Elf_X_Data simulates the Elf_Data structure.
 Elf_X_Data::Elf_X_Data()
@@ -1538,11 +1547,27 @@ bool Elf_X::findDebugFile(std::string origfilename, string &output_name, char* &
         void *crcLocation = ((char *) data.d_buf() + data.d_size() - 4);
         debugFileCrc = *(unsigned *) crcLocation;
      }
-     else if(strcmp(name, BUILD_ID_NAME) == 0) {
-        char *buildId = (char *) scn.get_data().d_buf();
-        string filename = string(buildId + 2) + ".debug";
-        string subdir = string(buildId, 2);
-        debugFileFromBuildID = "/usr/lib/debug/.build-id/" + subdir + "/" + filename;
+     else if (scn.sh_type() == SHT_NOTE) {
+        // Look for a build-id note in this section.  It is usually a note by
+        // itself in section .note.gnu.build-id, but not necessarily so.
+        for (Elf_X_Nhdr note = scn.get_note();
+              note.isValid(); note = note.next()) {
+           if (note.n_type() == 3 // NT_GNU_BUILD_ID
+                 && note.n_namesz() == sizeof("GNU")
+                 && strcmp(note.get_name(), "GNU") == 0
+                 && note.n_descsz() >= 2) {
+              // This is a raw build-id, now convert it to hex
+              const unsigned char *desc = (const unsigned char *)note.get_desc();
+              stringstream buildid_path;
+              buildid_path << "/usr/lib/debug/.build-id/"
+                 << hex << setfill('0') << setw(2) << (unsigned)desc[0] << '/';
+              for (unsigned long j = 1; j < note.n_descsz(); ++j)
+                 buildid_path << (unsigned)desc[j];
+              buildid_path << ".debug";
+              debugFileFromBuildID = buildid_path.str();
+              break;
+           }
+        }
      }
   }
 
@@ -1585,3 +1610,68 @@ bool Elf_X::findDebugFile(std::string origfilename, string &output_name, char* &
 
   return false;
 }
+
+// ------------------------------------------------------------------------
+// Class Elf_X_Nhdr simulates the Elf(32|64)_Nhdr structure.
+Elf_X_Nhdr::Elf_X_Nhdr()
+    : data(NULL), nhdr(NULL)
+{ }
+
+Elf_X_Nhdr::Elf_X_Nhdr(Elf_Data *data_, size_t offset)
+    : data(data_), nhdr(NULL)
+{
+    // 32|64 are actually the same, which simplifies things
+    assert(sizeof(Elf32_Nhdr) == sizeof(Elf64_Nhdr));
+
+    if (data && offset < data->d_size) {
+        size_t size = data->d_size - offset;
+        if (sizeof(*nhdr) <= size) {
+            size -= sizeof(*nhdr);
+            nhdr = (Elf32_Nhdr *)((char *)data->d_buf + offset);
+            if (n_namesz() > size || n_descsz() > size - n_namesz())
+                nhdr = NULL;
+        }
+    }
+    if (!nhdr)
+        data = NULL;
+}
+
+// Read Interface
+unsigned long Elf_X_Nhdr::n_namesz() const
+{
+    return isValid() ? nhdr->n_namesz : 0;
+}
+
+unsigned long Elf_X_Nhdr::n_descsz() const
+{
+    return isValid() ? nhdr->n_descsz : 0;
+}
+
+unsigned long Elf_X_Nhdr::n_type() const
+{
+    return isValid() ? nhdr->n_type : 0;
+}
+
+bool Elf_X_Nhdr::isValid() const
+{
+    return (data && nhdr);
+}
+
+const char* Elf_X_Nhdr::get_name() const
+{
+    return isValid() ? (char *)nhdr + sizeof(*nhdr) : NULL;
+}
+
+const void* Elf_X_Nhdr::get_desc() const
+{
+    return isValid() ? get_name() + n_namesz() : NULL;
+}
+
+Elf_X_Nhdr Elf_X_Nhdr::next() const
+{
+    if (!isValid())
+        return Elf_X_Nhdr();
+
+    size_t offset = (char *)get_desc() + n_descsz() - (char *)data->d_buf;
+    return Elf_X_Nhdr(data, offset);
+}
-- 
1.7.11.7