fault_handler: use SafeCopy to decode x86 instruction length.

Prevent a crash in the x86 fault handler when pc points to garbage and
we try to figure out its instruction length.

Bug: http://b/30836730
Test: m test-art-host
Change-Id: Ic1fbb8856e30140f0e1ebc9caccf9559e88ff137

diff --git a/runtime/arch/x86/fault_handler_x86.cc b/runtime/arch/x86/fault_handler_x86.cc
index f407ebf..014cc15 100644 (file)
--- a/runtime/arch/x86/fault_handler_x86.cc
+++ b/runtime/arch/x86/fault_handler_x86.cc
@@ -25,6 +25,7 @@
 #include "globals.h"
 #include "base/logging.h"
 #include "base/hex_dump.h"
+#include "base/safe_copy.h"
 #include "thread.h"
 #include "thread-inl.h"
 
@@ -78,6 +79,30 @@ extern "C" void art_quick_test_suspend();
 // Get the size of an instruction in bytes.
 // Return 0 if the instruction is not handled.
 static uint32_t GetInstructionSize(const uint8_t* pc) {
+  // Don't segfault if pc points to garbage.
+  char buf[15];  // x86/x86-64 have a maximum instruction length of 15 bytes.
+  ssize_t bytes = SafeCopy(buf, pc, sizeof(buf));
+
+  if (bytes == 0) {
+    // Nothing was readable.
+    return 0;
+  }
+
+  if (bytes == -1) {
+    // SafeCopy not supported, assume that the entire range is readable.
+    bytes = 16;
+  } else {
+    pc = reinterpret_cast<uint8_t*>(buf);
+  }
+
+#define INCREMENT_PC()          \
+  do {                          \
+    pc++;                       \
+    if (pc - startpc > bytes) { \
+      return 0;                 \
+    }                           \
+  } while (0)
+
 #if defined(__x86_64)
   const bool x86_64 = true;
 #else
@@ -86,7 +111,8 @@ static uint32_t GetInstructionSize(const uint8_t* pc) {
 
   const uint8_t* startpc = pc;
 
-  uint8_t opcode = *pc++;
+  uint8_t opcode = *pc;
+  INCREMENT_PC();
   uint8_t modrm;
   bool has_modrm = false;
   bool two_byte = false;
@@ -118,7 +144,8 @@ static uint32_t GetInstructionSize(const uint8_t* pc) {
 
       // Group 4
       case 0x67:
-        opcode = *pc++;
+        opcode = *pc;
+        INCREMENT_PC();
         prefix_present = true;
         break;
     }
@@ -128,13 +155,15 @@ static uint32_t GetInstructionSize(const uint8_t* pc) {
   }
 
   if (x86_64 && opcode >= 0x40 && opcode <= 0x4f) {
-    opcode = *pc++;
+    opcode = *pc;
+    INCREMENT_PC();
   }
 
   if (opcode == 0x0f) {
     // Two byte opcode
     two_byte = true;
-    opcode = *pc++;
+    opcode = *pc;
+    INCREMENT_PC();
   }
 
   bool unhandled_instruction = false;
@@ -147,7 +176,8 @@ static uint32_t GetInstructionSize(const uint8_t* pc) {
       case 0xb7:
       case 0xbe:        // movsx
       case 0xbf:
-        modrm = *pc++;
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         break;
       default:
@@ -166,28 +196,32 @@ static uint32_t GetInstructionSize(const uint8_t* pc) {
       case 0x3c:
       case 0x3d:
       case 0x85:        // test.
-        modrm = *pc++;
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         break;
 
       case 0x80:        // group 1, byte immediate.
       case 0x83:
       case 0xc6:
-        modrm = *pc++;
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         immediate_size = 1;
         break;
 
       case 0x81:        // group 1, word immediate.
       case 0xc7:        // mov
-        modrm = *pc++;
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         immediate_size = operand_size_prefix ? 2 : 4;
         break;
 
       case 0xf6:
       case 0xf7:
-        modrm = *pc++;
+        modrm = *pc;
+        INCREMENT_PC();
         has_modrm = true;
         switch ((modrm >> 3) & 7) {  // Extract "reg/opcode" from "modr/m".
           case 0:  // test
@@ -222,7 +256,7 @@ static uint32_t GetInstructionSize(const uint8_t* pc) {
 
     // Check for SIB.
     if (mod != 3U /* 0b11 */ && (modrm & 7U /* 0b111 */) == 4) {
-      ++pc;     // SIB
+      INCREMENT_PC();     // SIB
     }
 
     switch (mod) {
@@ -238,6 +272,9 @@ static uint32_t GetInstructionSize(const uint8_t* pc) {
   pc += displacement_size + immediate_size;
 
   VLOG(signals) << "x86 instruction length calculated as " << (pc - startpc);
+  if (pc - startpc > bytes) {
+    return 0;
+  }
   return pc - startpc;
 }