[sanitizers][windows] Correctly override functions with backward jmps

To reproduce: Download and run the latest Firefox ASAN build (https://firefox-ci-tc.services.mozilla.com/api/index/v1/task/gecko.v2.mozilla-central.latest.firefox.win64-asan-opt/artifacts/public/build/target.zip) on Windows 11 (version 10.0.22621 Build 22621); it will crash on launch. Note that this doesn't seem to crash on another Windows 11 VM I've tried, so I'm not sure how reproducible it is across machines, but it reproduces on my machine every time.

The problem seems to be that when overriding the memset function in OverrideFunctionWithRedirectJump(), the relative_offset is stored as a uptr. Per the Intel x64 instruction set reference (https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-instruction-set-reference-manual-325383.pdf - warning: large PDF), on page 646 the jmp instruction (specifically the near jump flavors that start with E9, which are the ones the OverrideFunctionWithRedirectJump() considers) treats the offset as a signed displacement. This causes an incorrect value to be stored for REAL(memset) which points to uninitialized memory, and a crash the next time that gets called.

The fix is to simply treat that offset as signed. I have also added a test case.

Fixes llvm#58846

Differential Revision: https://reviews.llvm.org/D137788

Author: zero9178

compiler-rt/lib/interception/interception_win.cpp

@@ -738,7 +738,7 @@ bool OverrideFunctionWithRedirectJump(
     return false;
 
   if (orig_old_func) {
-    uptr relative_offset = *(u32*)(old_func + 1);
+    sptr relative_offset = *(s32 *)(old_func + 1);
     uptr absolute_target = old_func + relative_offset + kJumpInstructionLength;
     *orig_old_func = absolute_target;
   }

compiler-rt/lib/interception/tests/interception_win_test.cpp

@@ -85,7 +85,16 @@ const u8 kIdentityCodeWithJump[] = {
     0xC3,                   // ret
 };
 
-#else
+const u8 kIdentityCodeWithJumpBackwards[] = {
+    0x89, 0xC8,  // mov         eax, ecx
+    0xC3,        // ret
+    0xE9, 0xF8, 0xFF, 0xFF,
+    0xFF,  // jmp - 8
+    0xCC, 0xCC, 0xCC, 0xCC,
+};
+const u8 kIdentityCodeWithJumpBackwardsOffset = 3;
+
+#    else
 
 const u8 kIdentityCodeWithPrologue[] = {
     0x55,                   // push        ebp
@@ -134,7 +143,16 @@ const u8 kIdentityCodeWithJump[] = {
     0xC3,                   // ret
 };
 
-#endif
+const u8 kIdentityCodeWithJumpBackwards[] = {
+    0x8B, 0x44, 0x24, 0x04,  // mov         eax,dword ptr [esp + 4]
+    0xC3,                    // ret
+    0xE9, 0xF6, 0xFF, 0xFF,
+    0xFF,  // jmp - 10
+    0xCC, 0xCC, 0xCC, 0xCC,
+};
+const u8 kIdentityCodeWithJumpBackwardsOffset = 5;
+
+#    endif
 
 const u8 kPatchableCode1[] = {
     0xB8, 0x4B, 0x00, 0x00, 0x00,   // mov eax,4B
@@ -366,13 +384,14 @@ TEST(Interception, InternalGetProcAddress) {
   EXPECT_NE(DbgPrint_adddress, isdigit_address);
 }
 
-template<class T>
+template <class T>
 static void TestIdentityFunctionPatching(
-    const T &code,
-    TestOverrideFunction override,
-    FunctionPrefixKind prefix_kind = FunctionPrefixNone) {
+    const T &code, TestOverrideFunction override,
+    FunctionPrefixKind prefix_kind = FunctionPrefixNone,
+    int function_start_offset = 0) {
   uptr identity_address;
   LoadActiveCode(code, &identity_address, prefix_kind);
+  identity_address += function_start_offset;
   IdentityFunction identity = (IdentityFunction)identity_address;
 
   // Validate behavior before dynamic patching.
@@ -410,7 +429,7 @@ static void TestIdentityFunctionPatching(
   TestOnlyReleaseTrampolineRegions();
 }
 
-#if !SANITIZER_WINDOWS64
+#    if !SANITIZER_WINDOWS64
 TEST(Interception, OverrideFunctionWithDetour) {
   TestOverrideFunction override = OverrideFunctionWithDetour;
   FunctionPrefixKind prefix = FunctionPrefixDetour;
@@ -424,6 +443,9 @@ TEST(Interception, OverrideFunctionWithDetour) {
 TEST(Interception, OverrideFunctionWithRedirectJump) {
   TestOverrideFunction override = OverrideFunctionWithRedirectJump;
   TestIdentityFunctionPatching(kIdentityCodeWithJump, override);
+  TestIdentityFunctionPatching(kIdentityCodeWithJumpBackwards, override,
+                               FunctionPrefixNone,
+                               kIdentityCodeWithJumpBackwardsOffset);
 }
 
 TEST(Interception, OverrideFunctionWithHotPatch) {