Bug 1245477 - Crashes in lul::DerefTUW. r=nfroyd a=gchang
authorJulian Seward <jseward@acm.org>
Mon, 06 Feb 2017 09:03:38 +0100
changeset 376162 1b6df84a2353c86d95bf470e65ce17642770bbdb
parent 376161 6c660ca9126fbc8f2c9fd91d7c48853d2268d136
child 376163 9f8949f5a4d72d3a4d1d3bb805f621ffa099fb39
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd, gchang
bugs1245477
milestone53.0a2
Bug 1245477 - Crashes in lul::DerefTUW. r=nfroyd a=gchang
tools/profiler/lul/LulMain.cpp
--- a/tools/profiler/lul/LulMain.cpp
+++ b/tools/profiler/lul/LulMain.cpp
@@ -10,16 +10,17 @@
 #include <stdlib.h>
 #include <stdio.h>
 
 #include <algorithm>  // std::sort
 #include <string>
 
 #include "mozilla/Assertions.h"
 #include "mozilla/ArrayUtils.h"
+#include "mozilla/CheckedInt.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/MemoryChecking.h"
 #include "mozilla/Sprintf.h"
 
 #include "LulCommonExt.h"
 #include "LulElfExt.h"
 
 #include "LulMainInt.h"
@@ -29,16 +30,17 @@
 // Set this to 1 for verbose logging
 #define DEBUG_MAIN 0
 
 namespace lul {
 
 using std::string;
 using std::vector;
 using std::pair;
+using mozilla::CheckedInt;
 using mozilla::DebugOnly;
 
 
 // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
 //
 // Some functions in this file are marked RUNS IN NO-MALLOC CONTEXT.
 // Any such function -- and, hence, the transitive closure of those
 // reachable from it -- must not do any dynamic memory allocation.
@@ -1009,23 +1011,40 @@ LUL::CountMappings()
 
 // RUNS IN NO-MALLOC CONTEXT
 static
 TaggedUWord DerefTUW(TaggedUWord aAddr, const StackImage* aStackImg)
 {
   if (!aAddr.Valid()) {
     return TaggedUWord();
   }
+
+  // Lower limit check.  |aAddr.Value()| is the lowest requested address
+  // and |aStackImg->mStartAvma| is the lowest address we actually have,
+  // so the comparison is straightforward.
   if (aAddr.Value() < aStackImg->mStartAvma) {
     return TaggedUWord();
   }
-  if (aAddr.Value() + sizeof(uintptr_t) > aStackImg->mStartAvma
-                                          + aStackImg->mLen) {
+
+  // Upper limit check.  We must compute the highest requested address
+  // and the highest address we actually have, but being careful to
+  // avoid overflow.  In particular if |aAddr| is 0xFFF...FFF or the
+  // 3/7 values below that, then we will get overflow.  See bug #1245477.
+  typedef CheckedInt<uintptr_t> CheckedUWord;
+  CheckedUWord highest_requested_plus_one
+    = CheckedUWord(aAddr.Value()) + CheckedUWord(sizeof(uintptr_t));
+  CheckedUWord highest_available_plus_one
+    = CheckedUWord(aStackImg->mStartAvma) + CheckedUWord(aStackImg->mLen);
+  if (!highest_requested_plus_one.isValid()     // overflow?
+      || !highest_available_plus_one.isValid()  // overflow?
+      || (highest_requested_plus_one.value()
+          > highest_available_plus_one.value())) { // in range?
     return TaggedUWord();
   }
+
   return TaggedUWord(*(uintptr_t*)(aStackImg->mContents + aAddr.Value()
                                    - aStackImg->mStartAvma));
 }
 
 // RUNS IN NO-MALLOC CONTEXT
 static
 TaggedUWord EvaluateReg(int16_t aReg, const UnwindRegs* aOldRegs,
                         TaggedUWord aCFA)