From aa3c6111d9361d3fddb4cdbe2365e356e740f1b9 Mon Sep 17 00:00:00 2001 From: Sebastian Lackner Date: Wed, 8 Oct 2014 21:26:28 +0200 Subject: [PATCH] ntdll: Ensure force_exec_prot is also used for views with write watch permissions. --- dlls/kernel32/tests/virtual.c | 8 +++++--- dlls/ntdll/virtual.c | 36 ++++++++++++++++++++++------------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c index 4d8277dfd86..2d2554b690c 100644 --- a/dlls/kernel32/tests/virtual.c +++ b/dlls/kernel32/tests/virtual.c @@ -2102,7 +2102,6 @@ static void test_atl_thunk_emulation( ULONG dep_flags ) if (dep_flags & MEM_EXECUTE_OPTION_DISABLE) ok( num_execute_fault_calls == 1, "expected one STATUS_ACCESS_VIOLATION exception, got %d exceptions\n", num_execute_fault_calls ); else - todo_wine ok( num_execute_fault_calls == 0, "expected no STATUS_ACCESS_VIOLATION exception, got %d exceptions\n", num_execute_fault_calls ); count = 64; @@ -2113,7 +2112,11 @@ static void test_atl_thunk_emulation( ULONG dep_flags ) ret = send_message_excpt( hWnd, WM_USER, 0, 0 ); ok( ret == 42, "call returned wrong result, expected 42, got %d\n", ret ); ok( num_guard_page_calls == 0, "expected no STATUS_GUARD_PAGE_VIOLATION exception, got %d exceptions\n", num_guard_page_calls ); - ok( num_execute_fault_calls == 0, "expected no STATUS_ACCESS_VIOLATION exception, got %d exceptions\n", num_execute_fault_calls ); + if (dep_flags & MEM_EXECUTE_OPTION_DISABLE) + ok( num_execute_fault_calls == 0, "expected no STATUS_ACCESS_VIOLATION exception, got %d exceptions\n", num_execute_fault_calls ); + else + todo_wine + ok( num_execute_fault_calls == 0, "expected no STATUS_ACCESS_VIOLATION exception, got %d exceptions\n", num_execute_fault_calls ); /* Now a bit more complicated, the page containing the code is protected with * PAGE_GUARD memory protection. */ @@ -2127,7 +2130,6 @@ static void test_atl_thunk_emulation( ULONG dep_flags ) if (dep_flags & MEM_EXECUTE_OPTION_DISABLE) ok( num_execute_fault_calls == 1, "expected one STATUS_ACCESS_VIOLATION exception, got %d exceptions\n", num_execute_fault_calls ); else - todo_wine ok( num_execute_fault_calls == 0, "expected no STATUS_ACCESS_VIOLATION exception, got %d exceptions\n", num_execute_fault_calls ); ret = send_message_excpt( hWnd, WM_USER, 0, 0 ); diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index f8a5dd3f49c..3c9a4b5ad0f 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -590,6 +590,25 @@ static NTSTATUS get_vprot_flags( DWORD protect, unsigned int *vprot, BOOL image } +/*********************************************************************** + * mprotect_exec + * + * Wrapper for mprotect, adds PROT_EXEC if forced by force_exec_prot + */ +static inline int mprotect_exec( void *base, size_t size, int unix_prot, unsigned int view_protect ) +{ + if (force_exec_prot && !(view_protect & VPROT_NOEXEC) && + (unix_prot & PROT_READ) && !(unix_prot & PROT_EXEC)) + { + TRACE( "forcing exec permission on %p-%p\n", base, (char *)base + size - 1 ); + if (!mprotect( base, size, unix_prot | PROT_EXEC )) return 0; + /* exec + write may legitimately fail, in that case fall back to write only */ + if (!(unix_prot & PROT_WRITE)) return -1; + } + + return mprotect( base, size, unix_prot ); +} + /*********************************************************************** * VIRTUAL_SetProt * @@ -624,12 +643,12 @@ static BOOL VIRTUAL_SetProt( struct file_view *view, /* [in] Pointer to view */ p[i] = vprot | (p[i] & VPROT_WRITEWATCH); prot = VIRTUAL_GetUnixProt( p[i] ); if (prot == unix_prot) continue; - mprotect( addr, count << page_shift, unix_prot ); + mprotect_exec( addr, count << page_shift, unix_prot, view->protect ); addr += count << page_shift; unix_prot = prot; count = 0; } - if (count) mprotect( addr, count << page_shift, unix_prot ); + if (count) mprotect_exec( addr, count << page_shift, unix_prot, view->protect ); VIRTUAL_DEBUG_DUMP_VIEW( view ); return TRUE; } @@ -646,18 +665,9 @@ static BOOL VIRTUAL_SetProt( struct file_view *view, /* [in] Pointer to view */ return TRUE; } - if (force_exec_prot && !(view->protect & VPROT_NOEXEC) && - (unix_prot & PROT_READ) && !(unix_prot & PROT_EXEC)) - { - TRACE( "forcing exec permission on %p-%p\n", base, (char *)base + size - 1 ); - if (!mprotect( base, size, unix_prot | PROT_EXEC )) goto done; - /* exec + write may legitimately fail, in that case fall back to write only */ - if (!(unix_prot & PROT_WRITE)) return FALSE; - } + if (mprotect_exec( base, size, unix_prot, view->protect )) /* FIXME: last error */ + return FALSE; - if (mprotect( base, size, unix_prot )) return FALSE; /* FIXME: last error */ - -done: memset( p, vprot, size >> page_shift ); VIRTUAL_DEBUG_DUMP_VIEW( view ); return TRUE;