Bug 1432505 Do not compile inline assembly on MinGW, instead break and abort r?bobowen draft
authorTom Ritter <tom@mozilla.com>
Tue, 23 Jan 2018 11:36:14 -0600
changeset 723656 4f5daf90df153bd1bc85407b50da4d3d302cc37f
parent 723655 c55840a837033f3ff3b3ea4fcabef38985aa879e
child 724104 bf8e193d260d32ed4012d0d77a136ab5700d0fdd
child 724111 93aee7272d7de04251a1c5d3e83e120a30d44f7a
push id96495
push userbmo:tom@mozilla.com
push dateTue, 23 Jan 2018 17:37:54 +0000
reviewersbobowen
bugs1432505
milestone60.0a1
Bug 1432505 Do not compile inline assembly on MinGW, instead break and abort r?bobowen SmartStub is not called on MinGW, so we don't need to compile its assembly (which gcc does not like.) To be sure this is safe, we abort if it is called. MozReview-Commit-ID: Kh32tRs2rVC
security/sandbox/chromium-shim/patches/with_update/mingw_smartstub.patch
security/sandbox/chromium-shim/patches/with_update/patch_order.txt
security/sandbox/chromium/sandbox/win/src/sidestep_resolver.cc
new file mode 100644
--- /dev/null
+++ b/security/sandbox/chromium-shim/patches/with_update/mingw_smartstub.patch
@@ -0,0 +1,101 @@
+# HG changeset patch
+# User Tom Ritter <tom@mozilla.com>
+# Date 1516728974 21600
+#      Tue Jan 23 11:36:14 2018 -0600
+# Node ID 2f95faff9d3f5478d9c89fd9e25a8b65f02b807f
+# Parent  c55840a837033f3ff3b3ea4fcabef38985aa879e
+Bug 1432505 Do not compile inline assembly on MinGW, instead break and abort r?bobowen
+
+SmartStub is not called on MinGW, so we don't need to compile its
+assembly (which gcc does not like.) To be sure this is safe, we
+abort if it is called.
+
+MozReview-Commit-ID: Kh32tRs2rVC
+
+diff --git a/security/sandbox/chromium/sandbox/win/src/sidestep_resolver.cc b/security/sandbox/chromium/sandbox/win/src/sidestep_resolver.cc
+--- a/security/sandbox/chromium/sandbox/win/src/sidestep_resolver.cc
++++ b/security/sandbox/chromium/sandbox/win/src/sidestep_resolver.cc
+@@ -103,36 +103,37 @@ NTSTATUS SmartSidestepResolverThunk::Set
+     if (!NT_SUCCESS(ret))
+       return ret;
+   }
+ 
+   // Perform a standard sidestep patch on the last part of the thunk, but point
+   // to our internal smart interceptor.
+   size_t standard_bytes = storage_bytes - offsetof(SmartThunk, sidestep);
+   ret = SidestepResolverThunk::Setup(target_module, interceptor_module,
+-                                     target_name, NULL, &SmartStub,
++                                     target_name, NULL, reinterpret_cast<void*>(&SmartStub),
+                                      &thunk->sidestep, standard_bytes, NULL);
+   if (!NT_SUCCESS(ret))
+     return ret;
+ 
+   // Fix the internal thunk to pass the whole buffer to the interceptor.
+   SetInternalThunk(&thunk->sidestep.internal_thunk, GetInternalThunkSize(),
+-                   thunk_storage, &SmartStub);
++                   thunk_storage, reinterpret_cast<void*>(&SmartStub));
+ 
+   if (storage_used)
+     *storage_used = GetThunkSize();
+ 
+   return ret;
+ }
+ 
+ size_t SmartSidestepResolverThunk::GetThunkSize() const {
+   return GetInternalThunkSize() + kSizeOfSidestepStub +
+          offsetof(SmartThunk, sidestep);
+ }
+ 
++
+ // This code must basically either call the intended interceptor or skip the
+ // call and invoke instead the original function. In any case, we are saving
+ // the registers that may be trashed by our c++ code.
+ //
+ // This function is called with a first parameter inserted by us, that points
+ // to our SmartThunk. When we call the interceptor we have to replace this
+ // parameter with the one expected by that function (stored inside our
+ // structure); on the other hand, when we skip the interceptor we have to remove
+@@ -143,16 +144,20 @@ size_t SmartSidestepResolverThunk::GetTh
+ //  [param 2] = first real argument   [param 2] (esp+1c)          [param 2]
+ //  [param 1] = our SmartThunk        [param 1] (esp+18)          [ret address]
+ //  [ret address] = real caller       [ret address] (esp+14)      [xxx]
+ //  [xxx]                             [addr to jump to] (esp+10)  [xxx]
+ //  [xxx]                             [saved eax]                 [xxx]
+ //  [xxx]                             [saved ebx]                 [xxx]
+ //  [xxx]                             [saved ecx]                 [xxx]
+ //  [xxx]                             [saved edx]                 [xxx]
++//
++// This function is not called on MinGW, and causes a compilation error, so
++// we do nothing there.
++#if !defined(__MINGW32__)
+ __declspec(naked)
+ void SmartSidestepResolverThunk::SmartStub() {
+   __asm {
+     push eax                  // Space for the jump.
+     push eax                  // Save registers.
+     push ebx
+     push ecx
+     push edx
+@@ -184,16 +189,22 @@ void SmartSidestepResolverThunk::SmartSt
+     mov [esp + 0x10], ecx
+     pop edx                             // Restore registers.
+     pop ecx
+     pop ebx
+     pop eax
+     ret                                 // Jump to original function.
+   }
+ }
++#else // if __MINGW32__ is defined
++void SmartSidestepResolverThunk::SmartStub() {
++  __debugbreak();
++  abort();
++}
++#endif
+ 
+ bool SmartSidestepResolverThunk::IsInternalCall(const void* base,
+                                                 void* return_address) {
+   DCHECK_NT(base);
+   DCHECK_NT(return_address);
+ 
+   base::win::PEImage pe(base);
+   if (pe.GetImageSectionFromAddr(return_address))
--- a/security/sandbox/chromium-shim/patches/with_update/patch_order.txt
+++ b/security/sandbox/chromium-shim/patches/with_update/patch_order.txt
@@ -12,9 +12,10 @@ fix_Wcomma_warning_in_time_cc.patch
 allow_read_only_all_paths_rule.patch
 revert_TargetNtSetInformationThread_change.patch
 mingw_base_win_get_caller.patch
 mingw_duplicate_instatinations.patch
 mingw_msvc_requirement_error.patch
 mingw_copy_s.patch
 mingw_operator_new.patch
 mingw_cast_getprocaddress.patch
-mingw_fix_narrowing_conversion.patch
\ No newline at end of file
+mingw_fix_narrowing_conversion.patch
+mingw_smartstub.patch
\ No newline at end of file
--- a/security/sandbox/chromium/sandbox/win/src/sidestep_resolver.cc
+++ b/security/sandbox/chromium/sandbox/win/src/sidestep_resolver.cc
@@ -103,36 +103,37 @@ NTSTATUS SmartSidestepResolverThunk::Set
     if (!NT_SUCCESS(ret))
       return ret;
   }
 
   // Perform a standard sidestep patch on the last part of the thunk, but point
   // to our internal smart interceptor.
   size_t standard_bytes = storage_bytes - offsetof(SmartThunk, sidestep);
   ret = SidestepResolverThunk::Setup(target_module, interceptor_module,
-                                     target_name, NULL, &SmartStub,
+                                     target_name, NULL, reinterpret_cast<void*>(&SmartStub),
                                      &thunk->sidestep, standard_bytes, NULL);
   if (!NT_SUCCESS(ret))
     return ret;
 
   // Fix the internal thunk to pass the whole buffer to the interceptor.
   SetInternalThunk(&thunk->sidestep.internal_thunk, GetInternalThunkSize(),
-                   thunk_storage, &SmartStub);
+                   thunk_storage, reinterpret_cast<void*>(&SmartStub));
 
   if (storage_used)
     *storage_used = GetThunkSize();
 
   return ret;
 }
 
 size_t SmartSidestepResolverThunk::GetThunkSize() const {
   return GetInternalThunkSize() + kSizeOfSidestepStub +
          offsetof(SmartThunk, sidestep);
 }
 
+
 // This code must basically either call the intended interceptor or skip the
 // call and invoke instead the original function. In any case, we are saving
 // the registers that may be trashed by our c++ code.
 //
 // This function is called with a first parameter inserted by us, that points
 // to our SmartThunk. When we call the interceptor we have to replace this
 // parameter with the one expected by that function (stored inside our
 // structure); on the other hand, when we skip the interceptor we have to remove
@@ -143,16 +144,20 @@ size_t SmartSidestepResolverThunk::GetTh
 //  [param 2] = first real argument   [param 2] (esp+1c)          [param 2]
 //  [param 1] = our SmartThunk        [param 1] (esp+18)          [ret address]
 //  [ret address] = real caller       [ret address] (esp+14)      [xxx]
 //  [xxx]                             [addr to jump to] (esp+10)  [xxx]
 //  [xxx]                             [saved eax]                 [xxx]
 //  [xxx]                             [saved ebx]                 [xxx]
 //  [xxx]                             [saved ecx]                 [xxx]
 //  [xxx]                             [saved edx]                 [xxx]
+//
+// This function is not called on MinGW, and causes a compilation error, so
+// we do nothing there.
+#if !defined(__MINGW32__)
 __declspec(naked)
 void SmartSidestepResolverThunk::SmartStub() {
   __asm {
     push eax                  // Space for the jump.
     push eax                  // Save registers.
     push ebx
     push ecx
     push edx
@@ -184,16 +189,22 @@ void SmartSidestepResolverThunk::SmartSt
     mov [esp + 0x10], ecx
     pop edx                             // Restore registers.
     pop ecx
     pop ebx
     pop eax
     ret                                 // Jump to original function.
   }
 }
+#else // if __MINGW32__ is defined
+void SmartSidestepResolverThunk::SmartStub() {
+  __debugbreak();
+  abort();
+}
+#endif
 
 bool SmartSidestepResolverThunk::IsInternalCall(const void* base,
                                                 void* return_address) {
   DCHECK_NT(base);
   DCHECK_NT(return_address);
 
   base::win::PEImage pe(base);
   if (pe.GetImageSectionFromAddr(return_address))