Add ELF header analysis when checking for instruction pointer in code.

src/processor/exploitability_linux.h

 // Copyright (c) 2013 Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 // notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
@@ skipping 19 matching lines @@
 // exploitability_linux.h: Linux specific exploitability engine.
 //
 // Provides a guess at the exploitability of the crash for the Linux
 // platform given a minidump and process_state.
 //
 // Author: Matthew Riley
 
 #ifndef GOOGLE_BREAKPAD_PROCESSOR_EXPLOITABILITY_LINUX_H_
 #define GOOGLE_BREAKPAD_PROCESSOR_EXPLOITABILITY_LINUX_H_
 
+#include <elf.h>
+
 #include "google_breakpad/common/breakpad_types.h"
 #include "google_breakpad/processor/exploitability.h"
 
 namespace google_breakpad {
 
 class ExploitabilityLinux : public Exploitability {
  public:
   ExploitabilityLinux(Minidump *dump,
                       ProcessState *process_state);
 
   virtual ExploitabilityRating CheckPlatformExploitability();
 
  private:
   // This method takes the address of the instruction pointer and returns
   // whether the instruction pointer lies in a valid instruction region.
   bool InstructionPointerInCode(uint64_t instruction_ptr);
 
   // This method checks the exception that triggered the creation of the
   // minidump and reports whether the exception suggests no exploitability.
   bool BenignCrashTrigger(const MDRawExceptionStream *raw_exception_stream);
+
+  // Checks if the minidump architecture is 32-bit or 64-bit.
+  bool Architecture32Bit();

[ivanpe, 2015/07/14 00:23:02]

Maybe a better name would be: Is32BitArchitecture()?

[liuandrew, 2015/07/15 21:50:14]

I changed the method name to ArchitectureType() to better match the enum that it
returns.

+
+  // Copies ELF header data from the minidump's memory to a pointer.

[ivanpe, 2015/07/14 00:23:02]

It is a bit unclear which ones are input and which ones are output parameters. 
Can you also document the return value?  Is it a buffer?   What is its size?  Is
there a transfer of ownership?

[ahonig, 2015/07/14 19:32:18]

To clarify Ivan's point

It looks from the interface that this function allocates memory and returns the
allocated chunk which the caller must free.  You should explicitly state that.  

I think a much cleaner interface would be to have the caller allocate the
memory.  It's almost always cleaner to have code that deletes the memory as
close to the allocation as possible.  In this case the caller knows the size, so
I don't see why you can't have the caller allocate the memory.

[liuandrew, 2015/07/15 21:50:14]

Done.

+  // This method takes in the region of memory to be copied, the starting
+  // memory address to copy, and the number of bytes to copy.
+  void *LoadElfHeader(MinidumpMemoryRegion *memory,
+                      const uint64_t base_address,
+                      size_t header_size);
+
+  // Copies the ELF header of a 32-bit module.
+  // This method takes in the region of memory to be copied, and the
+  // starting memory address to copy.
+  Elf32_Ehdr *LoadElf32Header(MinidumpMemoryRegion *memory,

[ivanpe, 2015/07/14 00:23:02]

Please, document the return value and clarify whether there is a transfer of
ownership.

[liuandrew, 2015/07/15 21:50:14]

method removed

+                              const uint64_t base_address);
+
+  // Copies the ELF header of a 64-bit module.
+  // This method takes in the region of memory to be copied, and the
+  // starting memory address to copy.
+  Elf64_Ehdr *LoadElf64Header(MinidumpMemoryRegion *memory,

[ivanpe, 2015/07/14 00:23:02]

Please, document the return value and clarify whether there is a transfer of
ownership.

[liuandrew, 2015/07/15 21:50:14]

method removed

+                              const uint64_t base_address);
+
+  // Copies over a 32-bit module's program header table.
+  // This method takes in the region of memory to be copied, the base
+  // address of the module, the offset from the base address to the
+  // starting memory address to copy, the size of an entry in the program
+  // header table, and the number of entries in the program header table.
+  Elf32_Phdr *LoadElf32PHeader(MinidumpMemoryRegion *memory,

[ivanpe, 2015/07/14 00:23:02]

Please, document the return value and clarify whether there is a transfer of
ownership.

[liuandrew, 2015/07/15 21:50:14]

method removed

+                               const uint64_t base_address,
+                               const uint64_t e_phoff,
+                               const uint16_t e_phentsize,
+                               const uint16_t e_phnum);
+
+  // Copies over a 64-bit module's program header table.
+  // This method takes in the region of memory to be copied, the base
+  // address of the module, the offset from the base address to the
+  // starting memory address to copy, the size of an entry in the program
+  // header table, and the number of entries in the program header table.
+  Elf64_Phdr *LoadElf64PHeader(MinidumpMemoryRegion *memory,

[ivanpe, 2015/07/14 00:23:02]

Please, document the return value and clarify whether there is a transfer of
ownership.

[liuandrew, 2015/07/15 21:50:14]

method removed

+                               const uint64_t base_address,
+                               const uint64_t e_phoff,
+                               const uint16_t e_phentsize,
+                               const uint16_t e_phnum);
 };
 
 }  // namespace google_breakpad
 
 #endif  // GOOGLE_BREAKPAD_PROCESSOR_EXPLOITABILITY_LINUX_H_