From 32fcda1f46f297676088d0147552bf763fcb6a51 Mon Sep 17 00:00:00 2001 From: "mark@chromium.org" Date: Mon, 12 May 2014 15:32:56 +0000 Subject: Update mach_override to 919148f9, picking up: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit commit 919148f94db54fc04d287eb6a42c0c36b166bbfa Merge: e2ca3c3 84a5bd9 Author: Jonathan 'Wolf' Rentzsch Date: Sun May 11 21:51:20 2014 -0500 Merge pull request #16 from mark-chromium/patch-1 [FIX] Stop using mach_host_self and host_page_size, fixing a port right leak. (Mark Mentovai) commit 84a5bd929213b9e0f059d3bc8c5b738e9fe4e620 Author: Mark Mentovai Date: Fri May 9 16:40:10 2014 -0400 Stop using mach_host_self and host_page_size, fixing a port right leak It is incorrect to use mach_host_self without disposing of the send send right to the host port with mach_port_deallocate when done with it. http://crbug.com/105513 shows the sorts of problems that can arise when send rights aren’t properly deallocated. mach_host_self was only used by mach_override to be able to call host_page_size. host_page_size is unnecessary, because it always returns a constant value, PAGE_SIZE, which is also known at user-land compile time. See libsyscall/mach/mach_init.c. User code is better off just using this macro directly, and not fumbling with the system calls to obtain and properly dispose of a send right to the host port. (You need to mach_port_deallocate the ports you get from mach_host_self and mach_thread_self, but you must not normally deallocate the one from mach_task_self, because mach_task_self is actually just a macro that references a global variable. It doesn’t add any port rights at all. See . If you bypass the macro and call the real mach_task_self system call, you do need to call mach_port_deallocate, but this situation is incredibly rare.) R=rsesek@chromium.org Review URL: https://codereview.chromium.org/282523004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269785 0039d316-1c4b-4281-b951-d872f2087c98 --- third_party/mach_override/README.chromium | 11 ++-- third_party/mach_override/mach_override.c | 84 +++++++++++++------------------ 2 files changed, 37 insertions(+), 58 deletions(-) diff --git a/third_party/mach_override/README.chromium b/third_party/mach_override/README.chromium index aa803e7..8a7c3fd 100644 --- a/third_party/mach_override/README.chromium +++ b/third_party/mach_override/README.chromium @@ -2,8 +2,8 @@ Name: mach_override Short Name: mach_override Version: Newer than 1.2. HEAD from branch semver-1.x. URL: https://github.com/rentzsch/mach_override -Date: 2013-08-21 -Revision: 1a1bb35291a915c545842cd64b5e12e1e76883fc +Date: 2014-05-11 +Revision: 919148f94db54fc04d287eb6a42c0c36b166bbfa License: MIT and 2-clause BSD Security Critical: Yes @@ -16,9 +16,4 @@ mach_override includes a copy of libudis86 1.7.1, available separately from http://udis86.sourceforge.net/ and https://github.com/vmt/udis86 . -Local Modifications: -Ensure no rwx pages remain after mach_override_ptr: -https://codereview.chromium.org/21208002/ - -Randomize mach_override_ptr trampoline addresses on 32-bit: -https://codereview.chromium.org/22798004/ +Local Modifications: None. diff --git a/third_party/mach_override/mach_override.c b/third_party/mach_override/mach_override.c index 46d2152..85a75e5c 100644 --- a/third_party/mach_override/mach_override.c +++ b/third_party/mach_override/mach_override.c @@ -9,7 +9,6 @@ #endif #include -#include #include #include #include @@ -160,12 +159,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)(PAGE_SIZE - 1); int e = err_none; - e |= mprotect((void *)page, pageSize, PROT_EXEC | PROT_READ); - e |= msync((void *)page, pageSize, MS_INVALIDATE ); + e |= mprotect((void *)page, PAGE_SIZE, PROT_EXEC | PROT_READ); + e |= msync((void *)page, PAGE_SIZE, MS_INVALIDATE ); if (e) { err = err_cannot_override; } @@ -343,12 +340,11 @@ mach_override_ptr( #endif if ( !err ) atomic_mov64((uint64_t *)originalFunctionPtr, jumpRelativeInstruction); - - mach_error_t prot_err = err_none; + mach_error_t prot_err = err_none; prot_err = vm_protect( mach_task_self(), (vm_address_t) originalFunctionPtr, 8, false, (VM_PROT_READ | VM_PROT_EXECUTE) ); - if (prot_err) fprintf(stderr, "err = %x %s:%d\n", prot_err, __FILE__, __LINE__); + if(prot_err) fprintf(stderr, "err = %x %s:%d\n", prot_err, __FILE__, __LINE__); } #endif @@ -393,52 +389,46 @@ 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 ) <= PAGE_SIZE ); + vm_address_t page = 0; #if defined(__i386__) - vm_address_t page = 0; - mach_error_t err = vm_allocate( mach_task_self(), &page, pageSize, VM_FLAGS_ANYWHERE ); - if( err == err_none ) { - *island = (BranchIsland*) page; - return err_none; - } - return err; + err = vm_allocate( mach_task_self(), &page, PAGE_SIZE, VM_FLAGS_ANYWHERE ); + if( err == err_none ) + *island = (BranchIsland*) page; #else #if defined(__ppc__) || defined(__POWERPC__) - vm_address_t first = 0xfeffffff; - vm_address_t last = 0xfe000000 + pageSize; + vm_address_t first = 0xfeffffff; + vm_address_t last = 0xfe000000 + PAGE_SIZE; #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; + // 64-bit ASLR is in bits 13-28 + vm_address_t first = ((uint64_t)originalFunctionAddress & ~( (0xFUL << 28) | (PAGE_SIZE - 1) ) ) | (0x1UL << 31); + vm_address_t last = (uint64_t)originalFunctionAddress & ~((0x1UL << 32) - 1); #endif - vm_address_t page = first; - int allocated = 0; - vm_map_t task_self = mach_task_self(); - - while( !err && !allocated && page != last ) { + page = first; + int allocated = 0; + vm_map_t task_self = mach_task_self(); - err = vm_allocate( task_self, &page, pageSize, 0 ); - if( err == err_none ) - allocated = 1; - else if( err == KERN_NO_SPACE ) { + while( !err && !allocated && page != last ) { + + err = vm_allocate( task_self, &page, PAGE_SIZE, 0 ); + if( err == err_none ) + allocated = 1; + else if( err == KERN_NO_SPACE ) { #if defined(__x86_64__) - page -= pageSize; + page -= PAGE_SIZE; #else - page += pageSize; + page += PAGE_SIZE; #endif - err = err_none; - } + err = err_none; } - if( allocated ) - *island = (BranchIsland*) page; - else if( !allocated && !err ) - err = KERN_NO_SPACE; -#endif } + if( allocated ) + *island = (BranchIsland*) page; + else if( !allocated && !err ) + err = KERN_NO_SPACE; +#endif } else { void *block = malloc( sizeof( BranchIsland ) ); if( block ) @@ -471,14 +461,8 @@ 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 ); - err = vm_deallocate( - mach_task_self(), - (vm_address_t) island, pageSize ); - } + assert( sizeof( BranchIsland ) <= PAGE_SIZE ); + err = vm_deallocate(mach_task_self(), (vm_address_t) island, PAGE_SIZE ); } else { free( island ); } -- cgit v1.1