Bug 1655680 - Support JAE rel32 in our detour. r=handyman

After the fix for bug 1642626, we need to detour `KERNELBASE!CloseHandle`
instead of K32's stub, which contains `JAE rel32`.

I also found a mistake in the fix for bug 1642626.  When we put a conditional
jump in a trampoline, we need to reverse a condition, but the JAE case mistakenly
filled JAE straight.  This patch corrects it to filling JB.

Differential Revision: https://phabricator.services.mozilla.com/D85477
This commit is contained in:
Toshihito Kikuchi 2020-08-05 07:21:00 +00:00
parent 4277dac0fe
commit ae04ca7814
3 changed files with 86 additions and 56 deletions

View File

@ -644,6 +644,8 @@ class WindowsDllDetourPatcher final
return !!aTramp;
}
// Write an opposite conditional jump because the destination branches
// are swapped.
if (aType == JumpType::Je) {
// JNE RIP+14
aTramp.WriteByte(0x75);
@ -653,8 +655,8 @@ class WindowsDllDetourPatcher final
aTramp.WriteByte(0x74);
aTramp.WriteByte(14);
} else if (aType == JumpType::Jae) {
// JAE RIP+14
aTramp.WriteByte(0x73);
// JB RIP+14
aTramp.WriteByte(0x72);
aTramp.WriteByte(14);
}
@ -1084,13 +1086,18 @@ class WindowsDllDetourPatcher final
} else {
COPY_CODES(nModRmSibBytes);
}
} else if (*origBytes == 0x84) {
// je rel32
} else if (*origBytes >= 0x83 && *origBytes <= 0x85) {
// 0f 83 cd JAE rel32
// 0f 84 cd JE rel32
// 0f 85 cd JNE rel32
const JumpType kJumpTypes[] = {JumpType::Jae, JumpType::Je,
JumpType::Jne};
auto jumpType = kJumpTypes[*origBytes - 0x83];
++origBytes;
--tramp; // overwrite the 0x0f we copied above
if (!GenerateJump(tramp, origBytes.ReadDisp32AsAbsolute(),
JumpType::Je)) {
jumpType)) {
return;
}
} else {

View File

@ -11,6 +11,24 @@
#ifndef mozilla_AssemblyPayloads_h
#define mozilla_AssemblyPayloads_h
#define PADDING_256_NOP \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;" \
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
extern "C" {
#if defined(__clang__)
@ -43,28 +61,32 @@ __declspec(dllexport) __attribute__((naked)) void DoubleJump() {
"jmpq *%%rax;"
// 0x100 bytes padding to generate jmp rel32 instead of jmp rel8
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
PADDING_256_NOP
"label1:"
"jmp label2;"
:
: "i"(JumpDestination));
}
__declspec(dllexport) __attribute__((naked)) void NearJump() {
asm volatile(
"jae label3;"
"je label3;"
"jne label3;"
"label4:"
"mov %0, %%rax;"
"jmpq *%%rax;"
// 0x100 bytes padding to generate jae rel32 instead of jae rel8
PADDING_256_NOP
"label3:"
"jmp label4;"
:
: "i"(JumpDestination));
}
# elif defined(_M_IX86)
constexpr uintptr_t JumpDestination = 0x7fff0000;
@ -121,22 +143,7 @@ __declspec(dllexport) __attribute__((naked)) void DoubleJump() {
"jmp *%%eax;"
// 0x100 bytes padding to generate jmp rel32 instead of jmp rel8
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
"nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;nop;"
PADDING_256_NOP
"label1:"
"jmp label2;"

View File

@ -706,15 +706,16 @@ bool TestShortDetour() {
#endif
}
template <typename InterceptorType>
bool TestAssemblyFunctions() {
constexpr uintptr_t NoStubAddressCheck = 0;
struct TestCase {
const char* functionName;
uintptr_t expectedStub;
explicit TestCase(const char* aFunctionName, uintptr_t aExpectedStub)
: functionName(aFunctionName), expectedStub(aExpectedStub) {}
} testCases[] = {
constexpr uintptr_t NoStubAddressCheck = 0;
struct TestCase {
const char* mFunctionName;
uintptr_t mExpectedStub;
bool mPatchedOnce;
explicit TestCase(const char* aFunctionName, uintptr_t aExpectedStub)
: mFunctionName(aFunctionName),
mExpectedStub(aExpectedStub),
mPatchedOnce(false) {}
} g_AssemblyTestCases[] = {
#if defined(__clang__)
// We disable these testcases because the code coverage instrumentation injects
// code in a way that WindowsDllInterceptor doesn't understand.
@ -725,6 +726,10 @@ bool TestAssemblyFunctions() {
TestCase("MovPushRet", JumpDestination),
TestCase("MovRaxJump", JumpDestination),
TestCase("DoubleJump", JumpDestination),
// Passing NoStubAddressCheck as the following testcases return
// a trampoline address instead of the original destination.
TestCase("NearJump", NoStubAddressCheck),
# elif defined(_M_IX86)
// Skip the stub address check as we always generate a trampoline for x86.
TestCase("PushRet", NoStubAddressCheck),
@ -736,49 +741,60 @@ bool TestAssemblyFunctions() {
# endif
# endif // MOZ_CODE_COVERAGE
#endif // defined(__clang__)
};
};
template <typename InterceptorType>
bool TestAssemblyFunctions() {
static const auto patchedFunction = []() { patched_func_called = true; };
InterceptorType interceptor;
interceptor.Init("TestDllInterceptor.exe");
for (const auto& testCase : testCases) {
for (auto& testCase : g_AssemblyTestCases) {
if (testCase.mExpectedStub == NoStubAddressCheck && testCase.mPatchedOnce) {
// For the testcases with NoStubAddressCheck, we revert a hook by
// jumping into the original stub, which is not detourable again.
continue;
}
typename InterceptorType::template FuncHookType<void (*)()> hook;
bool result = hook.Set(interceptor, testCase.functionName, patchedFunction);
bool result =
hook.Set(interceptor, testCase.mFunctionName, patchedFunction);
if (!result) {
printf(
"TEST-FAILED | WindowsDllInterceptor | "
"Failed to detour %s.\n",
testCase.functionName);
testCase.mFunctionName);
return false;
}
testCase.mPatchedOnce = true;
const auto actualStub = reinterpret_cast<uintptr_t>(hook.GetStub());
if (testCase.expectedStub != NoStubAddressCheck &&
actualStub != testCase.expectedStub) {
if (testCase.mExpectedStub != NoStubAddressCheck &&
actualStub != testCase.mExpectedStub) {
printf(
"TEST-FAILED | WindowsDllInterceptor | "
"Wrong stub was backed up for %s: %zx\n",
testCase.functionName, actualStub);
testCase.mFunctionName, actualStub);
return false;
}
patched_func_called = false;
auto originalFunction = reinterpret_cast<void (*)()>(
GetProcAddress(GetModuleHandle(nullptr), testCase.functionName));
GetProcAddress(GetModuleHandle(nullptr), testCase.mFunctionName));
originalFunction();
if (!patched_func_called) {
printf(
"TEST-FAILED | WindowsDllInterceptor | "
"Hook from %s was not called\n",
testCase.functionName);
testCase.mFunctionName);
return false;
}
printf("TEST-PASS | WindowsDllInterceptor | %s\n", testCase.functionName);
printf("TEST-PASS | WindowsDllInterceptor | %s\n", testCase.mFunctionName);
}
return true;