Bug 1505482 - Allow DLL patcher to resolve some backward short JMPs. r=aklotz, a=RyanVM
authorDavid Parks <dparks@mozilla.com>
Wed, 26 Dec 2018 17:28:37 +0000
changeset 506432 5e8f96ef4c33
parent 506431 3bfc60606908
child 506433 27ce905c5ac0
push id10411
push userryanvm@gmail.com
push dateMon, 31 Dec 2018 00:36:58 +0000
treeherdermozilla-beta@5e8f96ef4c33 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz, RyanVM
bugs1505482
milestone65.0
Bug 1505482 - Allow DLL patcher to resolve some backward short JMPs. r=aklotz, a=RyanVM In Windows 7 x64, GetFileAttributesW begins with a short, backwards jump that can't safely be converted by the interceptor. Additionally, the function doesn't have enough NOP space after the JMP for the trampoline. However, the target of the short JMP is a long JMP, followed by plenty of NOP space. This patch moves the trampoline location from the first JMP to the second. Differential Revision: https://phabricator.services.mozilla.com/D11258
mozglue/misc/interceptor/PatcherBase.h
mozglue/misc/interceptor/TargetFunction.h
--- a/mozglue/misc/interceptor/PatcherBase.h
+++ b/mozglue/misc/interceptor/PatcherBase.h
@@ -23,30 +23,43 @@ class WindowsDllPatcherBase {
 
   ReadOnlyTargetFunction<MMPolicyT> ResolveRedirectedAddress(
       FARPROC aOriginalFunction) {
     ReadOnlyTargetFunction<MMPolicyT> origFn(mVMPolicy, aOriginalFunction);
     // If function entry is jmp rel8 stub to the internal implementation, we
     // resolve redirected address from the jump target.
     if (origFn[0] == 0xeb) {
       int8_t offset = (int8_t)(origFn[1]);
+      uintptr_t abstarget = origFn.GetAddress() + 2 + offset;
+
+#if defined(_M_X64)
+      // We redirect to the target of a short jump backwards if the target
+      // is another jump (only 32-bit displacement is currently supported).
+      // This case is used by GetFileAttributesW in Win7.
+      if ((offset < 0) && (origFn.IsValidAtOffset(2 + offset))) {
+        ReadOnlyTargetFunction<MMPolicyT> redirectFn(mVMPolicy, abstarget);
+        if ((redirectFn[0] == 0xff) && (redirectFn[1] == 0x25)) {
+          return redirectFn;
+        }
+      }
+#endif
+
       if (offset <= 0) {
         // Bail out for negative offset: probably already patched by some
         // third-party code.
         return std::move(origFn);
       }
 
       for (int8_t i = 0; i < offset; i++) {
         if (origFn[2 + i] != 0x90) {
           // Bail out on insufficient nop space.
           return std::move(origFn);
         }
       }
 
-      uintptr_t abstarget = (origFn + 2 + offset).GetAddress();
       return EnsureTargetIsAccessible(std::move(origFn), abstarget);
     }
 
 #if defined(_M_IX86)
     // If function entry is jmp [disp32] such as used by kernel32,
     // we resolve redirected address from import table.
     if (origFn[0] == 0xff && origFn[1] == 0x25) {
       uintptr_t abstarget = (origFn + 2).template ChasePointer<uintptr_t*>();
--- a/mozglue/misc/interceptor/TargetFunction.h
+++ b/mozglue/misc/interceptor/TargetFunction.h
@@ -677,16 +677,20 @@ class MOZ_STACK_CLASS ReadOnlyTargetFunc
     return reinterpret_cast<uintptr_t>(::EncodePointer(aPtr));
   }
 
   static uintptr_t DecodePtr(uintptr_t aEncodedPtr) {
     return reinterpret_cast<uintptr_t>(
         ::DecodePointer(reinterpret_cast<PVOID>(aEncodedPtr)));
   }
 
+  bool IsValidAtOffset(const int8_t aOffset) const {
+    return mTargetBytes->IsValidAtOffset(aOffset);
+  }
+
   uint8_t const& operator*() const {
     mTargetBytes->EnsureLimit(mOffset);
     return *(mTargetBytes->GetLocalBytes() + mOffset);
   }
 
   uint8_t const& operator[](uint32_t aIndex) const {
     mTargetBytes->EnsureLimit(mOffset + aIndex);
     return *(mTargetBytes->GetLocalBytes() + mOffset + aIndex);