Bug 1461421 Use OffsetOf to calculate the location of parameters_ rather than making assumptions about the parent class r=bobowen
authorTom Ritter <tom@mozilla.com>
Thu, 07 Jun 2018 13:08:27 -0500
changeset 424333 cf350ebb3004dc8eb8c7ea4b22b4ae79ea958ca1
parent 424332 9aa9b21d01081f7c63c879100d4d95d0a27c06bb
child 424334 028265406fe7486dc66f142e234927f7b3736f20
push id104778
push usershindli@mozilla.com
push dateThu, 28 Jun 2018 23:25:48 +0000
treeherdermozilla-inbound@226fffcc9736 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobowen
bugs1461421
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1461421 Use OffsetOf to calculate the location of parameters_ rather than making assumptions about the parent class r=bobowen MozReview-Commit-ID: D7REZiAIMpN
security/sandbox/chromium/sandbox/win/src/crosscall_params.h
security/sandbox/chromium/sandbox/win/src/crosscall_server.cc
--- a/security/sandbox/chromium/sandbox/win/src/crosscall_params.h
+++ b/security/sandbox/chromium/sandbox/win/src/crosscall_params.h
@@ -55,16 +55,17 @@ union MultiType {
   ULONG_PTR ulong_ptr;
 };
 
 // Maximum number of IPC parameters currently supported.
 // To increase this value, we have to:
 //  - Add another Callback typedef to Dispatcher.
 //  - Add another case to the switch on SharedMemIPCServer::InvokeCallback.
 //  - Add another case to the switch in GetActualAndMaxBufferSize
+//  - Add another case to the switch in GetMinDeclaredActualCallParamsSize
 const int kMaxIpcParams = 9;
 
 // Contains the information about a parameter in the ipc buffer.
 struct ParamInfo {
   ArgType type_;
   uint32_t offset_;
   uint32_t size_;
 };
@@ -271,16 +272,18 @@ class ActualCallParams : public CrossCal
  protected:
   ActualCallParams() : CrossCallParams(0, NUMBER_PARAMS) { }
 
  private:
   ParamInfo param_info_[NUMBER_PARAMS + 1];
   char parameters_[BLOCK_SIZE - sizeof(CrossCallParams)
                    - sizeof(ParamInfo) * (NUMBER_PARAMS + 1)];
   DISALLOW_COPY_AND_ASSIGN(ActualCallParams);
+
+  friend uint32_t GetMinDeclaredActualCallParamsSize(uint32_t param_count);
 };
 
 static_assert(sizeof(ActualCallParams<1, 1024>) == 1024, "bad size buffer");
 static_assert(sizeof(ActualCallParams<2, 1024>) == 1024, "bad size buffer");
 static_assert(sizeof(ActualCallParams<3, 1024>) == 1024, "bad size buffer");
 
 }  // namespace sandbox
 
--- a/security/sandbox/chromium/sandbox/win/src/crosscall_server.cc
+++ b/security/sandbox/chromium/sandbox/win/src/crosscall_server.cc
@@ -22,30 +22,31 @@ namespace {
 
 // The buffer for a message must match the max channel size.
 const size_t kMaxBufferSize = sandbox::kIPCChannelSize;
 
 }
 
 namespace sandbox {
 
+// The template types are used to calculate the maximum expected size.
+typedef ActualCallParams<0, kMaxBufferSize> ActualCP0;
+typedef ActualCallParams<1, kMaxBufferSize> ActualCP1;
+typedef ActualCallParams<2, kMaxBufferSize> ActualCP2;
+typedef ActualCallParams<3, kMaxBufferSize> ActualCP3;
+typedef ActualCallParams<4, kMaxBufferSize> ActualCP4;
+typedef ActualCallParams<5, kMaxBufferSize> ActualCP5;
+typedef ActualCallParams<6, kMaxBufferSize> ActualCP6;
+typedef ActualCallParams<7, kMaxBufferSize> ActualCP7;
+typedef ActualCallParams<8, kMaxBufferSize> ActualCP8;
+typedef ActualCallParams<9, kMaxBufferSize> ActualCP9;
+
 // Returns the actual size for the parameters in an IPC buffer. Returns
 // zero if the |param_count| is zero or too big.
 uint32_t GetActualBufferSize(uint32_t param_count, void* buffer_base) {
-  // The template types are used to calculate the maximum expected size.
-  typedef ActualCallParams<1, kMaxBufferSize> ActualCP1;
-  typedef ActualCallParams<2, kMaxBufferSize> ActualCP2;
-  typedef ActualCallParams<3, kMaxBufferSize> ActualCP3;
-  typedef ActualCallParams<4, kMaxBufferSize> ActualCP4;
-  typedef ActualCallParams<5, kMaxBufferSize> ActualCP5;
-  typedef ActualCallParams<6, kMaxBufferSize> ActualCP6;
-  typedef ActualCallParams<7, kMaxBufferSize> ActualCP7;
-  typedef ActualCallParams<8, kMaxBufferSize> ActualCP8;
-  typedef ActualCallParams<9, kMaxBufferSize> ActualCP9;
-
   // Retrieve the actual size and the maximum size of the params buffer.
   switch (param_count) {
     case 0:
       return 0;
     case 1:
       return reinterpret_cast<ActualCP1*>(buffer_base)->GetSize();
     case 2:
       return reinterpret_cast<ActualCP2*>(buffer_base)->GetSize();
@@ -63,16 +64,45 @@ uint32_t GetActualBufferSize(uint32_t pa
       return reinterpret_cast<ActualCP8*>(buffer_base)->GetSize();
     case 9:
       return reinterpret_cast<ActualCP9*>(buffer_base)->GetSize();
     default:
       return 0;
   }
 }
 
+// Returns the minimum size for the parameters in an IPC buffer. Returns
+// zero if the |param_count| is less than zero or too big.
+uint32_t GetMinDeclaredActualCallParamsSize(uint32_t param_count) {
+  switch (param_count) {
+    case 0:
+      return offsetof(ActualCP0, parameters_);
+    case 1:
+      return offsetof(ActualCP1, parameters_);
+    case 2:
+      return offsetof(ActualCP2, parameters_);
+    case 3:
+      return offsetof(ActualCP3, parameters_);
+    case 4:
+      return offsetof(ActualCP4, parameters_);
+    case 5:
+      return offsetof(ActualCP5, parameters_);
+    case 6:
+      return offsetof(ActualCP6, parameters_);
+    case 7:
+      return offsetof(ActualCP7, parameters_);
+    case 8:
+      return offsetof(ActualCP8, parameters_);
+    case 9:
+      return offsetof(ActualCP9, parameters_);
+    default:
+      return 0;
+  }
+}
+
 // Verifies that the declared sizes of an IPC buffer are within range.
 bool IsSizeWithinRange(uint32_t buffer_size,
                        uint32_t min_declared_size,
                        uint32_t declared_size) {
   if ((buffer_size < min_declared_size) ||
       (sizeof(CrossCallParamsEx) > min_declared_size)) {
     // Minimal computed size bigger than existing buffer or param_count
     // integer overflow.
@@ -132,18 +162,17 @@ CrossCallParamsEx* CrossCallParamsEx::Cr
   // will catch memory access violations so we don't crash.
   __try {
     CrossCallParams* call_params =
         reinterpret_cast<CrossCallParams*>(buffer_base);
 
     // Check against the minimum size given the number of stated params
     // if too small we bail out.
     param_count = call_params->GetParamsCount();
-    min_declared_size = sizeof(CrossCallParams) +
-                        ((param_count + 1) * sizeof(ParamInfo));
+    min_declared_size = GetMinDeclaredActualCallParamsSize(param_count);
 
     // Retrieve the declared size which if it fails returns 0.
     declared_size = GetActualBufferSize(param_count, buffer_base);
 
     if (!IsSizeWithinRange(buffer_size, min_declared_size, declared_size))
       return NULL;
 
     // Now we copy the actual amount of the message.
@@ -152,18 +181,17 @@ CrossCallParamsEx* CrossCallParamsEx::Cr
     copied_params = reinterpret_cast<CrossCallParamsEx*>(backing_mem);
     memcpy(backing_mem, call_params, declared_size);
 
     // Avoid compiler optimizations across this point. Any value stored in
     // memory should be stored for real, and values previously read from memory
     // should be actually read.
     _ReadWriteBarrier();
 
-    min_declared_size = sizeof(CrossCallParams) +
-                        ((param_count + 1) * sizeof(ParamInfo));
+    min_declared_size = GetMinDeclaredActualCallParamsSize(param_count);
 
     // Check that the copied buffer is still valid.
     if (copied_params->GetParamsCount() != param_count ||
         GetActualBufferSize(param_count, backing_mem) != declared_size ||
         !IsSizeWithinRange(buffer_size, min_declared_size, declared_size)) {
       delete [] backing_mem;
       return NULL;
     }