Update breakpad for Android packed relocations.

client/linux/minidump_writer/linux_dumper.cc

Index: client/linux/minidump_writer/linux_dumper.cc
diff --git a/client/linux/minidump_writer/linux_dumper.cc b/client/linux/minidump_writer/linux_dumper.cc
index ebb008d6692e6ffa5d7a788db811c4076cf35d11..a06db8f920500927d47d9125d8a65fade300fc22 100644
--- a/client/linux/minidump_writer/linux_dumper.cc
+++ b/client/linux/minidump_writer/linux_dumper.cc
@@ -52,6 +52,22 @@
 #include "common/linux/safe_readlink.h"
 #include "third_party/lss/linux_syscall_support.h"
 
+#if defined(__ANDROID__)
+
+// Android packed relocations definitions are not yet available from the
+// NDK header files, so we have to provide them manually here.
+#ifndef DT_LOOS
+#define DT_LOOS 0x6000000d
+#endif
+#ifndef DT_ANDROID_REL
+static const int DT_ANDROID_REL = DT_LOOS + 2;
+#endif
+#ifndef DT_ANDROID_RELA
+static const int DT_ANDROID_RELA = DT_LOOS + 4;
+#endif
+
+#endif  // __ANDROID __
+
 static const char kMappedFileUnsafePrefix[] = "/dev/";
 static const char kDeletedSuffix[] = " (deleted)";
 static const char kReservedFlags[] = " ---p";
@@ -92,6 +108,14 @@ bool LinuxDumper::Init() {
   return ReadAuxv() && EnumerateThreads() && EnumerateMappings();
 }
 
+bool LinuxDumper::LateInit() {
+#if defined(__ANDROID__)
+  return LatePostprocessMappings();
+#else
+  return true;
+#endif
+}
+
 bool
 LinuxDumper::ElfFileIdentifierForMapping(const MappingInfo& mapping,
                                          bool member,
@@ -395,6 +419,127 @@ bool LinuxDumper::EnumerateMappings() {
   return !mappings_.empty();
 }
 
+#if defined(__ANDROID__)
+
+// Read the memory at start_addr, expecting to find an ELF header. The first

[Lei Zhang, 2015/06/18 21:45:37]

Comments should go in the header. Ditto below.

[simonb (inactive), 2015/06/19 12:19:27]

Done.

+// LOAD segment in an ELF shared library has offset zero, so the ELF file
+// header is at the start of this map entry, and in already mapped memory.
+bool LinuxDumper::GetLoadedElfHeader(uintptr_t start_addr, ElfW(Ehdr)* ehdr) {
+  CopyFromProcess(ehdr, pid_,
+                  reinterpret_cast<const void*>(start_addr),
+                  sizeof(*ehdr));
+  if (my_memcmp(&ehdr->e_ident, ELFMAG, SELFMAG) != 0) {

[Lei Zhang, 2015/06/18 21:45:37]

Just: return (my_memcmp(...) != 0); ?

[simonb (inactive), 2015/06/19 12:19:27]

Done.

+    return false;
+  }
+  return true;
+}
+
+// Iterate ELF program headers to find the min vaddr of LOAD segments, and
+// the vaddr and count of entries for the DYNAMIC table. The program header
+// table is also in already mapped memory.
+void LinuxDumper::ParseLoadedElfProgramHeaders(ElfW(Ehdr)* ehdr,
+                                               uintptr_t start_addr,
+                                               uintptr_t* min_vaddr_ptr,
+                                               uintptr_t* dyn_vaddr_ptr,
+                                               size_t* dyn_count_ptr) {
+  uintptr_t phdr_addr = start_addr + ehdr->e_phoff;

[Lei Zhang, 2015/06/18 21:45:37]

Can this or the addition to |phdr_addr| below overflow?

[simonb (inactive), 2015/06/19 12:19:27]

To somewhat amplify Primiano's comment below... not unless memory here is
corrupted, in which case all bets are off anyway.

The dynamic linker cannot map a segment of size N at an address greater than
0xfff..f - N.  And static linker cannot create a library where e_phoff is
greater than N.  If we encounter conditions that apparently violate this, we
have either a corrupted .so file or corrupted (readonly-mapped!) memory.

In the unlikely case we encounter this, we either inspect mapped memory that is
not actually the program headers, dynamic tags, and so on, or we wander outside
mapped memory entirely, where CopyFromProcess returns all-zeros for attempts to
read unmapped addresses.  Either way we will almost certainly fail to find
DT_ANDROID_REL[A] in HasAndroidPackedRelocations, not adjust start_addr, and get
the same results out of breakpad as if this attempt to correct start_addr were
not present.

+
+  const uintptr_t max_addr = UINTPTR_MAX;
+  uintptr_t min_vaddr = max_addr;
+  uintptr_t dyn_vaddr = 0;
+  size_t dyn_count = 0;
+
+  for (size_t i = 0; i < ehdr->e_phnum; ++i) {
+    ElfW(Phdr) phdr;
+    CopyFromProcess(&phdr, pid_,
+                    reinterpret_cast<const void*>(phdr_addr),
+                    sizeof(phdr));
+    if (phdr.p_type == PT_LOAD && phdr.p_vaddr < min_vaddr) {
+      min_vaddr = phdr.p_vaddr;
+    }
+    if (phdr.p_type == PT_DYNAMIC) {
+      dyn_vaddr = phdr.p_vaddr;
+      dyn_count = phdr.p_memsz / sizeof(ElfW(Dyn));
+    }
+    phdr_addr += sizeof(phdr);
+  }
+
+  *min_vaddr_ptr = min_vaddr;
+  *dyn_vaddr_ptr = dyn_vaddr;
+  *dyn_count_ptr = dyn_count;
+}
+
+// Retrieve and check dynamic tags, and return true if tags for Android
+// packed relocations as present. Dynamic tags are found at dyn_vaddr past
+// the load_bias.
+bool LinuxDumper::HasAndroidPackedRelocations(uintptr_t load_bias,
+                                              uintptr_t dyn_vaddr,
+                                              size_t dyn_count) {
+  uintptr_t dyn_addr = load_bias + dyn_vaddr;

[Lei Zhang, 2015/06/18 21:45:37]

Can this or the addition to |dyn_addr| below overflow?

[Primiano Tucci (use gerrit), 2015/06/18 21:56:13]

 but in that case CopyFromProcess  will just fill up any failed ptrace(PEEKDATA)
with zeros, which is the all point of using CopyFromProcess rather than trying
to dereference stuff yourself in the forked process image.

+  for (size_t i = 0; i < dyn_count; ++i) {
+    ElfW(Dyn) dyn;
+    CopyFromProcess(&dyn, pid_,
+                    reinterpret_cast<const void*>(dyn_addr),
+                    sizeof(dyn));
+    if (dyn.d_tag == DT_ANDROID_REL || dyn.d_tag == DT_ANDROID_RELA) {
+      return true;
+    }
+    dyn_addr += sizeof(dyn);
+  }
+  return false;
+}
+
+// Return the effective load_bias, used by the system linker (or Chromium
+// crazy linker) for the ELF shared library mapped at the given start_addr.
+// The effective load_bias is start_addr adjusted downwards by the min vaddr
+// in the library LOAD segments.
+uintptr_t LinuxDumper::GetEffectiveLoadBias(ElfW(Ehdr)* ehdr,
+                                            uintptr_t start_addr) {
+  uintptr_t min_vaddr = 0;
+  uintptr_t dyn_vaddr = 0;
+  size_t dyn_count = 0;
+  ParseLoadedElfProgramHeaders(ehdr, start_addr,
+                               &min_vaddr, &dyn_vaddr, &dyn_count);
+  // If min vaddr is non-zero and we find Android packed relocation tags,

[Lei Zhang, 2015/06/18 21:45:37]

min vaddr -> |min_vaddr|

[simonb (inactive), 2015/06/19 12:19:28]

Done.

+  // return the effective load_bias.
+  if (min_vaddr != 0) {
+    const uintptr_t load_bias = start_addr - min_vaddr;

[Lei Zhang, 2015/06/18 21:45:37]

Is |min_vaddr| guaranteeded to be less than |start_addr| ? i.e. this won't
underflow.

[simonb (inactive), 2015/06/19 12:19:27]

Done.

[Lei Zhang, 2015/06/19 18:59:28]

Can I get a yes/no answer on this question?

[simonb (inactive), 2015/06/22 16:52:42]

Sorry, mis-clicked Done here.

What all of this code is doing is re-calculating the value of load_bias that the
linker used internally, and in the same way as the linker.  Line 346 of:
https://android.googlesource.com/platform/bionic/+/android-m-preview/linker/l...
Or in the case of the chromium crazy linker:
https://cs.corp.google.com/#clankium/src/third_party/android_crazy_linker/src...

The linker selects a (somewhat) random address for start_addr, in Android's case
with ASLR.  min_vaddr is equal to the space saved by relocation packing, so
around 0x200000 (32-bit) to 0x600000 (64-bit).  ASLR from mmap in the linker
would have to return an address lower than this for min_vaddr to be greater than
start_addr.

If it does, the linker's load_bias is 'negative' through underflow, so our's
must be also.  load_bias is an 'effective' value, and addresses between it and
start_addr are never accessed.  When added back in inside the linker, the
symmetrical overflow gets the right values.

The probability of load_bias becoming negative is low, but perhaps non-zero.  If
it occurs, the right actions for breakpad to take are to wrap it on use, using
the the target's overflow/underflow, so 32/64 bit depending on the target's
type, to arrive at the right addresses.

Having said that, it appears that breakpad non-client code may not wrap as it
should for this case.  For example, the microdump reader casts to uint64_t
before use, so this would not wrap the adjustment in the way the target did. 
I'm not sure about other parts of breakpad at the moment.

Any ideas for the best fix for this?

Unstated here is that the load_bias and the start_addr for a library are not
necessarily the same, but breakpad assumes that they always are because they
have been up to now.  Relocation packing uncouples them by introducing a
non-zero vaddr to the first LOAD segment.  Breakpad reads start_addr from
/proc/self/maps, but maps does not expose load_bias, and that is what breakpad
really requires.

+    if (HasAndroidPackedRelocations(load_bias, dyn_vaddr, dyn_count)) {
+      return load_bias;
+    }
+  }
+  // Either min vaddr is zero, or it is non-zero but we did not find the
+  // expected Android packed relocations tags.
+  return start_addr;
+}
+
+// Iterate mappings_, and adjust any start_addr fields that are for mapped

[Lei Zhang, 2015/06/18 21:45:38]

|mappings_|

[simonb (inactive), 2015/06/19 12:19:27]

Done.

+// libraries that contain Android packed relocations.
+bool LinuxDumper::LatePostprocessMappings() {
+  for (size_t i = 0; i < mappings_.size(); ++i) {
+    // Only consider exec mappings that indicate a file path was mapped, and
+    // where the ELF header indicates a mapped shared library.
+    MappingInfo* mapping = mappings_[i];
+    if (!(mapping->exec && mapping->name[0] == '/')) {
+      continue;
+    }
+    ElfW(Ehdr) ehdr;
+    if (!GetLoadedElfHeader(mapping->start_addr, &ehdr)) {
+      continue;
+    }
+    if (ehdr.e_type == ET_DYN) {
+      // Compute the effective load_bias for this mapped library, and update
+      // the mapping to hold that rather than start_addr. Where the library
+      // does not contain Android packed relocations, GetEffectiveLoadBias()
+      // returns start_addr and the mapping entry is not changed.
+      mapping->start_addr = GetEffectiveLoadBias(&ehdr, mapping->start_addr);
+    }
+  }
+  return true;

[Lei Zhang, 2015/06/18 21:45:37]

This function can't return anything but true as is.

[simonb (inactive), 2015/06/19 12:19:27]

Done.

I have left LateInit() returning bool for now, even though the only thing
currently done by LateInit() is to call this function, which 'cannot' fail. 
LateInit() seems more in the way of a framework extension than underlying
implementation.

+}
+
+#endif  // __ANDROID__
+
 // Get information about the stack, given the stack pointer. We don't try to
 // walk the stack since we might not have all the information needed to do
 // unwind. So we just grab, up to, 32k of stack.