From 5e2ed6bfe94eb322efe891c07aa8e58b125f36c2 Mon Sep 17 00:00:00 2001 From: Alexandre Julliard Date: Wed, 5 Nov 2008 20:32:32 +0100 Subject: [PATCH] ntdll,server: Fixed access checks for OpenFileMapping and MapViewOfFile. --- dlls/kernel32/tests/virtual.c | 4 +-- dlls/kernel32/virtual.c | 19 ++++++++---- dlls/ntdll/virtual.c | 65 ++++++++++++++++++------------------------ include/wine/server_protocol.h | 3 +- server/mapping.c | 2 +- server/protocol.def | 1 + server/trace.c | 3 +- 7 files changed, 49 insertions(+), 48 deletions(-) diff --git a/dlls/kernel32/tests/virtual.c b/dlls/kernel32/tests/virtual.c index 032af2b9c8b..2c30973e399 100644 --- a/dlls/kernel32/tests/virtual.c +++ b/dlls/kernel32/tests/virtual.c @@ -482,7 +482,7 @@ static void test_MapViewOfFile(void) ok( info.State == MEM_COMMIT, "%x != MEM_COMMIT\n", info.State ); ok( info.Protect == PAGE_READONLY, "%x != PAGE_READONLY\n", info.Protect ); } - else todo_wine win_skip( "no access checks on win9x\n" ); + else win_skip( "no access checks on win9x\n" ); UnmapViewOfFile( ptr ); CloseHandle( mapping ); @@ -507,7 +507,7 @@ static void test_MapViewOfFile(void) ok( info.State == MEM_COMMIT, "%x != MEM_COMMIT\n", info.State ); ok( info.Protect == PAGE_READWRITE, "%x != PAGE_READWRITE\n", info.Protect ); } - else todo_wine win_skip( "no access checks on win9x\n" ); + else win_skip( "no access checks on win9x\n" ); UnmapViewOfFile( ptr ); CloseHandle( mapping ); diff --git a/dlls/kernel32/virtual.c b/dlls/kernel32/virtual.c index 38a408a14fe..4db68ca7921 100644 --- a/dlls/kernel32/virtual.c +++ b/dlls/kernel32/virtual.c @@ -353,20 +353,20 @@ HANDLE WINAPI CreateFileMappingW( HANDLE hFile, LPSECURITY_ATTRIBUTES sa, /* fall through */ case PAGE_READONLY: case PAGE_WRITECOPY: - access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ; + access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ | SECTION_MAP_EXECUTE; break; case PAGE_READWRITE: - access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ | SECTION_MAP_WRITE; + access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ | SECTION_MAP_WRITE | SECTION_MAP_EXECUTE; break; case PAGE_EXECUTE: - access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_EXECUTE; + access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_EXECUTE | SECTION_MAP_EXECUTE_EXPLICIT; break; case PAGE_EXECUTE_READ: case PAGE_EXECUTE_WRITECOPY: - access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ | SECTION_MAP_EXECUTE; + access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ | SECTION_MAP_EXECUTE | SECTION_MAP_EXECUTE_EXPLICIT; break; case PAGE_EXECUTE_READWRITE: - access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ | SECTION_MAP_WRITE | SECTION_MAP_EXECUTE; + access = STANDARD_RIGHTS_REQUIRED | SECTION_QUERY | SECTION_MAP_READ | SECTION_MAP_WRITE | SECTION_MAP_EXECUTE | SECTION_MAP_EXECUTE_EXPLICIT; break; default: SetLastError( ERROR_INVALID_PARAMETER ); @@ -469,7 +469,14 @@ HANDLE WINAPI OpenFileMappingW( DWORD access, BOOL inherit, LPCWSTR name) attr.SecurityQualityOfService = NULL; RtlInitUnicodeString( &nameW, name ); - if (access == FILE_MAP_COPY) access = FILE_MAP_READ; + if (access & FILE_MAP_COPY) access |= SECTION_MAP_READ; + access |= STANDARD_RIGHTS_REQUIRED | SECTION_QUERY; + + if (GetVersion() & 0x80000000) + { + /* win9x doesn't do access checks, so try with full access first */ + if (!NtOpenSection( &ret, access | SECTION_MAP_READ | SECTION_MAP_WRITE, &attr )) return ret; + } if ((status = NtOpenSection( &ret, access, &attr ))) { diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index a7ea2b35e36..6fd86317246 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -520,13 +520,6 @@ static BYTE VIRTUAL_GetProt( DWORD protect ) vprot = VPROT_READ | VPROT_WRITE; break; case PAGE_WRITECOPY: - /* MSDN CreateFileMapping() states that if PAGE_WRITECOPY is given, - * that the hFile must have been opened with GENERIC_READ and - * GENERIC_WRITE access. This is WRONG as tests show that you - * only need GENERIC_READ access (at least for Win9x, - * FIXME: what about NT?). Thus, we don't put VPROT_WRITE in - * PAGE_WRITECOPY and PAGE_EXECUTE_WRITECOPY. - */ vprot = VPROT_READ | VPROT_WRITECOPY; break; case PAGE_EXECUTE: @@ -539,7 +532,6 @@ static BYTE VIRTUAL_GetProt( DWORD protect ) vprot = VPROT_EXEC | VPROT_READ | VPROT_WRITE; break; case PAGE_EXECUTE_WRITECOPY: - /* See comment for PAGE_WRITECOPY above */ vprot = VPROT_EXEC | VPROT_READ | VPROT_WRITECOPY; break; case PAGE_NOACCESS: @@ -2072,10 +2064,11 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p { NTSTATUS res; ULONGLONG full_size; + ACCESS_MASK access; SIZE_T size = 0; SIZE_T mask = get_mask( zero_bits ); int unix_handle = -1, needs_close; - unsigned int prot; + unsigned int map_vprot, vprot; void *base; struct file_view *view; DWORD header_size; @@ -2119,11 +2112,32 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p return result.map_view.status; } + switch(protect) + { + case PAGE_NOACCESS: + access = SECTION_QUERY; + break; + case PAGE_READWRITE: + case PAGE_EXECUTE_READWRITE: + access = SECTION_QUERY | SECTION_MAP_WRITE; + break; + case PAGE_READONLY: + case PAGE_WRITECOPY: + case PAGE_EXECUTE: + case PAGE_EXECUTE_READ: + case PAGE_EXECUTE_WRITECOPY: + access = SECTION_QUERY | SECTION_MAP_READ; + break; + default: + return STATUS_INVALID_PARAMETER; + } + SERVER_START_REQ( get_mapping_info ) { req->handle = handle; + req->access = access; res = wine_server_call( req ); - prot = reply->protect; + map_vprot = reply->protect; base = reply->base; full_size = reply->size; header_size = reply->header_size; @@ -2140,7 +2154,7 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p if ((res = server_get_unix_fd( handle, 0, &unix_handle, &needs_close, NULL, NULL ))) goto done; - if (prot & VPROT_IMAGE) + if (map_vprot & VPROT_IMAGE) { if (shared_file) { @@ -2171,35 +2185,12 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p if (*size_ptr) size = ROUND_SIZE( offset.u.LowPart, *size_ptr ); else size = size - offset.QuadPart; - switch(protect) - { - case PAGE_NOACCESS: - break; - case PAGE_READWRITE: - case PAGE_EXECUTE_READWRITE: - if (!(prot & VPROT_WRITE)) - { - res = STATUS_INVALID_PARAMETER; - goto done; - } - /* fall through */ - case PAGE_READONLY: - case PAGE_WRITECOPY: - case PAGE_EXECUTE: - case PAGE_EXECUTE_READ: - case PAGE_EXECUTE_WRITECOPY: - if (prot & VPROT_READ) break; - /* fall through */ - default: - res = STATUS_INVALID_PARAMETER; - goto done; - } - /* Reserve a properly aligned area */ server_enter_uninterrupted_section( &csVirtual, &sigset ); - res = map_view( &view, *addr_ptr, size, mask, FALSE, prot ); + vprot = VIRTUAL_GetProt( protect ) | (map_vprot & VPROT_COMMITTED); + res = map_view( &view, *addr_ptr, size, mask, FALSE, vprot ); if (res) { server_leave_uninterrupted_section( &csVirtual, &sigset ); @@ -2211,7 +2202,7 @@ NTSTATUS WINAPI NtMapViewOfSection( HANDLE handle, HANDLE process, PVOID *addr_p TRACE("handle=%p size=%lx offset=%x%08x\n", handle, size, offset.u.HighPart, offset.u.LowPart ); - res = map_file_into_view( view, unix_handle, 0, size, offset.QuadPart, prot, !dup_mapping ); + res = map_file_into_view( view, unix_handle, 0, size, offset.QuadPart, vprot, !dup_mapping ); if (res == STATUS_SUCCESS) { *addr_ptr = view->base; diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index dbca959bd35..c5a8a5e500b 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -1716,6 +1716,7 @@ struct get_mapping_info_request { struct request_header __header; obj_handle_t handle; + unsigned int access; }; struct get_mapping_info_reply { @@ -5070,6 +5071,6 @@ union generic_reply struct set_window_layered_info_reply set_window_layered_info_reply; }; -#define SERVER_PROTOCOL_VERSION 345 +#define SERVER_PROTOCOL_VERSION 346 #endif /* __WINE_WINE_SERVER_PROTOCOL_H */ diff --git a/server/mapping.c b/server/mapping.c index 34fc6ced096..f5df22fdcf6 100644 --- a/server/mapping.c +++ b/server/mapping.c @@ -562,7 +562,7 @@ DECL_HANDLER(get_mapping_info) struct fd *fd; if ((mapping = (struct mapping *)get_handle_obj( current->process, req->handle, - 0, &mapping_ops ))) + req->access | SECTION_QUERY, &mapping_ops ))) { reply->size = mapping->size; reply->protect = mapping->protect; diff --git a/server/protocol.def b/server/protocol.def index 0969330f773..bb7b47cdeae 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1351,6 +1351,7 @@ enum char_info_mode /* Get information about a file mapping */ @REQ(get_mapping_info) obj_handle_t handle; /* handle to the mapping */ + unsigned int access; /* wanted access rights */ @REPLY file_pos_t size; /* mapping size */ int protect; /* protection flags */ diff --git a/server/trace.c b/server/trace.c index 38cabcbe01d..8b6e622f033 100644 --- a/server/trace.c +++ b/server/trace.c @@ -1769,7 +1769,8 @@ static void dump_open_mapping_reply( const struct open_mapping_reply *req ) static void dump_get_mapping_info_request( const struct get_mapping_info_request *req ) { - fprintf( stderr, " handle=%p", req->handle ); + fprintf( stderr, " handle=%p,", req->handle ); + fprintf( stderr, " access=%08x", req->access ); } static void dump_get_mapping_info_reply( const struct get_mapping_info_reply *req ) -- 2.11.4.GIT