Bug 1505482: Allow DLL patcher to resolve some backward short JMPs (r=aklotz)
authorDavid Parks <dparks@mozilla.com>
Wed, 26 Dec 2018 17:28:37 +0000
changeset 451951 6150c08114a8
parent 451950 330193fc829f
child 451952 27a4dfa5c033
child 451964 ad6f51d4af0b
push id75265
push usercsabou@mozilla.com
push dateWed, 26 Dec 2018 18:47:51 +0000
treeherderautoland@6150c08114a8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz
bugs1505482
milestone66.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 1505482: Allow DLL patcher to resolve some backward short JMPs (r=aklotz) 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);