Alexandre Julliard : ntdll: Enforce correct protection values in virtual memory functions.
Module: wine Branch: master Commit: a2089abd948564ff4aa930ebac8f8894b49a6f39 URL: http://source.winehq.org/git/wine.git/?a=commit;h=a2089abd948564ff4aa930ebac...
Author: Alexandre Julliard julliard@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) {
-
Alexandre Julliard