commit 65a9e94015b6f0f7c9577baf2f10b13bfcbe1890
parent 4492b391756a982d466fbdfcd328041861e9b314
Author: Greg Stoll <gstoll@mozilla.com>
Date: Tue, 28 Oct 2025 15:19:16 +0000
Bug 1919213 part 3 - avoid jumps and calls inside 13 bytes r=yjuglaret,win-reviewers,handyman
If there is a jump or call inside the first 13 bytes of the function that
we're detouring, we don't handle that case correctly. This should be very
unlikely, but now we will fail to detour. Also fixes up a test that was
attempting to do this.
Differential Revision: https://phabricator.services.mozilla.com/D268653
Diffstat:
3 files changed, 60 insertions(+), 13 deletions(-)
diff --git a/toolkit/xre/dllservices/mozglue/interceptor/PatcherDetour.h b/toolkit/xre/dllservices/mozglue/interceptor/PatcherDetour.h
@@ -667,7 +667,17 @@ class WindowsDllDetourPatcher final
enum class JumpType { Je, Jne, Jae, Jmp, Call, Js };
static bool GenerateJump(Trampoline<MMPolicyT>& aTramp,
- uintptr_t aAbsTargetAddress, const JumpType aType) {
+ uintptr_t aAbsTargetAddress, const JumpType aType,
+ const ReadOnlyTargetFunction<MMPolicyT>& origBytes,
+ uint32_t aBytesToPatch) {
+ if (aAbsTargetAddress >= origBytes.GetBaseAddress() &&
+ aAbsTargetAddress < origBytes.GetBaseAddress() + aBytesToPatch) {
+ // We don't correctly detour jumps or calls with target inside the section
+ // that we are patching. We don't think we need this for any realistic
+ // use case, so just fail here.
+ return false;
+ }
+
// Near call, absolute indirect, address given in r/m32
if (aType == JumpType::Call) {
// CALL [RIP+0]
@@ -1176,8 +1186,8 @@ class WindowsDllDetourPatcher final
++origBytes;
--tramp; // overwrite the 0x0f we copied above
- if (!GenerateJump(tramp, origBytes.ReadDisp32AsAbsolute(),
- jumpType)) {
+ if (!GenerateJump(tramp, origBytes.ReadDisp32AsAbsolute(), jumpType,
+ origBytes, bytesRequired)) {
return;
}
} else if (*origBytes == 0x88) {
@@ -1186,7 +1196,7 @@ class WindowsDllDetourPatcher final
--tramp; // overwrite the 0x0f we copied above
if (!GenerateJump(tramp, origBytes.ReadDisp32AsAbsolute(),
- JumpType::Js)) {
+ JumpType::Js, origBytes, bytesRequired)) {
return;
}
} else {
@@ -1367,7 +1377,8 @@ class WindowsDllDetourPatcher final
foundJmp = (origBytes[1] & 0x38) == 0x20;
if (!GenerateJump(tramp, origBytes.ChasePointerFromDisp(),
- foundJmp ? JumpType::Jmp : JumpType::Call)) {
+ foundJmp ? JumpType::Jmp : JumpType::Call,
+ origBytes, bytesRequired)) {
return;
}
} else {
@@ -1539,7 +1550,8 @@ class WindowsDllDetourPatcher final
++origBytes;
if (!GenerateJump(tramp, origBytes.ReadDisp32AsAbsolute(),
- foundJmp ? JumpType::Jmp : JumpType::Call)) {
+ foundJmp ? JumpType::Jmp : JumpType::Call, origBytes,
+ bytesRequired)) {
return;
}
} else if (*origBytes >= 0x73 && *origBytes <= 0x75) {
@@ -1560,8 +1572,8 @@ class WindowsDllDetourPatcher final
origBytes += 2;
- if (!GenerateJump(tramp, origBytes.OffsetToAbsolute(offset),
- jumpType)) {
+ if (!GenerateJump(tramp, origBytes.OffsetToAbsolute(offset), jumpType,
+ origBytes, bytesRequired)) {
return;
}
} else if (*origBytes == 0x78) {
@@ -1578,7 +1590,7 @@ class WindowsDllDetourPatcher final
origBytes += 2;
if (!GenerateJump(tramp, origBytes.OffsetToAbsolute(offset),
- JumpType::Js)) {
+ JumpType::Js, origBytes, bytesRequired)) {
return;
}
} else if (*origBytes == 0xff) {
@@ -1593,7 +1605,7 @@ class WindowsDllDetourPatcher final
// FF 15 CALL [disp32]
origBytes += 2;
if (!GenerateJump(tramp, origBytes.ChasePointerFromDisp(),
- JumpType::Call)) {
+ JumpType::Call, origBytes, bytesRequired)) {
return;
}
} else if (reg == 4) {
@@ -1606,7 +1618,8 @@ class WindowsDllDetourPatcher final
uintptr_t jmpDest = origBytes.ChasePointerFromDisp();
- if (!GenerateJump(tramp, jmpDest, JumpType::Jmp)) {
+ if (!GenerateJump(tramp, jmpDest, JumpType::Jmp, origBytes,
+ bytesRequired)) {
return;
}
} else {
@@ -1710,7 +1723,8 @@ class WindowsDllDetourPatcher final
// if we found a _conditional_ jump or a CALL (or no control operations
// at all) then we still need to run the rest of aOriginalFunction.
if (!foundJmp) {
- if (!GenerateJump(tramp, origBytes.GetAddress(), JumpType::Jmp)) {
+ if (!GenerateJump(tramp, origBytes.GetAddress(), JumpType::Jmp, origBytes,
+ bytesRequired)) {
return;
}
}
diff --git a/toolkit/xre/dllservices/tests/AssemblyPayloads.h b/toolkit/xre/dllservices/tests/AssemblyPayloads.h
@@ -166,6 +166,34 @@ __declspec(dllexport) MOZ_NAKED void RexCmpRipRelativeBytePtr() {
"nop;nop;nop;nop;nop;nop;nop;nop;");
}
+__declspec(dllexport) MOZ_NAKED void JmpInsideEarlyBytes() {
+ asm volatile(
+ "js label9;"
+ "label10:"
+ "jmpq *%%rax;"
+
+ // "label9" is within the first 13 bytes
+ "label9:"
+ "mov %0, %%rax;"
+ "jmp label10;"
+ :
+ : "i"(JumpDestination));
+}
+
+__declspec(dllexport) MOZ_NAKED void CallInsideEarlyBytes() {
+ asm volatile(
+ "call label11;"
+ "label12:"
+ "jmpq *%%rax;"
+
+ // "label11" is within the first 13 bytes
+ "label11:"
+ "mov %0, %%rax;"
+ "jmp label12;"
+ :
+ : "i"(JumpDestination));
+}
+
// A valid function that uses "cmp byte ptr [rip + offset], sil". It returns
// true if and only if gGlobalValue is equal to aValue.
__declspec(dllexport) MOZ_NAKED bool IsEqualToGlobalValue(
@@ -192,11 +220,14 @@ MOZ_NAKED void DetouredCallCode(uintptr_t aCallee) {
"testq %rcx, %rcx;"
"jz exit;"
"callq *%rcx;"
+ // Add padding here so we're not trying to jump within the
+ // first 13 bytes of the function
+ "nop;nop;nop;nop;nop;nop;nop;nop;"
"exit:"
"addq $0x28, %rsp;"
"retq;");
}
-constexpr uint8_t gDetouredCallCodeSize = 16; // size of function in bytes
+constexpr uint8_t gDetouredCallCodeSize = 24; // size of function in bytes
alignas(uint32_t) uint8_t gDetouredCallUnwindInfo[] = {
0x01, // Version (1), Flags (0)
0x04, // SizeOfProlog (4)
diff --git a/toolkit/xre/dllservices/tests/TestDllInterceptor.cpp b/toolkit/xre/dllservices/tests/TestDllInterceptor.cpp
@@ -836,6 +836,8 @@ MOZ_GLOBINIT struct TestCase {
TestCase("IndirectCall", NoStubAddressCheck),
TestCase("MovImm64", NoStubAddressCheck),
TestCase("RexCmpRipRelativeBytePtr", NoStubAddressCheck),
+ TestCase("JmpInsideEarlyBytes", ExpectedFail),
+ TestCase("CallInsideEarlyBytes", ExpectedFail),
# elif defined(_M_IX86)
// Skip the stub address check as we always generate a trampoline for x86.
TestCase("PushRet", NoStubAddressCheck,
