Bounds-check rbp when unwinding a stack frame that uses rbp as a frame pointer.

base/profiler/native_stack_sampler_mac.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/profiler/native_stack_sampler.h"
 
 #include <dlfcn.h>
 #include <libkern/OSByteOrder.h>
 #include <libunwind.h>
+#include <mach-o/compact_unwind_encoding.h>
 #include <mach-o/swap.h>
 #include <mach/kern_return.h>
 #include <mach/mach.h>
 #include <mach/thread_act.h>
 #include <pthread.h>
 #include <sys/resource.h>
 #include <sys/syslimits.h>
 
 #include <algorithm>
 #include <map>
@@ skipping 86 matching lines @@
     *reg = RewritePointerIfInOriginalStack(
         original_stack_bottom, original_stack_top, stack_copy_bottom, *reg);
   }
 }
 
 // Walks the stack represented by |unwind_context|, calling back to the provided
 // lambda for each frame. Returns false if an error occurred, otherwise returns
 // true.
 template <typename StackFrameCallback>
 bool WalkStackFromContext(unw_context_t* unwind_context,
+                          uintptr_t stack_top,
                           size_t* frame_count,
                           const StackFrameCallback& callback) {
   unw_cursor_t unwind_cursor;
   unw_init_local(&unwind_cursor, unwind_context);
 
   int step_result;
   unw_word_t ip;
   do {
     ++(*frame_count);
     unw_get_reg(&unwind_cursor, UNW_REG_IP, &ip);
 
     callback(static_cast<uintptr_t>(ip));
 
+    // If this stack frame has a frame pointer, stepping the cursor will involve

[Mike Wittman, 2017/05/19 21:21:21]

nit: this could be extracted to a separate function

[Avi (use Gerrit), 2017/05/22 19:03:37]

It could be, but it's short enough and is only used here, so I'm not sure I see
a reason to extract it.

[Mike Wittman, 2017/05/22 19:19:58]

I was thinking it would make this function a little easier to read, but the
effect is relatively marginal so feel free to leave as is.

+    // indexing memory access off of that pointer. In that case, sanity-check

[Mark Mentovai, 2017/05/22 20:29:44]

Does unw_init_remote() work instead of unw_init_local()? (Even though you’re
dealing with the same process.)

Probably not, it looks like it’s not fully/properly implemented. If it worked,
it’d get you something that accessed memory via mach_vm_read() or similar, which
will just fail when asked to read something unmapped or protected. Compare this
to the raw pointer dereference, which crashes under those circumstances.

[Avi (use Gerrit), 2017/05/22 20:32:35]

I saw that when looking at the source. Dunno why they'd spec out an API and then
neglect to actually implement it.

+    // the frame pointer register to ensure it's within bounds.
+    unw_proc_info_t proc_info;
+    unw_get_proc_info(&unwind_cursor, &proc_info);
+    if ((proc_info.format & UNWIND_X86_64_MODE_MASK) ==
+        UNWIND_X86_64_MODE_RBP_FRAME) {
+      unw_word_t rsp, rbp;
+      unw_get_reg(&unwind_cursor, UNW_X86_64_RSP, &rsp);
+      unw_get_reg(&unwind_cursor, UNW_X86_64_RBP, &rbp);
+      if (rbp < rsp || rbp > stack_top)
+        return false;
+    }
+
     step_result = unw_step(&unwind_cursor);
   } while (step_result > 0);
 
   if (step_result != 0)
     return false;
 
   return true;
 }
 
 const char* LibSystemKernelName() {
@@ skipping 26 matching lines @@
 
   // Problem 1: There is no official way to create a unw_context other than to
   // create it from the current state of the current thread's stack. To get
   // around this, forge a context. A unw_context is just a copy of the 16 main
   // registers followed by the instruction pointer, nothing more.
   // Coincidentally, the first 17 items of the x86_thread_state64_t type are
   // exactly those registers in exactly the same order, so just bulk copy them
   // over.
   unw_context_t unwind_context;
   memcpy(&unwind_context, &thread_state, sizeof(uintptr_t) * 17);
-  bool result = WalkStackFromContext(&unwind_context, &frame_count, callback);
+  bool result =
+      WalkStackFromContext(&unwind_context, stack_top, &frame_count, callback);
 
   if (!result)
     return;
 
   if (frame_count == 1) {
     // Problem 2: Because libunwind is designed to be triggered by user code on
     // their own thread, if it hits a library that has no unwind info for the
     // function that is being executed, it just stops. This isn't a problem in
     // the normal case, but in this case, it's quite possible that the stack
     // being walked is stopped in a function that bridges to the kernel and thus
     // is missing the unwind info.
 
     // For now, just unwind the single case where the thread is stopped in a
     // function in libsystem_kernel.
     uint64_t& rsp = unwind_context.data[7];
     uint64_t& rip = unwind_context.data[16];
     Dl_info info;
     if (dladdr(reinterpret_cast<void*>(rip), &info) != 0 &&
       strcmp(info.dli_fname, LibSystemKernelName()) == 0) {
       rip = *reinterpret_cast<uint64_t*>(rsp);
       rsp += 8;
-      WalkStackFromContext(&unwind_context, &frame_count, callback);
+      WalkStackFromContext(&unwind_context, stack_top, &frame_count, callback);
     }
   }
 }
 
 // Module identifiers ---------------------------------------------------------
 
 // Returns the unique build ID for a module loaded at |module_addr|. Returns the
 // empty string if the function fails to get the build ID.
 //
 // Build IDs are created by the concatenation of the module's GUID (Windows) /
@@ skipping 249 matching lines @@
       stack_rlimit.rlim_cur != RLIM_INFINITY) {
     return stack_rlimit.rlim_cur;
   }
 
   // If getrlimit somehow fails, return the default macOS main thread stack size
   // of 8 MB (DFLSSIZ in <i386/vmparam.h>) with extra wiggle room.
   return 12 * 1024 * 1024;
 }
 
 }  // namespace base