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 509204 5e8f96ef4c33d5e17c7affc20405bf234bfe2fc8
parent 509203 3bfc606069080981fb11574b18ac5101993b0338
child 509205 27ce905c5ac017098d33c3e402e974bc8d4444c6
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [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);