Bug 1113930 - Add frame size santy check in frame pointer stack walk. draft
authorXidorn Quan <quanxunzhen@gmail.com>
Fri, 18 Dec 2015 17:25:18 +1100
changeset 316294 51c7524a141d9e77737c3475ef70436f8da384be
parent 315901 d7ba0a23dcab86784d273469ccf533c7c4661165
child 316304 06583a7c8804cbbcac3d9f2b39af4a000b9d5e1a
push id8525
push userxquan@mozilla.com
push dateFri, 18 Dec 2015 09:05:00 +0000
bugs1113930
milestone46.0a1
Bug 1113930 - Add frame size santy check in frame pointer stack walk.
mozglue/misc/StackWalk.cpp
--- a/mozglue/misc/StackWalk.cpp
+++ b/mozglue/misc/StackWalk.cpp
@@ -1049,36 +1049,59 @@ MozDescribeCodeAddress(void* aPC, MozCod
   aDetails->foffset = 0;
   return false;
 }
 
 #endif
 
 #if defined(XP_WIN) || defined (XP_MACOSX) || defined (XP_LINUX)
 namespace mozilla {
+static inline bool
+IsFramePointerValid(void** aNext, void** aCurrent, void* aStackEnd)
+{
+  auto next = reinterpret_cast<uintptr_t>(aNext);
+  auto current = reinterpret_cast<uintptr_t>(aCurrent);
+  auto stackEnd = reinterpret_cast<uintptr_t>(aStackEnd);
+  // We don't need to check against the begining of the stack because
+  // we can assume that bp > sp
+  if (next <= current || next > stackEnd || (next & 3)) {
+    return false;
+  }
+
+  // Do we ever have a single stack frame larger than 16MB?
+  static MOZ_CONSTEXPR size_t kMaxFrameSize = 16 * 1024 * 1024;
+  // If the difference between the current position and the next one is
+  // larger than kMaxFrameSize, we consider the next pointer invalid,
+  // because we should hardly have such large stack frame.
+  // (On some *nix systems, we may want to use system call write() to
+  // check whether the specific page is accessible in the process. But
+  // that would add much more complexity with little extra value.)
+  if (current < stackEnd - kMaxFrameSize &&
+      next > current + kMaxFrameSize) {
+    return false;
+  }
+  return true;
+}
+
 bool
 FramePointerStackWalk(MozWalkStackCallback aCallback, uint32_t aSkipFrames,
                       uint32_t aMaxFrames, void* aClosure, void** bp,
                       void* aStackEnd)
 {
   // Stack walking code courtesy Kipp's "leaky".
 
   int32_t skip = aSkipFrames;
   uint32_t numFrames = 0;
   while (bp) {
     void** next = (void**)*bp;
     // bp may not be a frame pointer on i386 if code was compiled with
     // -fomit-frame-pointer, so do some sanity checks.
     // (bp should be a frame pointer on ppc(64) but checking anyway may help
     // a little if the stack has been corrupted.)
-    // We don't need to check against the begining of the stack because
-    // we can assume that bp > sp
-    if (next <= bp ||
-        next > aStackEnd ||
-        (uintptr_t(next) & 3)) {
+    if (!IsFramePointerValid(next, bp, aStackEnd)) {
       break;
     }
 #if (defined(__ppc__) && defined(XP_MACOSX)) || defined(__powerpc64__)
     // ppc mac or powerpc64 linux
     void* pc = *(bp + 2);
     bp += 3;
 #else // i386 or powerpc32 linux
     void* pc = *(bp + 1);