Alexandre Julliard : ntdll: Enforce correct protection values in virtual memory functions.

[Alexandre Julliard]

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

Author: Alexandre Julliard <julliard(a)winehq.org>
Date:   Thu Dec 11 14:05:42 2008 +0100

ntdll: Enforce correct protection values in virtual memory functions.

---

 dlls/kernel32/tests/virtual.c |   26 +++++++++++++++++++++++
 dlls/ntdll/loader.c           |    2 +-
 dlls/ntdll/virtual.c          |   46 ++++++++++++++++++++++------------------
 3 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c
index 0e53a1c..0fdd69e 100644
--- a/dlls/kernel32/tests/virtual.c
+++ b/dlls/kernel32/tests/virtual.c
@@ -255,6 +255,32 @@ static void test_VirtualAlloc(void)
     ok(old_prot == PAGE_READONLY,
         "wrong old protection: got %04x instead of PAGE_READONLY\n", old_prot);
 
+    /* invalid protection values */
+    SetLastError(0xdeadbeef);
+    addr2 = VirtualAlloc(NULL, 0x1000, MEM_RESERVE, 0);
+    ok(!addr2, "VirtualAlloc succeeded\n");
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "wrong error %u\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    addr2 = VirtualAlloc(NULL, 0x1000, MEM_COMMIT, 0);
+    ok(!addr2, "VirtualAlloc succeeded\n");
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "wrong error %u\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    addr2 = VirtualAlloc(addr1, 0x1000, MEM_COMMIT, PAGE_READONLY | PAGE_EXECUTE);
+    ok(!addr2, "VirtualAlloc succeeded\n");
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "wrong error %u\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    ok(!VirtualProtect(addr1, 0x1000, PAGE_READWRITE | PAGE_EXECUTE_WRITECOPY, &old_prot),
+       "VirtualProtect succeeded\n");
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "wrong error %u\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    ok(!VirtualProtect(addr1, 0x1000, 0, &old_prot), "VirtualProtect succeeded\n");
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "wrong error %u\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
     ok(!VirtualFree(addr1, 0x10000, 0), "VirtualFree should fail with type 0\n");
     ok(GetLastError() == ERROR_INVALID_PARAMETER,
         "got %d, expected ERROR_INVALID_PARAMETER\n", GetLastError());
diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c
index 7526465..d3a0067 100644
--- a/dlls/ntdll/loader.c
+++ b/dlls/ntdll/loader.c
@@ -1464,7 +1464,7 @@ static NTSTATUS load_native_dll( LPCWSTR load_path, LPCWSTR name, HANDLE file,
     size.QuadPart = 0;
 
     status = NtCreateSection( &mapping, STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ,
-                              &attr, &size, 0, SEC_IMAGE, file );
+                              &attr, &size, PAGE_READONLY, SEC_IMAGE, file );
     if (status != STATUS_SUCCESS) return status;
 
     module = NULL;
diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c
index 9616477..da08e2f 100644
--- a/dlls/ntdll/virtual.c
+++ b/dlls/ntdll/virtual.c
@@ -503,7 +503,7 @@ static DWORD VIRTUAL_GetWin32Prot( BYTE vprot )
 
 
 /***********************************************************************
- *           VIRTUAL_GetProt
+ *           get_vprot_flags
  *
  * Build page protections from Win32 flags.
  *
@@ -513,41 +513,40 @@ static DWORD VIRTUAL_GetWin32Prot( BYTE vprot )
  * RETURNS
  *	Value of page protection flags
  */
-static BYTE VIRTUAL_GetProt( DWORD protect )
+static NTSTATUS get_vprot_flags( DWORD protect, unsigned int *vprot )
 {
-    BYTE vprot;
-
     switch(protect & 0xff)
     {
     case PAGE_READONLY:
-        vprot = VPROT_READ;
+        *vprot = VPROT_READ;
         break;
     case PAGE_READWRITE:
-        vprot = VPROT_READ | VPROT_WRITE;
+        *vprot = VPROT_READ | VPROT_WRITE;
         break;
     case PAGE_WRITECOPY:
-        vprot = VPROT_READ | VPROT_WRITECOPY;
+        *vprot = VPROT_READ | VPROT_WRITECOPY;
         break;
     case PAGE_EXECUTE:
-        vprot = VPROT_EXEC;
+        *vprot = VPROT_EXEC;
         break;
     case PAGE_EXECUTE_READ:
-        vprot = VPROT_EXEC | VPROT_READ;
+        *vprot = VPROT_EXEC | VPROT_READ;
         break;
     case PAGE_EXECUTE_READWRITE:
-        vprot = VPROT_EXEC | VPROT_READ | VPROT_WRITE;
+        *vprot = VPROT_EXEC | VPROT_READ | VPROT_WRITE;
         break;
     case PAGE_EXECUTE_WRITECOPY:
-        vprot = VPROT_EXEC | VPROT_READ | VPROT_WRITECOPY;
+        *vprot = VPROT_EXEC | VPROT_READ | VPROT_WRITECOPY;
         break;
     case PAGE_NOACCESS:
-    default:
-        vprot = 0;
+        *vprot = 0;
         break;
+    default:
+        return STATUS_INVALID_PARAMETER;
     }
-    if (protect & PAGE_GUARD) vprot |= VPROT_GUARD;
-    if (protect & PAGE_NOCACHE) vprot |= VPROT_NOCACHE;
-    return vprot;
+    if (protect & PAGE_GUARD) *vprot |= VPROT_GUARD;
+    if (protect & PAGE_NOCACHE) *vprot |= VPROT_NOCACHE;
+    return STATUS_SUCCESS;
 }
 
 
@@ -1632,7 +1631,8 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_
 
     if (is_beyond_limit( 0, size, working_set_limit )) return STATUS_WORKING_SET_LIMIT_RANGE;
 
-    vprot = VIRTUAL_GetProt( protect ) | VPROT_VALLOC;
+    if ((status = get_vprot_flags( protect, &vprot ))) return status;
+    vprot |= VPROT_VALLOC;
     if (type & MEM_COMMIT) vprot |= VPROT_COMMITTED;
 
     if (*ret)
@@ -1811,6 +1811,7 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T
     NTSTATUS status = STATUS_SUCCESS;
     char *base;
     BYTE vprot;
+    unsigned int new_vprot;
     SIZE_T size = *size_ptr;
     LPVOID addr = *addr_ptr;
 
@@ -1843,6 +1844,8 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T
 
     size = ROUND_SIZE( addr, size );
     base = ROUND_ADDR( addr, page_mask );
+    if ((status = get_vprot_flags( new_prot, &new_vprot ))) return status;
+    new_vprot |= VPROT_COMMITTED;
 
     server_enter_uninterrupted_section( &csVirtual, &sigset );
 
@@ -1856,8 +1859,7 @@ NTSTATUS WINAPI NtProtectVirtualMemory( HANDLE process, PVOID *addr_ptr, SIZE_T
         if (get_committed_size( view, base, &vprot ) >= size && (vprot & VPROT_COMMITTED))
         {
             if (old_prot) *old_prot = VIRTUAL_GetWin32Prot( vprot );
-            vprot = VIRTUAL_GetProt( new_prot ) | VPROT_COMMITTED;
-            if (!VIRTUAL_SetProt( view, base, size, vprot )) status = STATUS_ACCESS_DENIED;
+            if (!VIRTUAL_SetProt( view, base, size, new_vprot )) status = STATUS_ACCESS_DENIED;
         }
         else status = STATUS_NOT_COMMITTED;
     }
@@ -2135,6 +2137,8 @@ NTSTATUS WINAPI NtCreateSection( HANDLE *handle, ACCESS_MASK access, const OBJEC
 
     if (len > MAX_PATH*sizeof(WCHAR)) return STATUS_NAME_TOO_LONG;
 
+    if ((ret = get_vprot_flags( protect, &vprot ))) return ret;
+
     objattr.rootdir = wine_server_obj_handle( attr ? attr->RootDirectory : 0 );
     objattr.sd_len = 0;
     objattr.name_len = len;
@@ -2144,7 +2148,6 @@ NTSTATUS WINAPI NtCreateSection( HANDLE *handle, ACCESS_MASK access, const OBJEC
         if (ret != STATUS_SUCCESS) return ret;
     }
 
-    vprot = VIRTUAL_GetProt( protect );
     if (!(sec_flags & SEC_RESERVE)) vprot |= VPROT_COMMITTED;
     if (sec_flags & SEC_NOCACHE) vprot |= VPROT_NOCACHE;
     if (sec_flags & SEC_IMAGE) vprot |= VPROT_IMAGE;
@@ -2331,7 +2334,8 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p
 
     server_enter_uninterrupted_section( &csVirtual, &sigset );
 
-    vprot = VIRTUAL_GetProt( protect ) | (map_vprot & VPROT_COMMITTED);
+    get_vprot_flags( protect, &vprot );
+    vprot |= (map_vprot & VPROT_COMMITTED);
     res = map_view( &view, *addr_ptr, size, mask, FALSE, vprot );
     if (res)
     {