From 843c246e118fdea737b02436cd4e3d511c28f858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=C3=81vila=20de=20Esp=C3=ADndola?= Date: Fri, 10 Aug 2012 18:42:02 -0400 Subject: [PATCH 01/16] Don't compute the offset in every pass of the loop and don't pass originalFunctionPtr and reentryIsland to fixupInstructions, it only needs the offset. --- mach_override.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/mach_override.c b/mach_override.c index c1d60ef..ec7a354 100644 --- a/mach_override.c +++ b/mach_override.c @@ -138,8 +138,7 @@ eatKnownInstructions( static void fixupInstructions( - void *originalFunction, - void *escapeIsland, + uint32_t offset, void *instructionsToFix, int instructionCount, uint8_t *instructionSizes ); @@ -324,7 +323,8 @@ mach_override_ptr( // // Note that on i386, we do not support someone else changing the code under our feet if ( !err ) { - fixupInstructions(originalFunctionPtr, reentryIsland, originalInstructions, + uint32_t offset = (uintptr_t)originalFunctionPtr - (uintptr_t)reentryIsland; + fixupInstructions(offset, originalInstructions, originalInstructionCount, originalInstructionSizes ); if( reentryIsland ) @@ -697,8 +697,7 @@ eatKnownInstructions( static void fixupInstructions( - void *originalFunction, - void *escapeIsland, + uint32_t offset, void *instructionsToFix, int instructionCount, uint8_t *instructionSizes ) @@ -708,13 +707,10 @@ fixupInstructions( { if (*(uint8_t*)instructionsToFix == 0xE9) // 32-bit jump relative { - uint32_t offset = (uintptr_t)originalFunction - (uintptr_t)escapeIsland; uint32_t *jumpOffsetPtr = (uint32_t*)((uintptr_t)instructionsToFix + 1); *jumpOffsetPtr += offset; } - originalFunction = (void*)((uintptr_t)originalFunction + instructionSizes[index]); - escapeIsland = (void*)((uintptr_t)escapeIsland + instructionSizes[index]); instructionsToFix = (void*)((uintptr_t)instructionsToFix + instructionSizes[index]); } } From 607a4f0d0a34dbcf2d16e8a9a0ca6ba79e67d9eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=C3=81vila=20de=20Esp=C3=ADndola?= Date: Fri, 10 Aug 2012 19:08:07 -0400 Subject: [PATCH 02/16] Add support for moving the call+pop sequence that is used on 32 bits to extract the value of eip. --- mach_override.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/mach_override.c b/mach_override.c index ec7a354..c0f360b 100644 --- a/mach_override.c +++ b/mach_override.c @@ -41,6 +41,9 @@ long kIslandTemplate[] = { #elif defined(__i386__) #define kOriginalInstructionsSize 16 +// On X86 we migh need to instert an add with a 32 bit immediate after the +// original instructions. +#define kMaxFixupSizeIncrease 5 char kIslandTemplate[] = { // kOriginalInstructionsSize nop instructions so that we @@ -56,6 +59,8 @@ char kIslandTemplate[] = { #elif defined(__x86_64__) #define kOriginalInstructionsSize 32 +// On X86-64 we never need to instert a new instruction. +#define kMaxFixupSizeIncrease 0 #define kJumpAddress kOriginalInstructionsSize + 6 @@ -215,7 +220,7 @@ mach_override_ptr( &jumpRelativeInstruction, &eatenCount, originalInstructions, &originalInstructionCount, originalInstructionSizes ); - if (eatenCount > kOriginalInstructionsSize) { + if (eatenCount + kMaxFixupSizeIncrease > kOriginalInstructionsSize) { //printf ("Too many instructions eaten\n"); overridePossible = false; } @@ -582,6 +587,7 @@ static AsmInstructionMatch possibleInstructions[] = { { 0x3, {0xFF, 0x4C, 0x00}, {0x8B, 0x40, 0x00} }, // mov $imm(%eax-%edx), %reg { 0x4, {0xFF, 0xFF, 0xFF, 0x00}, {0x8B, 0x4C, 0x24, 0x00} }, // mov $imm(%esp), %ecx { 0x5, {0xFF, 0x00, 0x00, 0x00, 0x00}, {0xB8, 0x00, 0x00, 0x00, 0x00} }, // mov $imm, %eax + { 0x6, {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF}, {0xE8, 0x00, 0x00, 0x00, 0x00, 0x58} }, // call $imm; pop %eax { 0x0 } }; #elif defined(__x86_64__) @@ -710,7 +716,26 @@ fixupInstructions( uint32_t *jumpOffsetPtr = (uint32_t*)((uintptr_t)instructionsToFix + 1); *jumpOffsetPtr += offset; } - + + // 32-bit call relative to the next addr; pop %eax + if (*(uint8_t*)instructionsToFix == 0xE8) + { + // Just this call is larger than the jump we use, so we + // know this is the last instruction. + assert(index == (instructionCount - 1)); + assert(instructionSizes[index] == 6); + + // Insert "addl $offset, %eax" in the end so that when + // we jump to the rest of the function %eax has the + // value it would have if eip had been pushed by the + // call in its original position. + uint8_t *op = instructionsToFix; + op += 6; + *op = 0x05; // addl + uint32_t *addImmPtr = (uint32_t*)(op + 1); + *addImmPtr = offset; + } + instructionsToFix = (void*)((uintptr_t)instructionsToFix + instructionSizes[index]); } } From 4619c1989b9fa193ff92864853db6a6427cdc5da Mon Sep 17 00:00:00 2001 From: rentzsch Date: Sat, 11 Aug 2012 13:26:15 -0500 Subject: [PATCH 03/16] Add testing for clang and x86_64. --- Rakefile | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/Rakefile b/Rakefile index 91fdc55..dc2ac82 100644 --- a/Rakefile +++ b/Rakefile @@ -1,12 +1,40 @@ desc 'Build' task :build do + puts "\n=== Building ===" system('mkdir build') - system('gcc -o build/test_gcc_i386 -framework CoreServices *.c *.cp') + + puts "\n*** Building GCC i386 ***" + system('gcc -o build/test_gcc_i386 -arch i386 -framework CoreServices *.c *.cp') + + puts "\n*** Building GCC x86_64 ***" + system('gcc -o build/test_gcc_x86_64 -arch x86_64 -framework CoreServices *.c *.cp') + + puts "\n*** Building Clang i386 ***" + system('clang -o build/test_clang_i386 -arch i386 -framework CoreServices *.c *.cp') + + puts "\n*** Building Clang x86_64 ***" + system('clang -o build/test_clang_x86_64 -arch x86_64 -framework CoreServices *.c *.cp') end desc 'Test' -task :test do +task :test => [:build] do + puts "\n=== Testing ===" + + puts "\n*** Testing GCC i386 ***" system('build/test_gcc_i386') + puts '!!! FAILED !!!' if $?.exitstatus != 0 + + puts "\n*** Testing GCC x86_64 ***" + system('build/test_gcc_x86_64') + puts '!!! FAILED !!!' if $?.exitstatus != 0 + + puts "\n*** Testing Clang i386 ***" + system('build/test_clang_i386') + puts '!!! FAILED !!!' if $?.exitstatus != 0 + + puts "\n*** Testing Clang x86_64 ***" + system('build/test_clang_x86_64') + puts '!!! FAILED !!!' if $?.exitstatus != 0 end desc 'Clean up' @@ -14,4 +42,4 @@ task :clean do system('rm -rf build') end -task :default => [:build, :test] \ No newline at end of file +task :default => [:test, :clean] \ No newline at end of file From 12dffc6ac4d9e7749f4e0bfdc82bfde2a16a8b5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20=C3=81vila=20de=20Esp=C3=ADndola?= Date: Mon, 13 Aug 2012 11:49:01 -0400 Subject: [PATCH 04/16] Add support for matching and relocating rip relative leaq. --- mach_override.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/mach_override.c b/mach_override.c index c0f360b..c324317 100644 --- a/mach_override.c +++ b/mach_override.c @@ -604,7 +604,11 @@ static AsmInstructionMatch possibleInstructions[] = { { 0x5, {0xF8, 0x00, 0x00, 0x00, 0x00}, {0xB8, 0x00, 0x00, 0x00, 0x00} }, // mov $imm, %reg { 0x3, {0xFF, 0xFF, 0x00}, {0xFF, 0x77, 0x00} }, // pushq $imm(%rdi) { 0x2, {0xFF, 0xFF}, {0x31, 0xC0} }, // xor %eax, %eax - { 0x2, {0xFF, 0xFF}, {0x89, 0xF8} }, // mov %edi, %eax + { 0x2, {0xFF, 0xFF}, {0x89, 0xF8} }, // mov %edi, %eax + + //leaq offset(%rip),%rax + { 0x7, {0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00}, {0x48, 0x8d, 0x05, 0x00, 0x00, 0x00, 0x00} }, + { 0x0 } }; #endif @@ -708,6 +712,9 @@ fixupInstructions( int instructionCount, uint8_t *instructionSizes ) { + // The start of "leaq offset(%rip),%rax" + static const uint8_t LeaqHeader[] = {0x48, 0x8d, 0x05}; + int index; for (index = 0;index < instructionCount;index += 1) { @@ -717,6 +724,12 @@ fixupInstructions( *jumpOffsetPtr += offset; } + // leaq offset(%rip),%rax + if (memcmp(instructionsToFix, LeaqHeader, 3) == 0) { + uint32_t *LeaqOffsetPtr = (uint32_t*)((uintptr_t)instructionsToFix + 3); + *LeaqOffsetPtr += offset; + } + // 32-bit call relative to the next addr; pop %eax if (*(uint8_t*)instructionsToFix == 0xE8) { From 5c82a8572e2394773097909d61968da3ec889ca4 Mon Sep 17 00:00:00 2001 From: rentzsch Date: Mon, 20 Aug 2012 12:40:48 -0500 Subject: [PATCH 05/16] [DEV] clean before testing too --- Rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index dc2ac82..e1276be 100644 --- a/Rakefile +++ b/Rakefile @@ -42,4 +42,4 @@ task :clean do system('rm -rf build') end -task :default => [:test, :clean] \ No newline at end of file +task :default => [:clean, :test, :clean] \ No newline at end of file From 7c886254611b5a82e89b36a522ea5df5f7c543d6 Mon Sep 17 00:00:00 2001 From: Frerich Raabe Date: Wed, 19 Sep 2012 23:05:13 +0200 Subject: [PATCH 06/16] Fixed warnings reported when using -pedantic with gcc 4.2.1 Compiling mach_override.c with gcc 4.2.1 via gcc -Wall -pedantic -std=c99 -c mach_override.c would yield a lot of warnings saying 'warning: overflow in implicit constant conversion' in the opcode arrays. The reason was that 'char' is a signed type with this compiler, so 0x90 (as well as 0xFF) were too large, causing an overflow. We don't really care whether it's signed or unsigned, but in order to silence warnings in -pedantic builds just use 'unsigned char' to be explicit. --- mach_override.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mach_override.c b/mach_override.c index c1d60ef..4431229 100644 --- a/mach_override.c +++ b/mach_override.c @@ -42,7 +42,7 @@ long kIslandTemplate[] = { #define kOriginalInstructionsSize 16 -char kIslandTemplate[] = { +unsigned char kIslandTemplate[] = { // kOriginalInstructionsSize nop instructions so that we // should have enough space to host original instructions 0x90, 0x90, 0x90, 0x90, 0x90, 0x90, 0x90, 0x90, @@ -59,7 +59,7 @@ char kIslandTemplate[] = { #define kJumpAddress kOriginalInstructionsSize + 6 -char kIslandTemplate[] = { +unsigned char kIslandTemplate[] = { // kOriginalInstructionsSize nop instructions so that we // should have enough space to host original instructions 0x90, 0x90, 0x90, 0x90, 0x90, 0x90, 0x90, 0x90, From 0d9cac4f93fd4b54f6b995031faaf6705d72a449 Mon Sep 17 00:00:00 2001 From: Rafael Avila de Espindola Date: Thu, 20 Sep 2012 11:21:29 -0400 Subject: [PATCH 07/16] Use a constant for the page size. --- mach_override.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/mach_override.c b/mach_override.c index c324317..a7c34cf 100644 --- a/mach_override.c +++ b/mach_override.c @@ -21,6 +21,7 @@ #pragma mark - #pragma mark (Constants) +#define kPageSize 4096 #if defined(__ppc__) || defined(__POWERPC__) long kIslandTemplate[] = { @@ -160,12 +161,10 @@ fixupInstructions( #if defined(__i386__) || defined(__x86_64__) mach_error_t makeIslandExecutable(void *address) { mach_error_t err = err_none; - vm_size_t pageSize; - host_page_size( mach_host_self(), &pageSize ); - uintptr_t page = (uintptr_t)address & ~(uintptr_t)(pageSize-1); + uintptr_t page = (uintptr_t)address & ~(uintptr_t)(kPageSize-1); int e = err_none; - e |= mprotect((void *)page, pageSize, PROT_EXEC | PROT_READ | PROT_WRITE); - e |= msync((void *)page, pageSize, MS_INVALIDATE ); + e |= mprotect((void *)page, kPageSize, PROT_EXEC | PROT_READ | PROT_WRITE); + e |= msync((void *)page, kPageSize, MS_INVALIDATE ); if (e) { err = err_cannot_override; } @@ -388,13 +387,11 @@ allocateBranchIsland( mach_error_t err = err_none; if( allocateHigh ) { - vm_size_t pageSize; - err = host_page_size( mach_host_self(), &pageSize ); if( !err ) { - assert( sizeof( BranchIsland ) <= pageSize ); + assert( sizeof( BranchIsland ) <= kPageSize ); #if defined(__ppc__) || defined(__POWERPC__) vm_address_t first = 0xfeffffff; - vm_address_t last = 0xfe000000 + pageSize; + vm_address_t last = 0xfe000000 + kPageSize; #elif defined(__x86_64__) vm_address_t first = ((uint64_t)originalFunctionAddress & ~(uint64_t)(((uint64_t)1 << 31) - 1)) | ((uint64_t)1 << 31); // start in the middle of the page? vm_address_t last = 0x0; @@ -409,14 +406,14 @@ allocateBranchIsland( while( !err && !allocated && page != last ) { - err = vm_allocate( task_self, &page, pageSize, 0 ); + err = vm_allocate( task_self, &page, kPageSize, 0 ); if( err == err_none ) allocated = 1; else if( err == KERN_NO_SPACE ) { #if defined(__x86_64__) - page -= pageSize; + page -= kPageSize; #else - page += pageSize; + page += kPageSize; #endif err = err_none; } @@ -458,13 +455,11 @@ freeBranchIsland( mach_error_t err = err_none; if( island->allocatedHigh ) { - vm_size_t pageSize; - err = host_page_size( mach_host_self(), &pageSize ); if( !err ) { - assert( sizeof( BranchIsland ) <= pageSize ); + assert( sizeof( BranchIsland ) <= kPageSize ); err = vm_deallocate( mach_task_self(), - (vm_address_t) island, pageSize ); + (vm_address_t) island, kPageSize ); } } else { free( island ); From ec45934df60921e4227a44561222d1ed95074c0d Mon Sep 17 00:00:00 2001 From: Rafael Avila de Espindola Date: Thu, 20 Sep 2012 11:22:31 -0400 Subject: [PATCH 08/16] Constant propagate kAllocateHigh. --- mach_override.c | 85 ++++++++++++++++++------------------------------- 1 file changed, 31 insertions(+), 54 deletions(-) diff --git a/mach_override.c b/mach_override.c index a7c34cf..741df26 100644 --- a/mach_override.c +++ b/mach_override.c @@ -80,9 +80,6 @@ char kIslandTemplate[] = { #endif -#define kAllocateHigh 1 -#define kAllocateNormal 0 - /************************** * * Data Types @@ -93,7 +90,6 @@ char kIslandTemplate[] = { typedef struct { char instructions[sizeof(kIslandTemplate)]; - int allocatedHigh; } BranchIsland; /************************** @@ -107,7 +103,6 @@ typedef struct { mach_error_t allocateBranchIsland( BranchIsland **island, - int allocateHigh, void *originalFunctionAddress); mach_error_t @@ -242,7 +237,7 @@ mach_override_ptr( // Allocate and target the escape island to the overriding function. BranchIsland *escapeIsland = NULL; if( !err ) - err = allocateBranchIsland( &escapeIsland, kAllocateHigh, originalFunctionAddress ); + err = allocateBranchIsland( &escapeIsland, originalFunctionAddress ); if (err) fprintf(stderr, "err = %x %s:%d\n", err, __FILE__, __LINE__); @@ -284,7 +279,7 @@ mach_override_ptr( // technically our original function. BranchIsland *reentryIsland = NULL; if( !err && originalFunctionReentryIsland ) { - err = allocateBranchIsland( &reentryIsland, kAllocateHigh, escapeIsland); + err = allocateBranchIsland( &reentryIsland, escapeIsland); if( !err ) *originalFunctionReentryIsland = reentryIsland; } @@ -369,9 +364,6 @@ mach_override_ptr( Implementation: Allocates memory for a branch island. @param island <- The allocated island. - @param allocateHigh -> Whether to allocate the island at the end of the - address space (for use with the branch absolute - instruction). @result <- mach_error_t ***************************************************************************/ @@ -379,60 +371,49 @@ mach_override_ptr( mach_error_t allocateBranchIsland( BranchIsland **island, - int allocateHigh, void *originalFunctionAddress) { assert( island ); mach_error_t err = err_none; - if( allocateHigh ) { - if( !err ) { - assert( sizeof( BranchIsland ) <= kPageSize ); + if( !err ) { + assert( sizeof( BranchIsland ) <= kPageSize ); #if defined(__ppc__) || defined(__POWERPC__) - vm_address_t first = 0xfeffffff; - vm_address_t last = 0xfe000000 + kPageSize; + vm_address_t first = 0xfeffffff; + vm_address_t last = 0xfe000000 + kPageSize; #elif defined(__x86_64__) - vm_address_t first = ((uint64_t)originalFunctionAddress & ~(uint64_t)(((uint64_t)1 << 31) - 1)) | ((uint64_t)1 << 31); // start in the middle of the page? - vm_address_t last = 0x0; + vm_address_t first = ((uint64_t)originalFunctionAddress & ~(uint64_t)(((uint64_t)1 << 31) - 1)) | ((uint64_t)1 << 31); // start in the middle of the page? + vm_address_t last = 0x0; #else - vm_address_t first = 0xffc00000; - vm_address_t last = 0xfffe0000; + vm_address_t first = 0xffc00000; + vm_address_t last = 0xfffe0000; #endif - vm_address_t page = first; - int allocated = 0; - vm_map_t task_self = mach_task_self(); + vm_address_t page = first; + int allocated = 0; + vm_map_t task_self = mach_task_self(); - while( !err && !allocated && page != last ) { + while( !err && !allocated && page != last ) { - err = vm_allocate( task_self, &page, kPageSize, 0 ); - if( err == err_none ) - allocated = 1; - else if( err == KERN_NO_SPACE ) { + err = vm_allocate( task_self, &page, kPageSize, 0 ); + if( err == err_none ) + allocated = 1; + else if( err == KERN_NO_SPACE ) { #if defined(__x86_64__) - page -= kPageSize; + page -= kPageSize; #else - page += kPageSize; + page += kPageSize; #endif - err = err_none; - } + err = err_none; } - if( allocated ) - *island = (BranchIsland*) page; - else if( !allocated && !err ) - err = KERN_NO_SPACE; } - } else { - void *block = malloc( sizeof( BranchIsland ) ); - if( block ) - *island = block; - else + if( allocated ) + *island = (BranchIsland*) page; + else if( !allocated && !err ) err = KERN_NO_SPACE; } - if( !err ) - (**island).allocatedHigh = allocateHigh; - + return err; } @@ -450,19 +431,15 @@ freeBranchIsland( { assert( island ); assert( (*(long*)&island->instructions[0]) == kIslandTemplate[0] ); - assert( island->allocatedHigh ); mach_error_t err = err_none; - if( island->allocatedHigh ) { - if( !err ) { - assert( sizeof( BranchIsland ) <= kPageSize ); - err = vm_deallocate( - mach_task_self(), - (vm_address_t) island, kPageSize ); - } - } else { - free( island ); + + if( !err ) { + assert( sizeof( BranchIsland ) <= kPageSize ); + err = vm_deallocate( + mach_task_self(), + (vm_address_t) island, kPageSize ); } return err; From 2ca0cfca53abdc4e0d27cec5b1df248692453592 Mon Sep 17 00:00:00 2001 From: Rafael Avila de Espindola Date: Thu, 20 Sep 2012 11:23:08 -0400 Subject: [PATCH 09/16] Simplify freeBranchIsland. --- mach_override.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/mach_override.c b/mach_override.c index 741df26..f680eb9 100644 --- a/mach_override.c +++ b/mach_override.c @@ -431,18 +431,9 @@ freeBranchIsland( { assert( island ); assert( (*(long*)&island->instructions[0]) == kIslandTemplate[0] ); - - mach_error_t err = err_none; - - - if( !err ) { - assert( sizeof( BranchIsland ) <= kPageSize ); - err = vm_deallocate( - mach_task_self(), - (vm_address_t) island, kPageSize ); - } - - return err; + assert( sizeof( BranchIsland ) <= kPageSize ); + return vm_deallocate( mach_task_self(), (vm_address_t) island, + kPageSize ); } /******************************************************************************* From 45593d9f6973bb527ed86c9abe2e1fe7791ac681 Mon Sep 17 00:00:00 2001 From: Rafael Avila de Espindola Date: Thu, 20 Sep 2012 11:25:12 -0400 Subject: [PATCH 10/16] Simplify allocateBranchIsland. --- mach_override.c | 48 +++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/mach_override.c b/mach_override.c index f680eb9..7918790 100644 --- a/mach_override.c +++ b/mach_override.c @@ -376,43 +376,41 @@ allocateBranchIsland( assert( island ); mach_error_t err = err_none; - - if( !err ) { - assert( sizeof( BranchIsland ) <= kPageSize ); + + assert( sizeof( BranchIsland ) <= kPageSize ); #if defined(__ppc__) || defined(__POWERPC__) - vm_address_t first = 0xfeffffff; - vm_address_t last = 0xfe000000 + kPageSize; + vm_address_t first = 0xfeffffff; + vm_address_t last = 0xfe000000 + kPageSize; #elif defined(__x86_64__) - vm_address_t first = ((uint64_t)originalFunctionAddress & ~(uint64_t)(((uint64_t)1 << 31) - 1)) | ((uint64_t)1 << 31); // start in the middle of the page? - vm_address_t last = 0x0; + vm_address_t first = ((uint64_t)originalFunctionAddress & ~(uint64_t)(((uint64_t)1 << 31) - 1)) | ((uint64_t)1 << 31); // start in the middle of the page? + vm_address_t last = 0x0; #else - vm_address_t first = 0xffc00000; - vm_address_t last = 0xfffe0000; + vm_address_t first = 0xffc00000; + vm_address_t last = 0xfffe0000; #endif - vm_address_t page = first; - int allocated = 0; - vm_map_t task_self = mach_task_self(); + vm_address_t page = first; + int allocated = 0; + vm_map_t task_self = mach_task_self(); - while( !err && !allocated && page != last ) { + while( !err && !allocated && page != last ) { - err = vm_allocate( task_self, &page, kPageSize, 0 ); - if( err == err_none ) - allocated = 1; - else if( err == KERN_NO_SPACE ) { + err = vm_allocate( task_self, &page, kPageSize, 0 ); + if( err == err_none ) + allocated = 1; + else if( err == KERN_NO_SPACE ) { #if defined(__x86_64__) - page -= kPageSize; + page -= kPageSize; #else - page += kPageSize; + page += kPageSize; #endif - err = err_none; - } + err = err_none; } - if( allocated ) - *island = (BranchIsland*) page; - else if( !allocated && !err ) - err = KERN_NO_SPACE; } + if( allocated ) + *island = (BranchIsland*) page; + else if( !allocated && !err ) + err = KERN_NO_SPACE; return err; } From 44488d3359a194266bf935d14a4f24a5ef4f46e5 Mon Sep 17 00:00:00 2001 From: Rafael Avila de Espindola Date: Thu, 20 Sep 2012 11:25:59 -0400 Subject: [PATCH 11/16] Use early returns. --- mach_override.c | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/mach_override.c b/mach_override.c index 7918790..60d357b 100644 --- a/mach_override.c +++ b/mach_override.c @@ -375,8 +375,6 @@ allocateBranchIsland( { assert( island ); - mach_error_t err = err_none; - assert( sizeof( BranchIsland ) <= kPageSize ); #if defined(__ppc__) || defined(__POWERPC__) vm_address_t first = 0xfeffffff; @@ -390,29 +388,25 @@ allocateBranchIsland( #endif vm_address_t page = first; - int allocated = 0; vm_map_t task_self = mach_task_self(); - - while( !err && !allocated && page != last ) { - err = vm_allocate( task_self, &page, kPageSize, 0 ); - if( err == err_none ) - allocated = 1; - else if( err == KERN_NO_SPACE ) { + while( page != last ) { + mach_error_t err = vm_allocate( task_self, &page, kPageSize, 0 ); + if( err == err_none ) { + *island = (BranchIsland*) page; + return err_none; + } + if( err != KERN_NO_SPACE ) + return err; #if defined(__x86_64__) - page -= kPageSize; + page -= kPageSize; #else - page += kPageSize; + page += kPageSize; #endif - err = err_none; - } + err = err_none; } - if( allocated ) - *island = (BranchIsland*) page; - else if( !allocated && !err ) - err = KERN_NO_SPACE; - return err; + return KERN_NO_SPACE; } /******************************************************************************* From a3f02dc82b2ab4b05393a53e3584712891e9ce3f Mon Sep 17 00:00:00 2001 From: Rafael Avila de Espindola Date: Thu, 20 Sep 2012 12:40:23 -0400 Subject: [PATCH 12/16] make allocateBranchIsland static --- mach_override.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mach_override.c b/mach_override.c index 60d357b..d2d69f4 100644 --- a/mach_override.c +++ b/mach_override.c @@ -100,7 +100,7 @@ typedef struct { #pragma mark - #pragma mark (Funky Protos) - mach_error_t +static mach_error_t allocateBranchIsland( BranchIsland **island, void *originalFunctionAddress); @@ -368,7 +368,7 @@ mach_override_ptr( ***************************************************************************/ - mach_error_t +static mach_error_t allocateBranchIsland( BranchIsland **island, void *originalFunctionAddress) From 458da5499d20596322ab1330b804213a6dc5a0f5 Mon Sep 17 00:00:00 2001 From: Rafael Avila de Espindola Date: Thu, 20 Sep 2012 12:42:39 -0400 Subject: [PATCH 13/16] Use vm_region_64 to skip an entire region at a time instead of one page at a time. --- mach_override.c | 87 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 27 deletions(-) diff --git a/mach_override.c b/mach_override.c index d2d69f4..faa2a7d 100644 --- a/mach_override.c +++ b/mach_override.c @@ -360,6 +360,12 @@ mach_override_ptr( #pragma mark - #pragma mark (Implementation) +static bool jump_in_range(intptr_t from, intptr_t to) { + intptr_t field_value = to - from - 5; + int32_t field_value_32 = field_value; + return field_value == field_value_32; +} + /******************************************************************************* Implementation: Allocates memory for a branch island. @@ -369,46 +375,73 @@ mach_override_ptr( ***************************************************************************/ static mach_error_t -allocateBranchIsland( +allocateBranchIslandAux( BranchIsland **island, - void *originalFunctionAddress) + void *originalFunctionAddress, + bool forward) { assert( island ); - assert( sizeof( BranchIsland ) <= kPageSize ); -#if defined(__ppc__) || defined(__POWERPC__) - vm_address_t first = 0xfeffffff; - vm_address_t last = 0xfe000000 + kPageSize; -#elif defined(__x86_64__) - vm_address_t first = ((uint64_t)originalFunctionAddress & ~(uint64_t)(((uint64_t)1 << 31) - 1)) | ((uint64_t)1 << 31); // start in the middle of the page? - vm_address_t last = 0x0; -#else - vm_address_t first = 0xffc00000; - vm_address_t last = 0xfffe0000; -#endif - vm_address_t page = first; vm_map_t task_self = mach_task_self(); - - while( page != last ) { - mach_error_t err = vm_allocate( task_self, &page, kPageSize, 0 ); - if( err == err_none ) { - *island = (BranchIsland*) page; - return err_none; - } - if( err != KERN_NO_SPACE ) - return err; -#if defined(__x86_64__) - page -= kPageSize; + vm_address_t original_address = (vm_address_t) originalFunctionAddress; + vm_address_t address = original_address; + + for (;;) { + vm_size_t vmsize = 0; + memory_object_name_t object = 0; + kern_return_t kr = 0; + vm_region_flavor_t flavor = VM_REGION_BASIC_INFO; + // Find the region the address is in. +#if __WORDSIZE == 32 + vm_region_basic_info_data_t info; + mach_msg_type_number_t info_count = VM_REGION_BASIC_INFO_COUNT; + kr = vm_region(task_self, &address, &vmsize, flavor, + (vm_region_info_t)&info, &info_count, &object); #else - page += kPageSize; + vm_region_basic_info_data_64_t info; + mach_msg_type_number_t info_count = VM_REGION_BASIC_INFO_COUNT_64; + kr = vm_region_64(task_self, &address, &vmsize, flavor, + (vm_region_info_t)&info, &info_count, &object); +#endif + if (kr != KERN_SUCCESS) + return kr; + assert((address & (kPageSize - 1)) == 0); + + // Go to the first page before or after this region + vm_address_t new_address = forward ? address + vmsize : address - kPageSize; +#if __WORDSIZE == 64 + if(!jump_in_range(original_address, new_address)) + break; #endif - err = err_none; + address = new_address; + + // Try to allocate this page. + kr = vm_allocate(task_self, &address, kPageSize, 0); + if (kr == KERN_SUCCESS) { + *island = (BranchIsland*) address; + return err_none; + } + if (kr != KERN_NO_SPACE) + return kr; } return KERN_NO_SPACE; } +static mach_error_t +allocateBranchIsland( + BranchIsland **island, + void *originalFunctionAddress) +{ + mach_error_t err = + allocateBranchIslandAux(island, originalFunctionAddress, true); + if (!err) + return err; + return allocateBranchIslandAux(island, originalFunctionAddress, false); +} + + /******************************************************************************* Implementation: Deallocates memory for a branch island. From 213c58c56f6780135d33e28fb84f7a1426bc992c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20P=2E=20Tjern=C3=B8?= Date: Wed, 14 Aug 2013 12:00:24 -0700 Subject: [PATCH 14/16] Fix x86 builds on Clang 3.2/Xcode 4.6. --- mach_override.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/mach_override.c b/mach_override.c index 0b5837e..844b660 100644 --- a/mach_override.c +++ b/mach_override.c @@ -730,7 +730,7 @@ fixupInstructions( // we jump to the rest of the function %eax has the // value it would have if eip had been pushed by the // call in its original position. - uint8_t *op = instructionsToFix; + uint8_t *op = (uint8_t*)instructionsToFix; op += 6; *op = 0x05; // addl uint32_t *addImmPtr = (uint32_t*)(op + 1); @@ -743,7 +743,12 @@ fixupInstructions( #endif #if defined(__i386__) -__asm( +void __attribute__((naked)) +atomic_mov64( + uint64_t *targetAddress, + uint64_t value) +{ + __asm( ".text;" ".align 2, 0x90;" "_atomic_mov64:;" @@ -775,8 +780,8 @@ __asm( " popl %ebx;" " popl %esi;" " popl %ebp;" - " ret" -); + ); +} #elif defined(__x86_64__) void atomic_mov64( uint64_t *targetAddress, From a791fe4a44cbf85db3e18675ea598da555281c05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20P=2E=20Tjern=C3=B8?= Date: Wed, 28 Aug 2013 13:51:20 -0700 Subject: [PATCH 15/16] Use OSAtomicCompareAndSwap64Barrier. Replacing atomic_mov64 with an implementation that just calls OSAtomicCompareAndSwap64Barrier on all OSes. --- mach_override.c | 55 ++++++++----------------------------------------- 1 file changed, 9 insertions(+), 46 deletions(-) diff --git a/mach_override.c b/mach_override.c index 844b660..b6f7143 100644 --- a/mach_override.c +++ b/mach_override.c @@ -10,6 +10,7 @@ #include #include #include +#include #include @@ -618,7 +619,6 @@ static Boolean codeMatchesInstruction(unsigned char *code, AsmInstructionMatch* return match; } -#if defined(__i386__) || defined(__x86_64__) static Boolean eatKnownInstructions( unsigned char *code, @@ -740,54 +740,17 @@ fixupInstructions( instructionsToFix = (void*)((uintptr_t)instructionsToFix + instructionSizes[index]); } } -#endif -#if defined(__i386__) -void __attribute__((naked)) -atomic_mov64( - uint64_t *targetAddress, - uint64_t value) -{ - __asm( - ".text;" - ".align 2, 0x90;" - "_atomic_mov64:;" - " pushl %ebp;" - " movl %esp, %ebp;" - " pushl %esi;" - " pushl %ebx;" - " pushl %ecx;" - " pushl %eax;" - " pushl %edx;" - - // atomic push of value to an address - // we use cmpxchg8b, which compares content of an address with - // edx:eax. If they are equal, it atomically puts 64bit value - // ecx:ebx in address. - // We thus put contents of address in edx:eax to force ecx:ebx - // in address - " mov 8(%ebp), %esi;" // esi contains target address - " mov 12(%ebp), %ebx;" - " mov 16(%ebp), %ecx;" // ecx:ebx now contains value to put in target address - " mov (%esi), %eax;" - " mov 4(%esi), %edx;" // edx:eax now contains value currently contained in target address - " lock; cmpxchg8b (%esi);" // atomic move. - - // restore registers - " popl %edx;" - " popl %eax;" - " popl %ecx;" - " popl %ebx;" - " popl %esi;" - " popl %ebp;" - ); -} -#elif defined(__x86_64__) void atomic_mov64( uint64_t *targetAddress, uint64_t value ) { - *targetAddress = value; + bool swapOk = false; + while (!swapOk) + { + uint64_t oldValue = *targetAddress; + swapOk = OSAtomicCompareAndSwap64Barrier(oldValue, value, (int64_t*)targetAddress); + } } -#endif -#endif + +#endif // defined(__i386__) || defined(__x86_64__) From fb2b096110e8940a562bd9d621d67b7f870982c3 Mon Sep 17 00:00:00 2001 From: rentzsch Date: Thu, 29 Aug 2013 17:52:42 -0500 Subject: [PATCH 16/16] Upgrade testing to avoid Gestalt. strerror, our test-victim function that we override, returns a slightly-different result on 10.6 and 10.7+. We used to call Gestalt to decide which we should expect, but Gestalt is deprecated. Apparently the current "best" solution is to read SystemVersion.plist (ref https://twitter.com/rentzsch/status/373203814993494016) but that's a lot of code to write just to decide on which string to expect. So this code now will accept either. But the assert* code was hard to follow with the strcmp() == 0, so I upgraded that yak as well. --- test_mach_override.cp | 55 +++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/test_mach_override.cp b/test_mach_override.cp index 77ad748..5f64111 100644 --- a/test_mach_override.cp +++ b/test_mach_override.cp @@ -5,8 +5,36 @@ #include #include "mach_override.h" -#define assertStrEqual( EXPECTED, ACTUAL ) if( strcmp( (EXPECTED), (ACTUAL) ) != 0 ) { printf( "EXPECTED: %s\nACTUAL: %s\n", (EXPECTED), (ACTUAL)); assert( strcmp( (EXPECTED), (ACTUAL) ) == 0 ); } -#define assertIntEqual( EXPECTED, ACTUAL ) if( (EXPECTED) != (ACTUAL) ) { printf( "EXPECTED: %d\nACTUAL: %d\n", (EXPECTED), (ACTUAL)); assert( (EXPECTED) == (ACTUAL) ); } +// A simple extension to assert that can print expectation information: +#define assertf(CONDITION, FORMAT, ...) \ + if (!(CONDITION)) { \ + printf((FORMAT), ##__VA_ARGS__); \ + assert(CONDITION); \ + } + +#define string_equals(s1, s2) \ + ({ \ + strcmp(s1, s2) == 0; \ + }) + +#define assertStrEqual(EXPECTED, ACTUAL) \ + assertf(string_equals(EXPECTED, ACTUAL), \ + "EXPECTED: %s\nACTUAL: %s\n", \ + EXPECTED, \ + ACTUAL); + +#define assertStr2Equal(EXPECTED1, EXPECTED2, ACTUAL) \ + assertf(string_equals(EXPECTED1, ACTUAL) || string_equals(EXPECTED2, ACTUAL), \ + "EXPECTED: %s OR %s\nACTUAL: %s\n", \ + EXPECTED1, \ + EXPECTED2, \ + ACTUAL); + +#define assertIntEqual(EXPECTED, ACTUAL) \ + assertf(EXPECTED == ACTUAL, \ + "EXPECTED: %d\nACTUAL: %d\n", \ + EXPECTED, \ + ACTUAL); //------------------------------------------------------------------------------ #pragma mark Test Local Override by Pointer @@ -41,29 +69,30 @@ void testLocalFunctionOverrideByPointer() { #pragma mark Test System Override by Pointer char* (*strerrorPtr)(int) = strerror; -const char* strerrReturnValue = "Unknown error: 0"; +const char* strerrReturnValueOn10_6 = "Unknown error: 0"; +const char* strerrReturnValueOn10_7 = "Undefined error: 0"; void testSystemFunctionOverrideByPointer() { - SInt32 sysv; - if (Gestalt( gestaltSystemVersion, &sysv ) == noErr && sysv >= 0x1070) - strerrReturnValue = "Undefined error: 0"; - // Test original. - assertStrEqual( strerrReturnValue, strerrorPtr( 0 ) ); + assertStr2Equal(strerrReturnValueOn10_6, + strerrReturnValueOn10_7, + strerrorPtr(0)); // Override system function by pointer. kern_return_t err; - MACH_OVERRIDE( char*, strerror, (int errnum), err ) { + MACH_OVERRIDE(char*, strerror, (int errnum), err) { // Test calling through the reentry island back into the original // implementation. - assertStrEqual( strerrReturnValue, strerror_reenter( 0 ) ); + assertStr2Equal(strerrReturnValueOn10_6, + strerrReturnValueOn10_7, + strerror_reenter(0)); - return (char *)"strerrorOverride"; + return (char*)"strerrorOverride"; } END_MACH_OVERRIDE(strerror); - assert( !err ); + assert(!err); // Test override took effect. - assertStrEqual( "strerrorOverride", strerrorPtr( 0 ) ); + assertStrEqual("strerrorOverride", strerrorPtr(0)); } //------------------------------------------------------------------------------