From 058069d1f39ae43945db227d766bfc17aa5cba00 Mon Sep 17 00:00:00 2001 From: Lysann Tranvouez Date: Thu, 2 Oct 2025 22:18:10 +0200 Subject: [PATCH] more tests and tweaks/fixes as a result --- src/arm64/detours_arm64.h | 1 + src/mach_detours.c | 56 +++++++++++------- tests/CMakeLists.txt | 4 +- tests/test.cpp | 79 ------------------------- tests/test_dylib_function.cpp | 108 ++++++++++++++++++++++++++++++++++ tests/test_local_function.cpp | 47 +++++++++++++++ 6 files changed, 194 insertions(+), 101 deletions(-) delete mode 100644 tests/test.cpp create mode 100644 tests/test_dylib_function.cpp create mode 100644 tests/test_local_function.cpp diff --git a/src/arm64/detours_arm64.h b/src/arm64/detours_arm64.h index 36c4f92..876ca75 100644 --- a/src/arm64/detours_arm64.h +++ b/src/arm64/detours_arm64.h @@ -7,6 +7,7 @@ #include "detours_internal.h" +#include #include #include diff --git a/src/mach_detours.c b/src/mach_detours.c index 9cd3c9c..24ef035 100644 --- a/src/mach_detours.c +++ b/src/mach_detours.c @@ -44,16 +44,20 @@ static detour_region* s_default_region = nullptr; static mach_error_t internal_detour_writable_trampoline_regions() { + mach_error_t result = err_none; + // Mark all the regions as writable. const mach_port_t port = mach_task_self(); for (detour_region* pRegion = s_regions_head; pRegion != NULL; pRegion = pRegion->next) { const mach_error_t error = mach_vm_protect(port, (mach_vm_address_t)pRegion, DETOUR_REGION_SIZE, false, VM_PROT_READ | VM_PROT_WRITE); if (error != err_none) { - return error; + DETOUR_BREAK(); + result = error; } + DETOUR_TRACE(("mach_vm_protect(%p, %" PRIu32 ", %d)\n", (void*)pRegion, DETOUR_REGION_SIZE, VM_PROT_READ | VM_PROT_WRITE)); } - return err_none; + return result; } static void internal_detour_runnable_trampoline_regions() @@ -66,6 +70,7 @@ static void internal_detour_runnable_trampoline_regions() if (error != err_none) { DETOUR_BREAK(); } + DETOUR_TRACE(("mach_vm_protect(%p, %" PRIu32 ", %d)\n", (void*)pRegion, DETOUR_REGION_SIZE, VM_PROT_READ | VM_PROT_EXECUTE)); } } @@ -96,7 +101,7 @@ static void* internal_detour_alloc_region_from_lo(const uint8_t* lo, const uint8 const vm_map_t task_self = mach_task_self(); for (vm_address_t page = (vm_address_t)try_addr; page < (vm_address_t)hi; page += PAGE_SIZE) { - DETOUR_TRACE((" Try %p\n", (void*)page)); + //DETOUR_TRACE((" Try %p\n", (void*)page)); const mach_error_t err = vm_allocate(task_self, &page, DETOUR_REGION_SIZE, 0); if (err == err_none) { @@ -121,7 +126,7 @@ static void* internal_detour_alloc_region_from_hi(const uint8_t* lo, const uint8 const vm_map_t task_self = mach_task_self(); for (vm_address_t page = try_addr; page > (vm_address_t)lo; page -= PAGE_SIZE) { - DETOUR_TRACE((" Try %p\n", (void*)page)); + //DETOUR_TRACE((" Try %p\n", (void*)page)); if ((void*)page >= s_system_region_lower_bound && (void*)page <= s_system_region_upper_bound) { // Skip region reserved for system DLLs, but preserve address space entropy. try_addr -= 0x08000000; @@ -339,6 +344,9 @@ mach_error_t detour_transaction_begin() mach_error_t detour_transaction_abort() { + if (s_transaction_thread == THREAD_NULL) { + return err_none; + } if (s_transaction_thread != mach_thread_self()) { return detour_err_wrong_thread; } @@ -349,6 +357,7 @@ mach_error_t detour_transaction_abort() DETOUR_CHECK( mach_vm_protect(port, (mach_vm_address_t)operation->target, operation->trampoline->restore_code_size, false, operation->perm)); + DETOUR_TRACE(("mach_vm_protect(%p, %" PRIu8 ", %d)\n", (void*)operation->target, operation->trampoline->restore_code_size, operation->perm)); if (operation->kind == detour_operation_kind_attach) { if (operation->trampoline) { @@ -422,7 +431,7 @@ mach_error_t detour_transaction_commit_ex(detour_func_t** out_failed_target) operation->target[8], operation->target[9], operation->target[10], operation->target[11])); detour_platform_operation_commit_detour(operation); - *operation->pointer = detour_platform_operation_get_trampoline_ptr(operation) + *operation->pointer = detour_platform_operation_get_trampoline_ptr(operation); DETOUR_TRACE(("detours: target=%p: " "%02x %02x %02x %02x " @@ -467,6 +476,7 @@ mach_error_t detour_transaction_commit_ex(detour_func_t** out_failed_target) DETOUR_CHECK( mach_vm_protect(port, (mach_vm_address_t)operation->target, operation->trampoline->restore_code_size, false, operation->perm)); + DETOUR_TRACE(("mach_vm_protect(%p, %" PRIu8 ", %d)\n", (void*)operation->target, operation->trampoline->restore_code_size, operation->perm)); if (operation->kind == detour_operation_kind_detach && operation->trampoline) { internal_detour_free_trampoline(operation->trampoline); @@ -675,10 +685,10 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour const uint8_t* src = target; uint8_t* trampoline_code = trampoline->code; uint8_t* trampoline_code_limit = trampoline_code + sizeof(trampoline->code); - uint32_t offset_target = 0; + uint32_t target_override_len = 0; uint32_t align_idx = 0; - while (offset_target < DETOUR_PLATFORM_SIZE_OF_JMP) { + while (target_override_len < DETOUR_PLATFORM_SIZE_OF_JMP) { const uint8_t* curr_op = src; uint32_t extra_len = 0; @@ -686,8 +696,8 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour src = internal_detour_copy_instruction(trampoline_code, src, &extra_len); DETOUR_TRACE((" after: src=%p (copied %d bytes)\n", src, (int)(src - curr_op))); trampoline_code += (src - curr_op) + extra_len; - offset_target = (int32_t)(src - target); - trampoline->align[align_idx].offset_target = offset_target; + target_override_len = (int32_t)(src - target); + trampoline->align[align_idx].offset_target = target_override_len; trampoline->align[align_idx].offset_trampoline = trampoline_code - trampoline->code; align_idx++; @@ -701,14 +711,14 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour } // Consume, but don't duplicate padding if it is needed and available. - while (offset_target < DETOUR_PLATFORM_SIZE_OF_JMP) { + while (target_override_len < DETOUR_PLATFORM_SIZE_OF_JMP) { const uint32_t len_filler = detour_platform_is_code_filler(src); if (len_filler == 0) { break; } src += len_filler; - offset_target = (int32_t)(src - target); + target_override_len = (int32_t)(src - target); } #if DETOUR_DEBUG @@ -725,7 +735,7 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour } #endif - if (offset_target < DETOUR_PLATFORM_SIZE_OF_JMP || align_idx > ARRAYSIZE(trampoline->align)) { + if (target_override_len < DETOUR_PLATFORM_SIZE_OF_JMP || align_idx > ARRAYSIZE(trampoline->align)) { // Too few instructions. error = detour_err_too_small; if (s_ignore_too_small) { @@ -741,17 +751,17 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour } trampoline->code_size = (uint8_t)(trampoline_code - trampoline->code); - trampoline->restore_code_size = (uint8_t)offset_target; - memcpy(trampoline->restore_code, target, offset_target); + trampoline->restore_code_size = (uint8_t)target_override_len; + memcpy(trampoline->restore_code, target, target_override_len); - if (offset_target > sizeof(trampoline->code) - DETOUR_PLATFORM_SIZE_OF_JMP) { + if (target_override_len > sizeof(trampoline->code) - DETOUR_PLATFORM_SIZE_OF_JMP) { // Too many instructions. error = detour_err_too_large; DETOUR_BREAK(); goto fail; } - trampoline->ptr_remain = target + offset_target; + trampoline->ptr_remain = target + target_override_len; trampoline->ptr_detour = (uint8_t*)detour; trampoline_code = trampoline->code + trampoline->code_size; @@ -762,7 +772,7 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour UNUSED_VARIABLE(trampoline_code); const mach_port_t port = mach_task_self(); - const mach_vm_address_t page_addr = internal_detour_round_down_to_page((uintptr_t)target); + //const mach_vm_address_t page_addr = internal_detour_round_down_to_page((uintptr_t)target); vm_region_submap_short_info_data_64_t region_info; { @@ -779,11 +789,12 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour } const vm_prot_t old_perm = region_info.protection; - error = mach_vm_protect(port, page_addr, PAGE_SIZE, false, VM_PROT_READ | VM_PROT_WRITE | VM_PROT_COPY); + error = mach_vm_protect(port, (mach_vm_address_t)target, target_override_len, false, VM_PROT_READ | VM_PROT_WRITE | VM_PROT_COPY); if (error != err_none) { DETOUR_BREAK(); goto fail; } + DETOUR_TRACE(("mach_vm_protect(%p, %" PRIu32 ", %d)\n", (void*)target, target_override_len, VM_PROT_READ | VM_PROT_WRITE | VM_PROT_COPY)); DETOUR_TRACE(("detours: target=%p: " "%02x %02x %02x %02x " @@ -793,7 +804,7 @@ mach_error_t detour_attach_ex(detour_func_t* inout_pointer, detour_func_t detour target[0], target[1], target[2], target[3], target[4], target[5], target[6], target[7], target[8], target[9], target[10], target[11])); - DETOUR_TRACE(("detours: trampoline =%p: " + DETOUR_TRACE(("detours: trampoline=%p: " "%02x %02x %02x %02x " "%02x %02x %02x %02x " "%02x %02x %02x %02x\n", @@ -860,7 +871,7 @@ mach_error_t detour_detach(detour_func_t* inout_pointer, detour_func_t detour) detour = detour_platform_skip_jmp(detour); // Verify that Trampoline is in place. - const int32_t restore_code_size = trampoline->restore_code_size; + const uint32_t restore_code_size = trampoline->restore_code_size; uint8_t* target = trampoline->ptr_remain - restore_code_size; if (restore_code_size == 0 || restore_code_size > sizeof(trampoline->code)) { error = KERN_FAILURE; @@ -899,12 +910,13 @@ mach_error_t detour_detach(detour_func_t* inout_pointer, detour_func_t detour) } const vm_prot_t old_perm = region_info.protection; - error = mach_vm_protect(port, (mach_vm_address_t)target, PAGE_SIZE, false, + error = mach_vm_protect(port, (mach_vm_address_t)target, restore_code_size, false, VM_PROT_READ | VM_PROT_WRITE | VM_PROT_COPY); if (error != err_none) { DETOUR_BREAK(); goto fail; } + DETOUR_TRACE(("mach_vm_protect(%p, %" PRIu32 ", %d)\n", (void*)target, restore_code_size, VM_PROT_READ | VM_PROT_WRITE | VM_PROT_COPY)); op->kind = detour_operation_kind_detach; op->pointer = (uint8_t**)inout_pointer; @@ -926,6 +938,7 @@ mach_error_t detour_attach_and_commit_ex(detour_func_t* inout_pointer, detour_fu { const mach_error_t error = detour_attach_ex(inout_pointer, detour, out_real_trampoline, out_real_target, out_real_detour); if (error != err_none) { + detour_transaction_abort(); return error; } return detour_transaction_commit(); @@ -935,6 +948,7 @@ mach_error_t detour_detach_and_commit(detour_func_t* inout_pointer, detour_func_ { const mach_error_t error = detour_detach(inout_pointer, detour); if (error != err_none) { + detour_transaction_abort(); return error; } return detour_transaction_commit(); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4063cfc..006fb39 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,7 +1,9 @@ # Copyright (c) Lysann Tranvouez. All rights reserved. add_executable(mach_detours_tests - test.cpp) + test_dylib_function.cpp + test_local_function.cpp +) # The target function must be in a shared library because otherwise it might be in the same code page as the test.cpp functions. # Between attach and commit the target function's code page is not executable, which can mean our test driver code would not diff --git a/tests/test.cpp b/tests/test.cpp deleted file mode 100644 index cf8ec4a..0000000 --- a/tests/test.cpp +++ /dev/null @@ -1,79 +0,0 @@ -// Copyright (c) Lysann Tranvouez. All rights reserved. - -#include - -#include - -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - -#include "lib_function.h" -int (*realLibFunction)() = libFunction; - -static int libFunctionDetourCounter = 0; -int libFunctionDetour() -{ - libFunctionDetourCounter++; - return 94; -} - -TEST_CASE( "Overriding custom function in dylib" ) -{ - libFunctionCounter = 0; - libFunctionDetourCounter = 0; - - REQUIRE( libFunction() == 42 ); - REQUIRE( libFunctionCounter == 1 ); - REQUIRE( libFunctionDetourCounter == 0 ); - - CHECK( detour_transaction_begin() == err_none ); - CHECK( detour_attach(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); - CHECK( detour_transaction_commit() == err_none ); - - REQUIRE( realLibFunction != libFunction ); - - REQUIRE( libFunctionCounter == 1 ); - REQUIRE( libFunctionDetourCounter == 0 ); - REQUIRE( libFunction() == 94 ); - REQUIRE( libFunctionCounter == 1 ); - REQUIRE( libFunctionDetourCounter == 1 ); -} - -//////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - -static int localFunctionCounter = 0; -int localFunction() -{ - localFunctionCounter++; - return 67; -} -static int localFunctionDetourCounter = 0; -int localFunctionDetour() -{ - localFunctionDetourCounter++; - return 12; -} - -TEST_CASE( "Overriding local function" ) -{ - int (*realFunction)() = localFunction; - localFunctionCounter = 0; - localFunctionDetourCounter = 0; - - REQUIRE( localFunction() == 67 ); - REQUIRE( localFunctionCounter == 1 ); - REQUIRE( localFunctionDetourCounter == 0 ); - - CHECK( detour_transaction_begin() == err_none ); - // Overriding a local function requires using detour_attach_and_commit instead of calling attach and commit individually. - // Otherwise when we return from attach (and before commit), the code page with the local function is marked as read+write (but _not_ executable). - // There is a good chance the code we return to (in this case the test function) is on the same memory page and can therefore not be executed (until we call commit). - CHECK( detour_attach_and_commit(reinterpret_cast(&realFunction), reinterpret_cast(localFunctionDetour)) == err_none ); - - REQUIRE( realFunction != localFunction ); - - REQUIRE( localFunctionCounter == 1 ); - REQUIRE( localFunctionDetourCounter == 0 ); - REQUIRE( localFunction() == 12 ); - REQUIRE( localFunctionCounter == 1 ); - REQUIRE( localFunctionDetourCounter == 1 ); -} diff --git a/tests/test_dylib_function.cpp b/tests/test_dylib_function.cpp new file mode 100644 index 0000000..1cf7cf0 --- /dev/null +++ b/tests/test_dylib_function.cpp @@ -0,0 +1,108 @@ +// Copyright (c) Lysann Tranvouez. All rights reserved. + +#include + +#include + +#include "lib_function.h" +int (*realLibFunction)() = libFunction; + +static int libFunctionDetourCounter = 0; +int libFunctionDetour() +{ + libFunctionDetourCounter++; + return 94; +} + +TEST_CASE( "Overriding custom function in dylib", "[dylib]" ) +{ + libFunctionCounter = 0; + libFunctionDetourCounter = 0; + + SECTION( "attaching installs a detour" ) + { + REQUIRE( realLibFunction == libFunction ); + REQUIRE( libFunction() == 42 ); + REQUIRE( libFunctionCounter == 1 ); + REQUIRE( libFunctionDetourCounter == 0 ); + + REQUIRE( detour_transaction_begin() == err_none ); + + SECTION( "attach + transaction_commit" ) + { + REQUIRE( detour_attach(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); + CHECK( detour_transaction_commit() == err_none ); + } + SECTION( "attach_and_commit" ) + { + CHECK( detour_attach_and_commit(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); + } + + CHECK( realLibFunction != libFunction ); + CHECK( libFunctionCounter == 1 ); + CHECK( libFunctionDetourCounter == 0 ); + CHECK( libFunction() == 94 ); + CHECK( libFunctionCounter == 1 ); + CHECK( libFunctionDetourCounter == 1 ); + + // clean up + REQUIRE( detour_transaction_begin() == err_none ); + CHECK( detour_detach_and_commit(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); + } + + SECTION( "detaching in separate transaction removes detour" ) + { + REQUIRE( detour_transaction_begin() == err_none ); + REQUIRE( detour_attach_and_commit(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); + REQUIRE( realLibFunction != libFunction ); + + REQUIRE( detour_transaction_begin() == err_none ); + + SECTION( "detach + transaction_commit" ) + { + REQUIRE( detour_detach(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); + CHECK( detour_transaction_commit() == err_none ); + } + SECTION( "detach_and_commit" ) + { + CHECK( detour_detach_and_commit(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); + } + + CHECK( realLibFunction == libFunction ); + CHECK( libFunctionCounter == 0 ); + CHECK( libFunctionDetourCounter == 0 ); + CHECK( libFunction() == 42 ); + CHECK( libFunctionCounter == 1 ); + CHECK( libFunctionDetourCounter == 0 ); + } + + SECTION( "aborting transaction means no detour" ) + { + REQUIRE( detour_transaction_begin() == err_none ); + REQUIRE( detour_attach(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); + CHECK( detour_transaction_abort() == err_none ); + + CHECK( realLibFunction == libFunction ); + CHECK( libFunctionCounter == 0 ); + CHECK( libFunctionDetourCounter == 0 ); + CHECK( libFunction() == 42 ); + CHECK( libFunctionCounter == 1 ); + CHECK( libFunctionDetourCounter == 0 ); + } + + SECTION( "an error in a transaction means no detour" ) + { + REQUIRE( detour_transaction_begin() == err_none ); + REQUIRE( detour_attach(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == err_none ); + // cannot detach because trampoline is not yet in place + CHECK( detour_detach(reinterpret_cast(&realLibFunction), reinterpret_cast(libFunctionDetour)) == KERN_FAILURE ); + CHECK( detour_transaction_commit() == KERN_FAILURE ); + + CHECK( realLibFunction == libFunction ); + CHECK( libFunctionCounter == 0 ); + CHECK( libFunctionDetourCounter == 0 ); + CHECK( libFunction() == 42 ); + CHECK( libFunctionCounter == 1 ); + CHECK( libFunctionDetourCounter == 0 ); + } +} diff --git a/tests/test_local_function.cpp b/tests/test_local_function.cpp new file mode 100644 index 0000000..97af062 --- /dev/null +++ b/tests/test_local_function.cpp @@ -0,0 +1,47 @@ +// Copyright (c) Lysann Tranvouez. All rights reserved. + +#include + +#include + +static int localFunctionCounter = 0; +int localFunction() +{ + localFunctionCounter++; + return 67; +} +int (*realLocalFunction)() = localFunction; +static int localFunctionDetourCounter = 0; +int localFunctionDetour() +{ + localFunctionDetourCounter++; + return 12; +} + +TEST_CASE( "Overriding local function", "[local][attach]" ) +{ + localFunctionCounter = 0; + localFunctionDetourCounter = 0; + + CHECK( realLocalFunction == localFunction ); + CHECK( localFunction() == 67 ); + CHECK( localFunctionCounter == 1 ); + CHECK( localFunctionDetourCounter == 0 ); + + CHECK( detour_transaction_begin() == err_none ); + // Overriding a local function requires using detour_attach_and_commit instead of calling attach and commit individually. + // Otherwise when we return from attach (and before commit), the code page with the local function is marked as read+write (but _not_ executable). + // There is a good chance the code we return to (in this case the test function) is on the same memory page and can therefore not be executed (until we call commit). + CHECK( detour_attach_and_commit(reinterpret_cast(&realLocalFunction), reinterpret_cast(localFunctionDetour)) == err_none ); + + CHECK( realLocalFunction != localFunction ); + CHECK( localFunctionCounter == 1 ); + CHECK( localFunctionDetourCounter == 0 ); + CHECK( localFunction() == 12 ); + CHECK( localFunctionCounter == 1 ); + CHECK( localFunctionDetourCounter == 1 ); + + // clean up + CHECK( detour_transaction_begin() == err_none ); + CHECK( detour_detach_and_commit(reinterpret_cast(&realLocalFunction), reinterpret_cast(localFunctionDetour)) == err_none ); +}