Sebastian Lackner : ntdll: Ensure force_exec_prot is also used for views with write watch permissions.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: aa3c6111d9361d3fddb4cdbe2365e356e740f1b9
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=aa3c6111d9361d3fddb4cdbe23...

Author: Sebastian Lackner 
Date:   Wed Oct  8 21:26:28 2014 +0200

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 4d8277d..2d2554b 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 f8a5dd3..3c9a4b5 100644
--- a/dlls/ntdll/virtual.c
+++ b/dlls/ntdll/virtual.c
@@ -591,6 +591,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
  *
  * Change the protection of a range of pages.
@@ -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( base, size, unix_prot )) return FALSE;  /* FIXME: last error */
+    if (mprotect_exec( base, size, unix_prot, view->protect )) /* FIXME: last error */
+        return FALSE;

-done:
     memset( p, vprot, size >> page_shift );
     VIRTUAL_DEBUG_DUMP_VIEW( view );
     return TRUE;