From a2089abd948564ff4aa930ebac8f8894b49a6f39 Mon Sep 17 00:00:00 2001 From: Alexandre Julliard Date: Thu, 11 Dec 2008 14:05:42 +0100 Subject: [PATCH] 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 0e53a1c7fe6..0fdd69e9c14 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 752646559b4..d3a006712db 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 96164779436..da08e2f5b56 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) { -- 2.11.4.GIT