StackSamplingProfiler: sanity check RUNTIME_FUNCTION contents

Avoids crashes due to bogus RUNTIME_FUNCTION returned by callback
installed by McAfee Host Intrusion Prevention dll.

BUG=541650

Review URL: https://codereview.chromium.org/1391143004

Cr-Commit-Position: refs/heads/master@{#354153}

3 files changed, 211 insertions, 51 deletions

diff --git a/base/profiler/win32_stack_frame_unwinder.cc b/base/profiler/win32_stack_frame_unwinder.cc
index 3bdd288..ea589bb 100644
--- a/base/profiler/win32_stack_frame_unwinder.cc
+++ b/base/profiler/win32_stack_frame_unwinder.cc
@@ -119,6 +119,18 @@ LeafUnwindBlacklist::~LeafUnwindBlacklist() {}
 
 // Win32StackFrameUnwinder ----------------------------------------------------
 
+// hipis0e011b8.dll from McAfee Host Intrusion Prevention has been observed to
+// provide a pointer to a bogus RUNTIME_FUNCTION structure. This function checks
+// that the values in the structure look plausible.
+bool SanityCheckRuntimeFunction(PRUNTIME_FUNCTION runtime_function,
+                                ULONG64 image_base,
+                                DWORD64 program_counter) {
+  const DWORD64 program_counter_offset = program_counter - image_base;
+  return (runtime_function->BeginAddress <= runtime_function->EndAddress &&
+          program_counter_offset >= runtime_function->BeginAddress &&
+          program_counter_offset <= runtime_function->EndAddress);
+}
+
 Win32StackFrameUnwinder::UnwindFunctions::~UnwindFunctions() {}
 Win32StackFrameUnwinder::UnwindFunctions::UnwindFunctions() {}
 
@@ -138,6 +150,9 @@ bool Win32StackFrameUnwinder::TryUnwind(CONTEXT* context) {
       unwind_functions_->LookupFunctionEntry(context->Rip, &image_base);
 
   if (runtime_function) {
+    if (!SanityCheckRuntimeFunction(runtime_function, image_base, context->Rip))
+      return false;
+
     unwind_functions_->VirtualUnwind(image_base, context->Rip, runtime_function,
                                      context);
     at_top_frame_ = false;
diff --git a/base/profiler/win32_stack_frame_unwinder.h b/base/profiler/win32_stack_frame_unwinder.h
index 4ddf76e..3e6c8f0 100644
--- a/base/profiler/win32_stack_frame_unwinder.h
+++ b/base/profiler/win32_stack_frame_unwinder.h
@@ -16,7 +16,11 @@ namespace base {
 #if !defined(_WIN64)
 // Allows code to compile for x86. Actual support for x86 will require either
 // refactoring these interfaces or separate architecture-specific interfaces.
-using PRUNTIME_FUNCTION = void*;
+struct RUNTIME_FUNCTION {
+  DWORD BeginAddress;
+  DWORD EndAddress;
+};
+using PRUNTIME_FUNCTION = RUNTIME_FUNCTION*;
 #endif  // !defined(_WIN64)
 
 // Instances of this class are expected to be created and destroyed for each
diff --git a/base/profiler/win32_stack_frame_unwinder_unittest.cc b/base/profiler/win32_stack_frame_unwinder_unittest.cc
index c9d1c42..ae5a2a6 100644
--- a/base/profiler/win32_stack_frame_unwinder_unittest.cc
+++ b/base/profiler/win32_stack_frame_unwinder_unittest.cc
@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <vector>
+
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/profiler/win32_stack_frame_unwinder.h"
@@ -22,69 +24,136 @@ class TestUnwindFunctions : public Win32StackFrameUnwinder::UnwindFunctions {
                      PRUNTIME_FUNCTION runtime_function,
                      CONTEXT* context) override;
 
-  void SetNoUnwindInfoForNextFrame();
-  void SetImageBaseForNextFrame(DWORD64 image_base);
+  // These functions set whether the next frame will have a RUNTIME_FUNCTION,
+  // and allow specification of a custom image_base.
+  void SetHasRuntimeFunction(CONTEXT* context);
+  void SetHasRuntimeFunction(DWORD64 image_base, CONTEXT* context);
+  void SetHasRuntimeFunction(DWORD64 image_base,
+                             const RUNTIME_FUNCTION& runtime_function,
+                             DWORD program_counter_offset,
+                             CONTEXT* context);
+  void SetNoRuntimeFunction(CONTEXT* context);
+  void SetNoRuntimeFunction(DWORD64 image_base, CONTEXT* context);
 
  private:
-  enum { kRuntimeFunctionPointerIncrement = 1, kImageBaseIncrement = 1 << 20 };
+  enum RuntimeFunctionState { NO_RUNTIME_FUNCTION, HAS_RUNTIME_FUNCTION };
+
+  enum { kImageBaseIncrement = 1 << 20 };
+
+  // Sets whether the next frame should have a RUNTIME_FUNCTION, and allows
+  // specification of a custom image_base.
+  void SetNextFrameState(RuntimeFunctionState runtime_function_state,
+                         DWORD64 image_base,
+                         CONTEXT* context);
 
-  static const PRUNTIME_FUNCTION kNonNullRuntimeFunctionPointer;
+  static RUNTIME_FUNCTION* const kInvalidRuntimeFunction;
 
-  DWORD64 supplied_program_counter_;
+  DWORD64 expected_program_counter_;
   DWORD64 custom_image_base_;
   DWORD64 next_image_base_;
-  bool next_lookup_returns_null_;
+  DWORD64 expected_image_base_;
+  RUNTIME_FUNCTION* next_runtime_function_;
+  std::vector<RUNTIME_FUNCTION> runtime_functions_;
 
   DISALLOW_COPY_AND_ASSIGN(TestUnwindFunctions);
 };
 
-// This value is opaque to Win32StackFrameUnwinder.
-const PRUNTIME_FUNCTION TestUnwindFunctions::kNonNullRuntimeFunctionPointer =
-    reinterpret_cast<PRUNTIME_FUNCTION>(8);
+RUNTIME_FUNCTION* const TestUnwindFunctions::kInvalidRuntimeFunction =
+    reinterpret_cast<RUNTIME_FUNCTION*>(static_cast<uintptr_t>(-1));
 
 TestUnwindFunctions::TestUnwindFunctions()
-    : supplied_program_counter_(0),
+    : expected_program_counter_(0),
       custom_image_base_(0),
       next_image_base_(kImageBaseIncrement),
-      next_lookup_returns_null_(false) {
+      expected_image_base_(0),
+      next_runtime_function_(kInvalidRuntimeFunction) {
 }
 
 PRUNTIME_FUNCTION TestUnwindFunctions::LookupFunctionEntry(
     DWORD64 program_counter,
     PDWORD64 image_base) {
-  supplied_program_counter_ = program_counter;
+  EXPECT_EQ(expected_program_counter_, program_counter);
   if (custom_image_base_) {
-    *image_base = custom_image_base_;
+    *image_base = expected_image_base_ = custom_image_base_;
     custom_image_base_ = 0;
   } else {
-    *image_base = next_image_base_;
+    *image_base = expected_image_base_ = next_image_base_;
     next_image_base_ += kImageBaseIncrement;
   }
-  if (next_lookup_returns_null_) {
-    next_lookup_returns_null_ = false;
-    return nullptr;
-  }
-
-  return kNonNullRuntimeFunctionPointer;
+  RUNTIME_FUNCTION* return_value = next_runtime_function_;
+  next_runtime_function_ = kInvalidRuntimeFunction;
+  return return_value;
 }
 
 void TestUnwindFunctions::VirtualUnwind(DWORD64 image_base,
                                         DWORD64 program_counter,
                                         PRUNTIME_FUNCTION runtime_function,
                                         CONTEXT* context) {
-  EXPECT_EQ(next_image_base_ - kImageBaseIncrement, image_base);
-  EXPECT_EQ(supplied_program_counter_, program_counter);
-  // This function should only be called when LookupFunctionEntry returns a
-  // non-null value.
-  EXPECT_EQ(kNonNullRuntimeFunctionPointer, runtime_function);
+  ASSERT_NE(kInvalidRuntimeFunction, runtime_function)
+      << "expected call to SetHasRuntimeFunction() or SetNoRuntimeFunction() "
+      << "before invoking TryUnwind()";
+  EXPECT_EQ(expected_image_base_, image_base);
+  expected_image_base_ = 0;
+  EXPECT_EQ(expected_program_counter_, program_counter);
+  expected_program_counter_ = 0;
+  // This function should only be called when LookupFunctionEntry returns
+  // a RUNTIME_FUNCTION.
+  EXPECT_EQ(&runtime_functions_.back(), runtime_function);
+}
+
+void TestUnwindFunctions::SetHasRuntimeFunction(CONTEXT* context) {
+  SetNextFrameState(HAS_RUNTIME_FUNCTION, 0, context);
+}
+
+void TestUnwindFunctions::SetHasRuntimeFunction(DWORD64 image_base,
+                                                CONTEXT* context) {
+  SetNextFrameState(HAS_RUNTIME_FUNCTION, image_base, context);
 }
 
-void TestUnwindFunctions::SetNoUnwindInfoForNextFrame() {
-  next_lookup_returns_null_ = true;
+void TestUnwindFunctions::SetHasRuntimeFunction(
+    DWORD64 image_base,
+    const RUNTIME_FUNCTION& runtime_function,
+    DWORD program_counter_offset,
+    CONTEXT* context) {
+  custom_image_base_ = image_base;
+  runtime_functions_.push_back(runtime_function);
+  next_runtime_function_ = &runtime_functions_.back();
+  expected_program_counter_ = context->Rip =
+      image_base + program_counter_offset;
 }
 
-void TestUnwindFunctions::SetImageBaseForNextFrame(DWORD64 image_base) {
-  next_image_base_ = image_base;
+void TestUnwindFunctions::SetNoRuntimeFunction(CONTEXT* context) {
+  SetNextFrameState(NO_RUNTIME_FUNCTION, 0, context);
+}
+
+void TestUnwindFunctions::SetNoRuntimeFunction(DWORD64 image_base,
+                                               CONTEXT* context) {
+  SetNextFrameState(NO_RUNTIME_FUNCTION, image_base, context);
+}
+
+
+void TestUnwindFunctions::SetNextFrameState(
+    RuntimeFunctionState runtime_function_state,
+    DWORD64 image_base,
+    CONTEXT* context) {
+  if (image_base)
+    custom_image_base_ = image_base;
+
+  if (runtime_function_state == HAS_RUNTIME_FUNCTION) {
+    RUNTIME_FUNCTION runtime_function = {};
+    runtime_function.BeginAddress = 16;
+    runtime_function.EndAddress = runtime_function.BeginAddress + 256;
+    runtime_functions_.push_back(runtime_function);
+    next_runtime_function_ = &runtime_functions_.back();
+
+    DWORD64 image_base = custom_image_base_ ? custom_image_base_ :
+        next_image_base_;
+    expected_program_counter_ = context->Rip =
+        image_base + runtime_function.BeginAddress + 8;
+  } else {
+    expected_program_counter_ = context->Rip = 100;
+    next_runtime_function_ = nullptr;
+  }
 }
 
 }  // namespace
@@ -115,8 +184,14 @@ Win32StackFrameUnwinderTest::CreateUnwinder() {
 TEST_F(Win32StackFrameUnwinderTest, FramesWithUnwindInfo) {
   scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
   CONTEXT context = {0};
+
+  unwind_functions_->SetHasRuntimeFunction(&context);
   EXPECT_TRUE(unwinder->TryUnwind(&context));
+
+  unwind_functions_->SetHasRuntimeFunction(&context);
   EXPECT_TRUE(unwinder->TryUnwind(&context));
+
+  unwind_functions_->SetHasRuntimeFunction(&context);
   EXPECT_TRUE(unwinder->TryUnwind(&context));
 }
 
@@ -127,12 +202,16 @@ TEST_F(Win32StackFrameUnwinderTest, FrameAtTopWithoutUnwindInfo) {
   CONTEXT context = {0};
   const DWORD64 original_rsp = 128;
   context.Rsp = original_rsp;
-  unwind_functions_->SetNoUnwindInfoForNextFrame();
+
+  unwind_functions_->SetNoRuntimeFunction(&context);
   EXPECT_TRUE(unwinder->TryUnwind(&context));
   EXPECT_EQ(original_rsp, context.Rip);
   EXPECT_EQ(original_rsp + 8, context.Rsp);
 
+  unwind_functions_->SetHasRuntimeFunction(&context);
   EXPECT_TRUE(unwinder->TryUnwind(&context));
+
+  unwind_functions_->SetHasRuntimeFunction(&context);
   EXPECT_TRUE(unwinder->TryUnwind(&context));
 }
 
@@ -144,11 +223,12 @@ TEST_F(Win32StackFrameUnwinderTest, BlacklistedModule) {
     // First stack, with a bad function below the top of the stack.
     scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
     CONTEXT context = {0};
+    unwind_functions_->SetHasRuntimeFunction(&context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
 
-    unwind_functions_->SetNoUnwindInfoForNextFrame();
-    unwind_functions_->SetImageBaseForNextFrame(
-        image_base_for_module_with_bad_function);
+    unwind_functions_->SetNoRuntimeFunction(
+        image_base_for_module_with_bad_function,
+        &context);
     EXPECT_FALSE(unwinder->TryUnwind(&context));
   }
 
@@ -157,9 +237,9 @@ TEST_F(Win32StackFrameUnwinderTest, BlacklistedModule) {
     // unwind info from the previously-seen module is blacklisted.
     scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
     CONTEXT context = {0};
-    unwind_functions_->SetNoUnwindInfoForNextFrame();
-    unwind_functions_->SetImageBaseForNextFrame(
-        image_base_for_module_with_bad_function);
+    unwind_functions_->SetNoRuntimeFunction(
+        image_base_for_module_with_bad_function,
+        &context);
     EXPECT_FALSE(unwinder->TryUnwind(&context));
   }
 
@@ -171,14 +251,17 @@ TEST_F(Win32StackFrameUnwinderTest, BlacklistedModule) {
     // module.
     scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
     CONTEXT context = {0};
-    unwind_functions_->SetImageBaseForNextFrame(
-        image_base_for_module_with_bad_function);
+    unwind_functions_->SetHasRuntimeFunction(
+        image_base_for_module_with_bad_function,
+        &context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
 
+    unwind_functions_->SetHasRuntimeFunction(&context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
 
-    unwind_functions_->SetImageBaseForNextFrame(
-        image_base_for_module_with_bad_function);
+    unwind_functions_->SetHasRuntimeFunction(
+        image_base_for_module_with_bad_function,
+        &context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
   }
 
@@ -190,13 +273,15 @@ TEST_F(Win32StackFrameUnwinderTest, BlacklistedModule) {
     // previously-seen module.
     scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
     CONTEXT context = {0};
-    unwind_functions_->SetNoUnwindInfoForNextFrame();
+    unwind_functions_->SetNoRuntimeFunction(&context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
 
+    unwind_functions_->SetHasRuntimeFunction(&context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
 
-    unwind_functions_->SetImageBaseForNextFrame(
-        image_base_for_module_with_bad_function);
+    unwind_functions_->SetHasRuntimeFunction(
+        image_base_for_module_with_bad_function,
+        &context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
   }
 }
@@ -211,12 +296,11 @@ TEST_F(Win32StackFrameUnwinderTest, ModuleFromQuestionableFrameNotBlacklisted) {
     // First stack, with both the first and second frames missing unwind info.
     scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
     CONTEXT context = {0};
-    unwind_functions_->SetNoUnwindInfoForNextFrame();
+    unwind_functions_->SetNoRuntimeFunction(&context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
 
-    unwind_functions_->SetNoUnwindInfoForNextFrame();
-    unwind_functions_->SetImageBaseForNextFrame(
-        image_base_for_questionable_module);
+    unwind_functions_->SetNoRuntimeFunction(image_base_for_questionable_module,
+                                            &context);
     EXPECT_FALSE(unwinder->TryUnwind(&context));
   }
 
@@ -224,11 +308,68 @@ TEST_F(Win32StackFrameUnwinderTest, ModuleFromQuestionableFrameNotBlacklisted) {
     // Second stack; check that the questionable module was not blacklisted.
     scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
     CONTEXT context = {0};
-    unwind_functions_->SetNoUnwindInfoForNextFrame();
-    unwind_functions_->SetImageBaseForNextFrame(
-        image_base_for_questionable_module);
+    unwind_functions_->SetNoRuntimeFunction(image_base_for_questionable_module,
+                                            &context);
+    EXPECT_TRUE(unwinder->TryUnwind(&context));
+  }
+}
+
+// Checks that frames with RUNTIME_FUNCTION structures with nonsensical values
+// are not unwound.
+TEST_F(Win32StackFrameUnwinderTest, RuntimeFunctionSanityCheck) {
+  const DWORD64 image_base_for_sanity_check = 3072;
+  {
+    // Test the expected case: end address greater than begin address and
+    // instruction pointer between the two.
+    scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
+    CONTEXT context = {0};
+    RUNTIME_FUNCTION runtime_function = {0};
+    runtime_function.BeginAddress = 128;
+    runtime_function.EndAddress = 512;
+    unwind_functions_->SetHasRuntimeFunction(image_base_for_sanity_check,
+                                             runtime_function, 256,
+                                             &context);
     EXPECT_TRUE(unwinder->TryUnwind(&context));
   }
+
+  {
+    // Test begin address greater than end address.
+    scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
+    CONTEXT context = {0};
+    RUNTIME_FUNCTION runtime_function = {0};
+    runtime_function.BeginAddress = 512;
+    runtime_function.EndAddress = 128;
+    unwind_functions_->SetHasRuntimeFunction(image_base_for_sanity_check,
+                                             runtime_function, 256,
+                                             &context);
+    EXPECT_FALSE(unwinder->TryUnwind(&context));
+  }
+
+  {
+    // Test instruction pointer before begin address.
+    scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
+    CONTEXT context = {0};
+    RUNTIME_FUNCTION runtime_function = {0};
+    runtime_function.BeginAddress = 128;
+    runtime_function.EndAddress = 512;
+    unwind_functions_->SetHasRuntimeFunction(image_base_for_sanity_check,
+                                             runtime_function, 50,
+                                             &context);
+    EXPECT_FALSE(unwinder->TryUnwind(&context));
+  }
+
+  {
+    // Test instruction pointer after end address.
+    scoped_ptr<Win32StackFrameUnwinder> unwinder = CreateUnwinder();
+    CONTEXT context = {0};
+    RUNTIME_FUNCTION runtime_function = {0};
+    runtime_function.BeginAddress = 128;
+    runtime_function.EndAddress = 512;
+    unwind_functions_->SetHasRuntimeFunction(image_base_for_sanity_check,
+                                             runtime_function, 600,
+                                             &context);
+    EXPECT_FALSE(unwinder->TryUnwind(&context));
+  }
 }
 
 }  // namespace base