[PATCH v4 0/8] MR1025: ntdll: Support memory address requirements in NtAllocateVirtualMemoryEx()
Various notes: - LowestStartingAddress is still unsupported; - Patch 5 ("wow64: Set HighestEndingAddress in wow64_NtAllocateVirtualMemoryEx() if it is absent") is fixing existing WOW issue: NtAllocateVirtualMemoryEx() called from wow64 currently does not constraing the allocation to 32 bit address space (wow64_NtAllocateVirtualMemory() passes zero_bits for that). - I initially thought of using a single inter process APC but added a different one due to zero_bits handling which is easier to convert in the target process. -- v4: ntdll: Support specified alignment in NtAllocateVirtualMemoryEx(). ntdll: Pass alignemnt mask to map_view(). wow64: Set HighestEndingAddress in wow64_NtAllocateVirtualMemoryEx() if it is absent. wow64: Support MEM_ADDRESS_REQUIREMENTS in wow64_NtAllocateVirtualMemoryEx(). ntdll/tests: Add tests for memory address requiements. ntdll: Support HighestEndingAddress in NtAllocateVirtualMemoryEx(). ntdll: Factor out allocate_virtual_memory(). ntdll: Pass limit instead of zero_bits to map_view(). https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index d8b888aa49e..ffdefc8acd0 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -1147,7 +1147,7 @@ static inline UINT_PTR get_zero_bits_mask( ULONG_PTR zero_bits ) { unsigned int shift; - if (zero_bits == 0) return ~(UINT_PTR)0; + if (zero_bits == 0) return 0; if (zero_bits < 32) shift = 32 + zero_bits; else @@ -1895,7 +1895,7 @@ static NTSTATUS map_fixed_area( void *base, size_t size, unsigned int vprot ) * virtual_mutex must be held by caller. */ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, - int top_down, unsigned int vprot, ULONG_PTR zero_bits ) + int top_down, unsigned int vprot, ULONG_PTR limit ) { void *ptr; NTSTATUS status; @@ -1915,7 +1915,7 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, alloc.size = size; alloc.top_down = top_down; - alloc.limit = (void*)(get_zero_bits_mask( zero_bits ) & (UINT_PTR)user_space_limit); + alloc.limit = limit ? (void*)(limit & (UINT_PTR)user_space_limit) : user_space_limit; if (mmap_enum_reserved_areas( alloc_reserved_area_callback, &alloc, top_down )) { @@ -1926,7 +1926,7 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, goto done; } - if (zero_bits) + if (limit) { if (!(ptr = map_free_area( address_space_start, alloc.limit, size, top_down, get_unix_prot(vprot) ))) @@ -2451,9 +2451,9 @@ static NTSTATUS virtual_map_image( HANDLE mapping, ACCESS_MASK access, void **ad if ((ULONG_PTR)base != image_info->base) base = NULL; if ((char *)base >= (char *)address_space_start) /* make sure the DOS area remains free */ - status = map_view( &view, base, size, alloc_type & MEM_TOP_DOWN, vprot, zero_bits ); + status = map_view( &view, base, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ) ); - if (status) status = map_view( &view, NULL, size, alloc_type & MEM_TOP_DOWN, vprot, zero_bits ); + if (status) status = map_view( &view, NULL, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits )); if (status) goto done; status = map_image_into_view( view, filename, unix_fd, base, image_info->header_size, @@ -2576,7 +2576,7 @@ static NTSTATUS virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_PTR z server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - res = map_view( &view, base, size, alloc_type & MEM_TOP_DOWN, vprot, zero_bits ); + res = map_view( &view, base, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ) ); if (res) goto done; TRACE( "handle=%p size=%lx offset=%x%08x\n", handle, size, offset.u.HighPart, offset.u.LowPart ); @@ -3114,7 +3114,8 @@ NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, ULONG_PTR zero_bits, SI server_enter_uninterrupted_section( &virtual_mutex, &sigset ); if ((status = map_view( &view, NULL, size + extra_size, FALSE, - VPROT_READ | VPROT_WRITE | VPROT_COMMITTED, zero_bits )) != STATUS_SUCCESS) + VPROT_READ | VPROT_WRITE | VPROT_COMMITTED, get_zero_bits_mask( zero_bits ) )) + != STATUS_SUCCESS) goto done; #ifdef VALGRIND_STACK_REGISTER @@ -3815,7 +3816,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG_PTR z if (vprot & VPROT_WRITECOPY) status = STATUS_INVALID_PAGE_PROTECTION; else if (is_dos_memory) status = allocate_dos_memory( &view, vprot ); - else status = map_view( &view, base, size, type & MEM_TOP_DOWN, vprot, zero_bits ); + else status = map_view( &view, base, size, type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ) ); if (status == STATUS_SUCCESS) base = view->base; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 97 ++++++++++++++++++++++++--------------- 1 file changed, 59 insertions(+), 38 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index ffdefc8acd0..2f0b5e78dc8 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -3718,11 +3718,12 @@ void virtual_set_large_address_space(void) /*********************************************************************** - * NtAllocateVirtualMemory (NTDLL.@) - * ZwAllocateVirtualMemory (NTDLL.@) + * allocate_virtual_memory + * + * NtAllocateVirtualMempory[Ex] implementation. */ -NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG_PTR zero_bits, - SIZE_T *size_ptr, ULONG type, ULONG protect ) +static NTSTATUS allocate_virtual_memory( void **ret, SIZE_T *size_ptr, ULONG type, ULONG protect, + ULONG_PTR limit ) { void *base; unsigned int vprot; @@ -3732,39 +3733,6 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG_PTR z SIZE_T size = *size_ptr; NTSTATUS status = STATUS_SUCCESS; - TRACE("%p %p %08lx %x %08x\n", process, *ret, size, type, protect ); - - if (!size) return STATUS_INVALID_PARAMETER; - if (zero_bits > 21 && zero_bits < 32) return STATUS_INVALID_PARAMETER_3; - if (zero_bits > 32 && zero_bits < granularity_mask) return STATUS_INVALID_PARAMETER_3; -#ifndef _WIN64 - if (!is_wow64 && zero_bits >= 32) return STATUS_INVALID_PARAMETER_3; -#endif - - if (process != NtCurrentProcess()) - { - apc_call_t call; - apc_result_t result; - - memset( &call, 0, sizeof(call) ); - - call.virtual_alloc.type = APC_VIRTUAL_ALLOC; - call.virtual_alloc.addr = wine_server_client_ptr( *ret ); - call.virtual_alloc.size = *size_ptr; - call.virtual_alloc.zero_bits = zero_bits; - call.virtual_alloc.op_type = type; - call.virtual_alloc.prot = protect; - status = server_queue_process_apc( process, &call, &result ); - if (status != STATUS_SUCCESS) return status; - - if (result.virtual_alloc.status == STATUS_SUCCESS) - { - *ret = wine_server_get_ptr( result.virtual_alloc.addr ); - *size_ptr = result.virtual_alloc.size; - } - return result.virtual_alloc.status; - } - /* Round parameters to a page boundary */ if (is_beyond_limit( 0, size, working_set_limit )) return STATUS_WORKING_SET_LIMIT_RANGE; @@ -3816,7 +3784,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG_PTR z if (vprot & VPROT_WRITECOPY) status = STATUS_INVALID_PAGE_PROTECTION; else if (is_dos_memory) status = allocate_dos_memory( &view, vprot ); - else status = map_view( &view, base, size, type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ) ); + else status = map_view( &view, base, size, type & MEM_TOP_DOWN, vprot, limit ); if (status == STATUS_SUCCESS) base = view->base; } @@ -3855,6 +3823,59 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG_PTR z return status; } + +/*********************************************************************** + * NtAllocateVirtualMemory (NTDLL.@) + * ZwAllocateVirtualMemory (NTDLL.@) + */ +NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG_PTR zero_bits, + SIZE_T *size_ptr, ULONG type, ULONG protect ) +{ + ULONG_PTR limit; + + TRACE("%p %p %08lx %x %08x\n", process, *ret, *size_ptr, type, protect ); + + if (!*size_ptr) return STATUS_INVALID_PARAMETER; + if (zero_bits > 21 && zero_bits < 32) return STATUS_INVALID_PARAMETER_3; + if (zero_bits > 32 && zero_bits < granularity_mask) return STATUS_INVALID_PARAMETER_3; +#ifndef _WIN64 + if (!is_wow64 && zero_bits >= 32) return STATUS_INVALID_PARAMETER_3; +#endif + + if (process != NtCurrentProcess()) + { + apc_call_t call; + apc_result_t result; + NTSTATUS status; + + memset( &call, 0, sizeof(call) ); + + call.virtual_alloc.type = APC_VIRTUAL_ALLOC; + call.virtual_alloc.addr = wine_server_client_ptr( *ret ); + call.virtual_alloc.size = *size_ptr; + call.virtual_alloc.zero_bits = zero_bits; + call.virtual_alloc.op_type = type; + call.virtual_alloc.prot = protect; + status = server_queue_process_apc( process, &call, &result ); + if (status != STATUS_SUCCESS) return status; + + if (result.virtual_alloc.status == STATUS_SUCCESS) + { + *ret = wine_server_get_ptr( result.virtual_alloc.addr ); + *size_ptr = result.virtual_alloc.size; + } + return result.virtual_alloc.status; + } + + if (!*ret) + limit = get_zero_bits_mask( zero_bits ); + else + limit = 0; + + return allocate_virtual_memory( ret, size_ptr, type, protect, limit ); +} + + /*********************************************************************** * NtAllocateVirtualMemoryEx (NTDLL.@) * ZwAllocateVirtualMemoryEx (NTDLL.@) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/server.c | 29 ++++++++++++++- dlls/ntdll/unix/virtual.c | 75 +++++++++++++++++++++++++++++++++++++-- include/winnt.h | 7 ++++ server/protocol.def | 17 +++++++++ server/thread.c | 1 + server/trace.c | 6 ++++ 6 files changed, 131 insertions(+), 4 deletions(-) diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index 77e8d5c7566..b7d8733f2bc 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -358,7 +358,7 @@ static NTSTATUS invoke_user_apc( CONTEXT *context, const user_apc_t *apc, NTSTAT */ static void invoke_system_apc( const apc_call_t *call, apc_result_t *result, BOOL self ) { - SIZE_T size, bits; + SIZE_T size, bits, limit; void *addr; memset( result, 0, sizeof(*result) ); @@ -401,6 +401,33 @@ static void invoke_system_apc( const apc_call_t *call, apc_result_t *result, BOO } else result->virtual_alloc.status = STATUS_WORKING_SET_LIMIT_RANGE; break; + case APC_VIRTUAL_ALLOC_EX: + { + MEM_ADDRESS_REQUIREMENTS r = { NULL }; + MEM_EXTENDED_PARAMETER ext = + { + .Type = MemExtendedParameterAddressRequirements, + .Pointer = &r + }; + SYSTEM_BASIC_INFORMATION sbi; + + virtual_get_system_info( &sbi, !!NtCurrentTeb()->WowTebOffset ); + result->type = call->type; + addr = wine_server_get_ptr( call->virtual_alloc_ex.addr ); + size = call->virtual_alloc_ex.size; + limit = min( (ULONG_PTR)sbi.HighestUserAddress, call->virtual_alloc_ex.limit ); + if ((ULONG_PTR)addr == call->virtual_alloc_ex.addr && size == call->virtual_alloc_ex.size) + { + r.HighestEndingAddress = (void *)limit; + result->virtual_alloc_ex.status = NtAllocateVirtualMemoryEx( NtCurrentProcess(), &addr, &size, + call->virtual_alloc_ex.op_type, + call->virtual_alloc_ex.prot, &ext, 1 ); + result->virtual_alloc_ex.addr = wine_server_client_ptr( addr ); + result->virtual_alloc_ex.size = size; + } + else result->virtual_alloc_ex.status = STATUS_WORKING_SET_LIMIT_RANGE; + break; + } case APC_VIRTUAL_FREE: result->type = call->type; addr = wine_server_get_ptr( call->virtual_free.addr ); diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 2f0b5e78dc8..6304417415e 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -1915,7 +1915,7 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, alloc.size = size; alloc.top_down = top_down; - alloc.limit = limit ? (void*)(limit & (UINT_PTR)user_space_limit) : user_space_limit; + alloc.limit = limit ? min( (void *)(limit + 1), user_space_limit ) : user_space_limit; if (mmap_enum_reserved_areas( alloc_reserved_area_callback, &alloc, top_down )) { @@ -3884,11 +3884,80 @@ NTSTATUS WINAPI NtAllocateVirtualMemoryEx( HANDLE process, PVOID *ret, SIZE_T *s ULONG protect, MEM_EXTENDED_PARAMETER *parameters, ULONG count ) { + ULONG_PTR limit = 0; + + TRACE("%p %p %08lx %x %08x %p %u\n", process, *ret, *size_ptr, type, protect, parameters, count ); + if (count && !parameters) return STATUS_INVALID_PARAMETER; - if (count) FIXME( "Ignoring %d extended parameters %p\n", count, parameters ); + if (count) + { + int mem_requirements_found = 0; + MEM_ADDRESS_REQUIREMENTS *r; + unsigned int i; + + for (i = 0; i < count; ++i) + { + if (parameters[i].Type == MemExtendedParameterInvalidType || parameters[i].Type >= MemExtendedParameterMax) + { + WARN( "Invalid parameter type %d.\n", parameters[i].Type ); + return STATUS_INVALID_PARAMETER; + } + if (parameters[i].Type != MemExtendedParameterAddressRequirements) + { + FIXME( "Parameter type %d is not supported.\n", parameters[i].Type ); + continue; + } + if (mem_requirements_found) + { + WARN( "Duplicate parameter.\n" ); + return STATUS_INVALID_PARAMETER; + } + mem_requirements_found = 1; + r = (MEM_ADDRESS_REQUIREMENTS *)parameters[i].Pointer; + + if (r->LowestStartingAddress || r->Alignment) + FIXME( "Not supported requirements LowestStartingAddress %p, Alignment %p.\n", + r->LowestStartingAddress, (void *)r->Alignment ); + + limit = (ULONG_PTR)r->HighestEndingAddress; + if (limit && (*ret || limit > (ULONG_PTR)user_space_limit || ((limit + 1) & (page_mask - 1)))) + { + WARN( "Invalid limit %p.\n", r->HighestEndingAddress); + return STATUS_INVALID_PARAMETER; + } + TRACE( "limit %p.\n", (void *)limit ); + } + } + + if (!*size_ptr) return STATUS_INVALID_PARAMETER; + + if (process != NtCurrentProcess()) + { + apc_call_t call; + apc_result_t result; + NTSTATUS status; + + memset( &call, 0, sizeof(call) ); + + call.virtual_alloc_ex.type = APC_VIRTUAL_ALLOC_EX; + call.virtual_alloc_ex.addr = wine_server_client_ptr( *ret ); + call.virtual_alloc_ex.size = *size_ptr; + call.virtual_alloc_ex.limit = limit; + call.virtual_alloc_ex.op_type = type; + call.virtual_alloc_ex.prot = protect; + status = server_queue_process_apc( process, &call, &result ); + if (status != STATUS_SUCCESS) return status; + + if (result.virtual_alloc_ex.status == STATUS_SUCCESS) + { + *ret = wine_server_get_ptr( result.virtual_alloc_ex.addr ); + *size_ptr = result.virtual_alloc_ex.size; + } + return result.virtual_alloc_ex.status; + } - return NtAllocateVirtualMemory( process, ret, 0, size_ptr, type, protect ); + return allocate_virtual_memory( ret, size_ptr, type, protect, limit ); } diff --git a/include/winnt.h b/include/winnt.h index 836bd7123e5..f5b01ca39ca 100644 --- a/include/winnt.h +++ b/include/winnt.h @@ -759,6 +759,13 @@ typedef struct _MEMORY_BASIC_INFORMATION DWORD Type; } MEMORY_BASIC_INFORMATION, *PMEMORY_BASIC_INFORMATION; +typedef struct _MEM_ADDRESS_REQUIREMENTS +{ + void *LowestStartingAddress; + void *HighestEndingAddress; + SIZE_T Alignment; +} MEM_ADDRESS_REQUIREMENTS, *PMEM_ADDRESS_REQUIREMENTS; + #define MEM_EXTENDED_PARAMETER_TYPE_BITS 8 typedef enum MEM_EXTENDED_PARAMETER_TYPE { diff --git a/server/protocol.def b/server/protocol.def index 86fb10951f0..06977c29054 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -493,6 +493,7 @@ enum apc_type APC_USER, APC_ASYNC_IO, APC_VIRTUAL_ALLOC, + APC_VIRTUAL_ALLOC_EX, APC_VIRTUAL_FREE, APC_VIRTUAL_QUERY, APC_VIRTUAL_PROTECT, @@ -535,6 +536,15 @@ typedef union unsigned int prot; /* memory protection flags */ } virtual_alloc; struct + { + enum apc_type type; /* APC_VIRTUAL_ALLOC */ + unsigned int op_type; /* type of operation */ + client_ptr_t addr; /* requested address */ + mem_size_t size; /* allocation size */ + mem_size_t limit; /* allocation address limit */ + unsigned int prot; /* memory protection flags */ + } virtual_alloc_ex; + struct { enum apc_type type; /* APC_VIRTUAL_FREE */ unsigned int op_type; /* type of operation */ @@ -630,6 +640,13 @@ typedef union mem_size_t size; /* resulting size */ } virtual_alloc; struct + { + enum apc_type type; /* APC_VIRTUAL_ALLOC */ + unsigned int status; /* status returned by call */ + client_ptr_t addr; /* resulting address */ + mem_size_t size; /* resulting size */ + } virtual_alloc_ex; + struct { enum apc_type type; /* APC_VIRTUAL_FREE */ unsigned int status; /* status returned by call */ diff --git a/server/thread.c b/server/thread.c index f49fbf40b78..ab783cc3a7d 100644 --- a/server/thread.c +++ b/server/thread.c @@ -1711,6 +1711,7 @@ DECL_HANDLER(queue_apc) thread = get_thread_from_handle( req->handle, THREAD_SET_CONTEXT ); break; case APC_VIRTUAL_ALLOC: + case APC_VIRTUAL_ALLOC_EX: case APC_VIRTUAL_FREE: case APC_VIRTUAL_PROTECT: case APC_VIRTUAL_FLUSH: diff --git a/server/trace.c b/server/trace.c index cb58ab5162a..560a1b617ea 100644 --- a/server/trace.c +++ b/server/trace.c @@ -189,6 +189,12 @@ static void dump_apc_call( const char *prefix, const apc_call_t *call ) dump_uint64( ",zero_bits=", &call->virtual_alloc.zero_bits ); fprintf( stderr, ",op_type=%x,prot=%x", call->virtual_alloc.op_type, call->virtual_alloc.prot ); break; + case APC_VIRTUAL_ALLOC_EX: + dump_uint64( "APC_VIRTUAL_ALLOC,addr==", &call->virtual_alloc_ex.addr ); + dump_uint64( ",size=", &call->virtual_alloc_ex.size ); + dump_uint64( ",limit=", &call->virtual_alloc_ex.limit ); + fprintf( stderr, ",op_type=%x,prot=%x", call->virtual_alloc_ex.op_type, call->virtual_alloc_ex.prot ); + break; case APC_VIRTUAL_FREE: dump_uint64( "APC_VIRTUAL_FREE,addr=", &call->virtual_free.addr ); dump_uint64( ",size=", &call->virtual_free.size ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 164 +++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 83ea35590f1..24a621b5136 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -423,6 +423,169 @@ static void test_NtAllocateVirtualMemoryEx(void) } } +static void test_NtAllocateVirtualMemoryEx_address_requirements(void) +{ + MEM_EXTENDED_PARAMETER ext[2]; + MEM_ADDRESS_REQUIREMENTS a; + NTSTATUS status; + SYSTEM_INFO si; + SIZE_T size; + void *addr; + + if (!pNtAllocateVirtualMemoryEx) + { + win_skip("NtAllocateVirtualMemoryEx() is missing\n"); + return; + } + + GetSystemInfo(&si); + + memset(&ext, 0, sizeof(ext)); + ext[0].Type = 0; + size = 0x1000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE | MEM_COMMIT, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + memset(&ext, 0, sizeof(ext)); + ext[0].Type = MemExtendedParameterMax; + size = 0x1000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE | MEM_COMMIT, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + memset(&a, 0, sizeof(a)); + ext[0].Type = MemExtendedParameterAddressRequirements; + ext[0].Pointer = &a; + size = 0x1000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE | MEM_COMMIT, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(!status, "Unexpected status %08lx.\n", status); + size = 0; + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(!status, "Unexpected status %08lx.\n", status); + + ext[1] = ext[0]; + size = 0x1000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE | MEM_COMMIT, + PAGE_EXECUTE_READWRITE, ext, 2); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + a.LowestStartingAddress = NULL; + a.Alignment = 0; + + a.HighestEndingAddress = (void *)(0x20001000 + 1); + size = 0x10000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + a.HighestEndingAddress = (void *)(0x20001000 - 2); + size = 0x10000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + a.HighestEndingAddress = (void *)(0x20000800 - 1); + size = 0x10000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + a.HighestEndingAddress = (char *)si.lpMaximumApplicationAddress + 0x1000; + size = 0x10000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + a.HighestEndingAddress = (char *)si.lpMaximumApplicationAddress; + size = 0x10000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(!status, "Unexpected status %08lx.\n", status); + size = 0; + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(!status, "Unexpected status %08lx.\n", status); + + a.HighestEndingAddress = (void *)(0x20001000 - 1); + size = 0x40000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(!status, "Unexpected status %08lx.\n", status); + ok(!((ULONG_PTR)addr & 0xffff), "Unexpected addr %p.\n", addr); + ok((ULONG_PTR)addr + size <= 0x20001000, "Unexpected addr %p.\n", addr); + + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_COMMIT, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + size = 0; + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(!status, "Unexpected status %08lx.\n", status); + + + size = 0x40000; + a.HighestEndingAddress = (void *)(0x20001000 - 1); + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr, 24, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE); + ok(status == STATUS_INVALID_PARAMETER_3 || status == STATUS_INVALID_PARAMETER, + "Unexpected status %08lx.\n", status); + + status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr, 0xffffffff, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE); + if (is_win64 || is_wow64) + ok(!status || status == STATUS_CONFLICTING_ADDRESSES, "Unexpected status %08lx.\n", status); + else + ok(status == STATUS_INVALID_PARAMETER_3 || status == STATUS_INVALID_PARAMETER, + "Unexpected status %08lx.\n", status); + + if (!status) + { + size = 0; + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(!status, "Unexpected status %08lx.\n", status); + } + + a.HighestEndingAddress = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(!status || status == STATUS_CONFLICTING_ADDRESSES, "Unexpected status %08lx.\n", status); + if (!status) + { + size = 0; + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(!status, "Unexpected status %08lx.\n", status); + } + + + a.HighestEndingAddress = (void *)(0x20001000 - 1); + a.Alignment = 0x10000; + size = 0x1000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(!status, "Unexpected status %08lx.\n", status); + ok(!((ULONG_PTR)addr & 0xffff), "Unexpected addr %p.\n", addr); + ok((ULONG_PTR)addr + size < 0x20001000, "Unexpected addr %p.\n", addr); + size = 0; + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(!status, "Unexpected status %08lx.\n", status); +} + struct test_stack_size_thread_args { DWORD expect_committed; @@ -1612,6 +1775,7 @@ START_TEST(virtual) test_NtAllocateVirtualMemory(); test_NtAllocateVirtualMemoryEx(); + test_NtAllocateVirtualMemoryEx_address_requirements(); test_NtFreeVirtualMemory(); test_RtlCreateUserStack(); test_NtMapViewOfSection(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/wow64/struct32.h | 7 +++++++ dlls/wow64/virtual.c | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/dlls/wow64/struct32.h b/dlls/wow64/struct32.h index 91acc26436f..071bdf092e0 100644 --- a/dlls/wow64/struct32.h +++ b/dlls/wow64/struct32.h @@ -674,4 +674,11 @@ struct __server_request_info32 struct __server_iovec32 data[__SERVER_MAX_DATA]; }; +typedef struct +{ + ULONG LowestStartingAddress; + ULONG HighestEndingAddress; + ULONG Alignment; +} MEM_ADDRESS_REQUIREMENTS32; + #endif /* __WOW64_STRUCT32_H */ diff --git a/dlls/wow64/virtual.c b/dlls/wow64/virtual.c index 8b7d022301f..2a4116d1515 100644 --- a/dlls/wow64/virtual.c +++ b/dlls/wow64/virtual.c @@ -91,10 +91,42 @@ NTSTATUS WINAPI wow64_NtAllocateVirtualMemoryEx( UINT *args ) void *addr; SIZE_T size; NTSTATUS status; + SIZE_T alloc_size = count * sizeof(*params); + MEM_EXTENDED_PARAMETER *params64; + MEM_ADDRESS_REQUIREMENTS *buf; + unsigned int i; + + if (count && !params) return STATUS_INVALID_PARAMETER; + + for (i = 0; i < count; ++i) + { + if (params[i].Type == MemExtendedParameterAddressRequirements) + alloc_size += sizeof(MEM_ADDRESS_REQUIREMENTS); + else if (params[i].Type && params[i].Type < MemExtendedParameterMax) + FIXME( "Unsupported parameter type %d.\n", params[i].Type); + } + + params64 = Wow64AllocateTemp( alloc_size ); + memcpy( params64, params, count * sizeof(*params64) ); + buf = (MEM_ADDRESS_REQUIREMENTS *)((char *)params64 + count * sizeof(*params64)); + for (i = 0; i < count; ++i) + { + if (params64[i].Type == MemExtendedParameterAddressRequirements) + { + MEM_ADDRESS_REQUIREMENTS32 *p = (MEM_ADDRESS_REQUIREMENTS32 *)params[i].Pointer; + + if (p->HighestEndingAddress > highest_user_address) return STATUS_INVALID_PARAMETER; + + buf->LowestStartingAddress = (void *)(ULONG_PTR)p->LowestStartingAddress; + buf->HighestEndingAddress = (void *)(ULONG_PTR)p->HighestEndingAddress; + buf->Alignment = p->Alignment; + params64[i].Pointer = buf; + ++buf; + } + } - if (count) FIXME( "%ld extended parameters %p\n", count, params ); status = NtAllocateVirtualMemoryEx( process, addr_32to64( &addr, addr32 ), size_32to64( &size, size32 ), - type, protect, params, count ); + type, protect, params64, count ); if (!status) { put_addr( addr32, addr ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
From: Paul Gofman <pgofman(a)codeweavers.com> To prevent 64 bit NtAllocateVirtualMemoryEx() allocating from above 32 bit user space limit. --- dlls/wow64/virtual.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/dlls/wow64/virtual.c b/dlls/wow64/virtual.c index 2a4116d1515..373a07a2236 100644 --- a/dlls/wow64/virtual.c +++ b/dlls/wow64/virtual.c @@ -93,6 +93,7 @@ NTSTATUS WINAPI wow64_NtAllocateVirtualMemoryEx( UINT *args ) NTSTATUS status; SIZE_T alloc_size = count * sizeof(*params); MEM_EXTENDED_PARAMETER *params64; + BOOL need_highest_address = !*addr32; MEM_ADDRESS_REQUIREMENTS *buf; unsigned int i; @@ -101,24 +102,49 @@ NTSTATUS WINAPI wow64_NtAllocateVirtualMemoryEx( UINT *args ) for (i = 0; i < count; ++i) { if (params[i].Type == MemExtendedParameterAddressRequirements) + { alloc_size += sizeof(MEM_ADDRESS_REQUIREMENTS); + need_highest_address = FALSE; + } else if (params[i].Type && params[i].Type < MemExtendedParameterMax) + { FIXME( "Unsupported parameter type %d.\n", params[i].Type); + } } + if (need_highest_address) + alloc_size += sizeof(*params) + sizeof(MEM_ADDRESS_REQUIREMENTS); params64 = Wow64AllocateTemp( alloc_size ); memcpy( params64, params, count * sizeof(*params64) ); - buf = (MEM_ADDRESS_REQUIREMENTS *)((char *)params64 + count * sizeof(*params64)); + if (need_highest_address) + { + buf = (MEM_ADDRESS_REQUIREMENTS *)((char *)params64 + (count + 1) * sizeof(*params64)); + params64[count].Type = MemExtendedParameterAddressRequirements; + params64[count].Pointer = buf; + memset(buf, 0, sizeof(*buf)); + buf->HighestEndingAddress = (void *)(ULONG_PTR)highest_user_address; + ++buf; + } + else + { + buf = (MEM_ADDRESS_REQUIREMENTS *)((char *)params64 + count * sizeof(*params64)); + } for (i = 0; i < count; ++i) { if (params64[i].Type == MemExtendedParameterAddressRequirements) { MEM_ADDRESS_REQUIREMENTS32 *p = (MEM_ADDRESS_REQUIREMENTS32 *)params[i].Pointer; - if (p->HighestEndingAddress > highest_user_address) return STATUS_INVALID_PARAMETER; - buf->LowestStartingAddress = (void *)(ULONG_PTR)p->LowestStartingAddress; - buf->HighestEndingAddress = (void *)(ULONG_PTR)p->HighestEndingAddress; + if (p->HighestEndingAddress) + { + if (p->HighestEndingAddress > highest_user_address) return STATUS_INVALID_PARAMETER; + buf->HighestEndingAddress = (void *)(ULONG_PTR)p->HighestEndingAddress; + } + else + { + buf->HighestEndingAddress = *addr32 ? NULL : (void *)(ULONG_PTR)highest_user_address; + } buf->Alignment = p->Alignment; params64[i].Pointer = buf; ++buf; @@ -126,7 +152,7 @@ NTSTATUS WINAPI wow64_NtAllocateVirtualMemoryEx( UINT *args ) } status = NtAllocateVirtualMemoryEx( process, addr_32to64( &addr, addr32 ), size_32to64( &size, size32 ), - type, protect, params64, count ); + type, protect, params64, count + need_highest_address ); if (!status) { put_addr( addr32, addr ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/unix/virtual.c | 66 ++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index 6304417415e..fbb5458fc23 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -1274,15 +1274,15 @@ static void* try_map_free_area( void *base, void *end, ptrdiff_t step, * Find a free area between views inside the specified range and map it. * virtual_mutex must be held by caller. */ -static void *map_free_area( void *base, void *end, size_t size, int top_down, int unix_prot ) +static void *map_free_area( void *base, void *end, size_t size, int top_down, int unix_prot, size_t align_mask ) { struct wine_rb_entry *first = find_view_inside_range( &base, &end, top_down ); - ptrdiff_t step = top_down ? -(granularity_mask + 1) : (granularity_mask + 1); + ptrdiff_t step = top_down ? -(align_mask + 1) : (align_mask + 1); void *start; if (top_down) { - start = ROUND_ADDR( (char *)end - size, granularity_mask ); + start = ROUND_ADDR( (char *)end - size, align_mask ); if (start >= end || start < base) return NULL; while (first) @@ -1290,7 +1290,7 @@ static void *map_free_area( void *base, void *end, size_t size, int top_down, in struct file_view *view = WINE_RB_ENTRY_VALUE( first, struct file_view, entry ); if ((start = try_map_free_area( (char *)view->base + view->size, (char *)start + size, step, start, size, unix_prot ))) break; - start = ROUND_ADDR( (char *)view->base - size, granularity_mask ); + start = ROUND_ADDR( (char *)view->base - size, align_mask ); /* stop if remaining space is not large enough */ if (!start || start >= end || start < base) return NULL; first = rb_prev( first ); @@ -1298,7 +1298,7 @@ static void *map_free_area( void *base, void *end, size_t size, int top_down, in } else { - start = ROUND_ADDR( (char *)base + granularity_mask, granularity_mask ); + start = ROUND_ADDR( (char *)base + align_mask, align_mask ); if (!start || start >= end || (char *)end - (char *)start < size) return NULL; while (first) @@ -1306,7 +1306,7 @@ static void *map_free_area( void *base, void *end, size_t size, int top_down, in struct file_view *view = WINE_RB_ENTRY_VALUE( first, struct file_view, entry ); if ((start = try_map_free_area( start, view->base, step, start, size, unix_prot ))) break; - start = ROUND_ADDR( (char *)view->base + view->size + granularity_mask, granularity_mask ); + start = ROUND_ADDR( (char *)view->base + view->size + align_mask, align_mask ); /* stop if remaining space is not large enough */ if (!start || start >= end || (char *)end - (char *)start < size) return NULL; first = rb_next( first ); @@ -1327,13 +1327,13 @@ static void *map_free_area( void *base, void *end, size_t size, int top_down, in * virtual_mutex must be held by caller. * The range must be inside the preloader reserved range. */ -static void *find_reserved_free_area( void *base, void *end, size_t size, int top_down ) +static void *find_reserved_free_area( void *base, void *end, size_t size, int top_down, size_t align_mask ) { struct range_entry *range; void *start; - base = ROUND_ADDR( (char *)base + granularity_mask, granularity_mask ); - end = (char *)ROUND_ADDR( (char *)end - size, granularity_mask ) + size; + base = ROUND_ADDR( (char *)base + align_mask, align_mask ); + end = (char *)ROUND_ADDR( (char *)end - size, align_mask ) + size; if (top_down) { @@ -1341,13 +1341,13 @@ static void *find_reserved_free_area( void *base, void *end, size_t size, int to range = free_ranges_lower_bound( start ); assert(range != free_ranges_end && range->end >= start); - if ((char *)range->end - (char *)start < size) start = ROUND_ADDR( (char *)range->end - size, granularity_mask ); + if ((char *)range->end - (char *)start < size) start = ROUND_ADDR( (char *)range->end - size, align_mask ); do { if (start >= end || start < base || (char *)end - (char *)start < size) return NULL; if (start < range->end && start >= range->base && (char *)range->end - (char *)start >= size) break; if (--range < free_ranges) return NULL; - start = ROUND_ADDR( (char *)range->end - size, granularity_mask ); + start = ROUND_ADDR( (char *)range->end - size, align_mask ); } while (1); } @@ -1357,13 +1357,13 @@ static void *find_reserved_free_area( void *base, void *end, size_t size, int to range = free_ranges_lower_bound( start ); assert(range != free_ranges_end && range->end >= start); - if (start < range->base) start = ROUND_ADDR( (char *)range->base + granularity_mask, granularity_mask ); + if (start < range->base) start = ROUND_ADDR( (char *)range->base + align_mask, align_mask ); do { if (start >= end || start < base || (char *)end - (char *)start < size) return NULL; if (start < range->end && start >= range->base && (char *)range->end - (char *)start >= size) break; if (++range == free_ranges_end) return NULL; - start = ROUND_ADDR( (char *)range->base + granularity_mask, granularity_mask ); + start = ROUND_ADDR( (char *)range->base + align_mask, align_mask ); } while (1); } @@ -1773,13 +1773,13 @@ static void reset_write_watches( void *base, SIZE_T size ) /*********************************************************************** * unmap_extra_space * - * Release the extra memory while keeping the range starting on the granularity boundary. + * Release the extra memory while keeping the range starting on the alignment boundary. */ -static inline void *unmap_extra_space( void *ptr, size_t total_size, size_t wanted_size ) +static inline void *unmap_extra_space( void *ptr, size_t total_size, size_t wanted_size, size_t align_mask ) { - if ((ULONG_PTR)ptr & granularity_mask) + if ((ULONG_PTR)ptr & align_mask) { - size_t extra = granularity_mask + 1 - ((ULONG_PTR)ptr & granularity_mask); + size_t extra = align_mask + 1 - ((ULONG_PTR)ptr & align_mask); munmap( ptr, extra ); ptr = (char *)ptr + extra; total_size -= extra; @@ -1796,6 +1796,7 @@ struct alloc_area int top_down; void *limit; void *result; + size_t align_mask; }; /*********************************************************************** @@ -1825,13 +1826,13 @@ static int alloc_reserved_area_callback( void *start, SIZE_T size, void *arg ) { /* range is split in two by the preloader reservation, try first part */ if ((alloc->result = find_reserved_free_area( start, preload_reserve_start, alloc->size, - alloc->top_down ))) + alloc->top_down, alloc->align_mask ))) return 1; /* then fall through to try second part */ start = preload_reserve_end; } } - if ((alloc->result = find_reserved_free_area( start, end, alloc->size, alloc->top_down ))) + if ((alloc->result = find_reserved_free_area( start, end, alloc->size, alloc->top_down, alloc->align_mask ))) return 1; return 0; @@ -1895,11 +1896,13 @@ static NTSTATUS map_fixed_area( void *base, size_t size, unsigned int vprot ) * virtual_mutex must be held by caller. */ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, - int top_down, unsigned int vprot, ULONG_PTR limit ) + int top_down, unsigned int vprot, ULONG_PTR limit, size_t align_mask ) { void *ptr; NTSTATUS status; + TRACE("align_mask %#zx.\n", align_mask); + if (base) { if (is_beyond_limit( base, size, address_space_limit )) @@ -1910,12 +1913,16 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, } else { - size_t view_size = size + granularity_mask + 1; struct alloc_area alloc; + size_t view_size; + + if (!align_mask) align_mask = granularity_mask; + view_size = size + align_mask + 1; alloc.size = size; alloc.top_down = top_down; alloc.limit = limit ? min( (void *)(limit + 1), user_space_limit ) : user_space_limit; + alloc.align_mask = align_mask; if (mmap_enum_reserved_areas( alloc_reserved_area_callback, &alloc, top_down )) { @@ -1929,7 +1936,7 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, if (limit) { if (!(ptr = map_free_area( address_space_start, alloc.limit, size, - top_down, get_unix_prot(vprot) ))) + top_down, get_unix_prot(vprot), align_mask ))) return STATUS_NO_MEMORY; TRACE( "got mem with map_free_area %p-%p\n", ptr, (char *)ptr + size ); goto done; @@ -1947,7 +1954,7 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, if (is_beyond_limit( ptr, view_size, user_space_limit )) add_reserved_area( ptr, view_size ); else break; } - ptr = unmap_extra_space( ptr, view_size, size ); + ptr = unmap_extra_space( ptr, view_size, size, align_mask ); } done: status = create_view( view_ret, ptr, size, vprot ); @@ -2107,7 +2114,7 @@ static NTSTATUS allocate_dos_memory( struct file_view **view, unsigned int vprot if (mmap_is_in_reserved_area( low_64k, dosmem_size - 0x10000 ) != 1) { addr = anon_mmap_tryfixed( low_64k, dosmem_size - 0x10000, unix_prot, 0 ); - if (addr == MAP_FAILED) return map_view( view, NULL, dosmem_size, FALSE, vprot, 0 ); + if (addr == MAP_FAILED) return map_view( view, NULL, dosmem_size, FALSE, vprot, 0, 0 ); } /* now try to allocate the low 64K too */ @@ -2451,9 +2458,9 @@ static NTSTATUS virtual_map_image( HANDLE mapping, ACCESS_MASK access, void **ad if ((ULONG_PTR)base != image_info->base) base = NULL; if ((char *)base >= (char *)address_space_start) /* make sure the DOS area remains free */ - status = map_view( &view, base, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ) ); + status = map_view( &view, base, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ), 0 ); - if (status) status = map_view( &view, NULL, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits )); + if (status) status = map_view( &view, NULL, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ), 0 ); if (status) goto done; status = map_image_into_view( view, filename, unix_fd, base, image_info->header_size, @@ -2576,7 +2583,7 @@ static NTSTATUS virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG_PTR z server_enter_uninterrupted_section( &virtual_mutex, &sigset ); - res = map_view( &view, base, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ) ); + res = map_view( &view, base, size, alloc_type & MEM_TOP_DOWN, vprot, get_zero_bits_mask( zero_bits ), 0 ); if (res) goto done; TRACE( "handle=%p size=%lx offset=%x%08x\n", handle, size, offset.u.HighPart, offset.u.LowPart ); @@ -3114,7 +3121,7 @@ NTSTATUS virtual_alloc_thread_stack( INITIAL_TEB *stack, ULONG_PTR zero_bits, SI server_enter_uninterrupted_section( &virtual_mutex, &sigset ); if ((status = map_view( &view, NULL, size + extra_size, FALSE, - VPROT_READ | VPROT_WRITE | VPROT_COMMITTED, get_zero_bits_mask( zero_bits ) )) + VPROT_READ | VPROT_WRITE | VPROT_COMMITTED, get_zero_bits_mask( zero_bits ), 0 )) != STATUS_SUCCESS) goto done; @@ -3784,7 +3791,8 @@ static NTSTATUS allocate_virtual_memory( void **ret, SIZE_T *size_ptr, ULONG typ if (vprot & VPROT_WRITECOPY) status = STATUS_INVALID_PAGE_PROTECTION; else if (is_dos_memory) status = allocate_dos_memory( &view, vprot ); - else status = map_view( &view, base, size, type & MEM_TOP_DOWN, vprot, limit ); + else status = map_view( &view, base, size, type & MEM_TOP_DOWN, vprot, limit, + granularity_mask ); if (status == STATUS_SUCCESS) base = view->base; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/tests/virtual.c | 42 ++++++++++++++++++++++++++++++++++++++ dlls/ntdll/unix/server.c | 7 +++++-- dlls/ntdll/unix/virtual.c | 24 ++++++++++++++++------ server/protocol.def | 1 + server/trace.c | 1 + 5 files changed, 67 insertions(+), 8 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 24a621b5136..1a476ac8f20 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -584,6 +584,48 @@ static void test_NtAllocateVirtualMemoryEx_address_requirements(void) size = 0; status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); ok(!status, "Unexpected status %08lx.\n", status); + + a.HighestEndingAddress = (void *)(0x20001000 - 1); + a.Alignment = 0x20000000; + size = 0x2000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_NO_MEMORY, "Unexpected status %08lx.\n", status); + + a.HighestEndingAddress = NULL; + a.Alignment = 0x8000; + size = 0x1000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + a.Alignment = 0x30000; + size = 0x1000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + a.Alignment = 0x40000; + size = 0x1000; + addr = NULL; + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(!status, "Unexpected status %08lx.\n", status); + ok(!((ULONG_PTR)addr & 0x3ffff), "Unexpected addr %p.\n", addr); + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_COMMIT, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); + + size = 0; + status = NtFreeVirtualMemory(NtCurrentProcess(), &addr, &size, MEM_RELEASE); + ok(!status, "Unexpected status %08lx.\n", status); + + status = pNtAllocateVirtualMemoryEx(NtCurrentProcess(), &addr, &size, MEM_RESERVE, + PAGE_EXECUTE_READWRITE, ext, 1); + ok(status == STATUS_INVALID_PARAMETER, "Unexpected status %08lx.\n", status); } struct test_stack_size_thread_args diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c index b7d8733f2bc..07d9c8d3acd 100644 --- a/dlls/ntdll/unix/server.c +++ b/dlls/ntdll/unix/server.c @@ -358,7 +358,7 @@ static NTSTATUS invoke_user_apc( CONTEXT *context, const user_apc_t *apc, NTSTAT */ static void invoke_system_apc( const apc_call_t *call, apc_result_t *result, BOOL self ) { - SIZE_T size, bits, limit; + SIZE_T size, bits, limit, align; void *addr; memset( result, 0, sizeof(*result) ); @@ -416,9 +416,12 @@ static void invoke_system_apc( const apc_call_t *call, apc_result_t *result, BOO addr = wine_server_get_ptr( call->virtual_alloc_ex.addr ); size = call->virtual_alloc_ex.size; limit = min( (ULONG_PTR)sbi.HighestUserAddress, call->virtual_alloc_ex.limit ); - if ((ULONG_PTR)addr == call->virtual_alloc_ex.addr && size == call->virtual_alloc_ex.size) + align = call->virtual_alloc_ex.align; + if ((ULONG_PTR)addr == call->virtual_alloc_ex.addr && size == call->virtual_alloc_ex.size + && align == call->virtual_alloc_ex.align) { r.HighestEndingAddress = (void *)limit; + r.Alignment = align; result->virtual_alloc_ex.status = NtAllocateVirtualMemoryEx( NtCurrentProcess(), &addr, &size, call->virtual_alloc_ex.op_type, call->virtual_alloc_ex.prot, &ext, 1 ); diff --git a/dlls/ntdll/unix/virtual.c b/dlls/ntdll/unix/virtual.c index fbb5458fc23..f42cde2c894 100644 --- a/dlls/ntdll/unix/virtual.c +++ b/dlls/ntdll/unix/virtual.c @@ -3730,7 +3730,7 @@ void virtual_set_large_address_space(void) * NtAllocateVirtualMempory[Ex] implementation. */ static NTSTATUS allocate_virtual_memory( void **ret, SIZE_T *size_ptr, ULONG type, ULONG protect, - ULONG_PTR limit ) + ULONG_PTR limit, ULONG_PTR align ) { void *base; unsigned int vprot; @@ -3792,7 +3792,7 @@ static NTSTATUS allocate_virtual_memory( void **ret, SIZE_T *size_ptr, ULONG typ if (vprot & VPROT_WRITECOPY) status = STATUS_INVALID_PAGE_PROTECTION; else if (is_dos_memory) status = allocate_dos_memory( &view, vprot ); else status = map_view( &view, base, size, type & MEM_TOP_DOWN, vprot, limit, - granularity_mask ); + align ? align - 1 : granularity_mask ); if (status == STATUS_SUCCESS) base = view->base; } @@ -3880,7 +3880,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG_PTR z else limit = 0; - return allocate_virtual_memory( ret, size_ptr, type, protect, limit ); + return allocate_virtual_memory( ret, size_ptr, type, protect, limit, 0 ); } @@ -3893,6 +3893,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemoryEx( HANDLE process, PVOID *ret, SIZE_T *s ULONG count ) { ULONG_PTR limit = 0; + ULONG_PTR align = 0; TRACE("%p %p %08lx %x %08x %p %u\n", process, *ret, *size_ptr, type, protect, parameters, count ); @@ -3924,17 +3925,27 @@ NTSTATUS WINAPI NtAllocateVirtualMemoryEx( HANDLE process, PVOID *ret, SIZE_T *s mem_requirements_found = 1; r = (MEM_ADDRESS_REQUIREMENTS *)parameters[i].Pointer; - if (r->LowestStartingAddress || r->Alignment) + if (r->LowestStartingAddress) FIXME( "Not supported requirements LowestStartingAddress %p, Alignment %p.\n", r->LowestStartingAddress, (void *)r->Alignment ); + if (r->Alignment) + { + if (*ret || (r->Alignment & (r->Alignment - 1)) || r->Alignment - 1 < granularity_mask) + { + WARN( "Invalid alignment %lu.\n", r->Alignment ); + return STATUS_INVALID_PARAMETER; + } + align = r->Alignment; + } + limit = (ULONG_PTR)r->HighestEndingAddress; if (limit && (*ret || limit > (ULONG_PTR)user_space_limit || ((limit + 1) & (page_mask - 1)))) { WARN( "Invalid limit %p.\n", r->HighestEndingAddress); return STATUS_INVALID_PARAMETER; } - TRACE( "limit %p.\n", (void *)limit ); + TRACE( "limit %p, align %p.\n", (void *)limit, (void *)align ); } } @@ -3952,6 +3963,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemoryEx( HANDLE process, PVOID *ret, SIZE_T *s call.virtual_alloc_ex.addr = wine_server_client_ptr( *ret ); call.virtual_alloc_ex.size = *size_ptr; call.virtual_alloc_ex.limit = limit; + call.virtual_alloc_ex.align = align; call.virtual_alloc_ex.op_type = type; call.virtual_alloc_ex.prot = protect; status = server_queue_process_apc( process, &call, &result ); @@ -3965,7 +3977,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemoryEx( HANDLE process, PVOID *ret, SIZE_T *s return result.virtual_alloc_ex.status; } - return allocate_virtual_memory( ret, size_ptr, type, protect, limit ); + return allocate_virtual_memory( ret, size_ptr, type, protect, limit, align ); } diff --git a/server/protocol.def b/server/protocol.def index 06977c29054..8c2fbeb4afe 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -542,6 +542,7 @@ typedef union client_ptr_t addr; /* requested address */ mem_size_t size; /* allocation size */ mem_size_t limit; /* allocation address limit */ + mem_size_t align; /* allocation alignment */ unsigned int prot; /* memory protection flags */ } virtual_alloc_ex; struct diff --git a/server/trace.c b/server/trace.c index 560a1b617ea..a0076d5449b 100644 --- a/server/trace.c +++ b/server/trace.c @@ -193,6 +193,7 @@ static void dump_apc_call( const char *prefix, const apc_call_t *call ) dump_uint64( "APC_VIRTUAL_ALLOC,addr==", &call->virtual_alloc_ex.addr ); dump_uint64( ",size=", &call->virtual_alloc_ex.size ); dump_uint64( ",limit=", &call->virtual_alloc_ex.limit ); + dump_uint64( ",align=", &call->virtual_alloc_ex.align ); fprintf( stderr, ",op_type=%x,prot=%x", call->virtual_alloc_ex.op_type, call->virtual_alloc_ex.prot ); break; case APC_VIRTUAL_FREE: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1025
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125363 Your paranoid android. === debian11 (build log) === Task: Could not create the win32 wineprefix: Failed to disable the crash dialogs: Task: WineTest did not produce the win32 report
v4: - Introduce allocate_virtual_memory() holding implementation instead of implementing NtAllocateVirtualMemory() on top of NtAllocateVirtualMemoryEx(); - Split the two biggest patches. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1025#note_11919
Alexandre Julliard (@julliard) commented about dlls/ntdll/unix/virtual.c:
+ FIXME( "Not supported requirements LowestStartingAddress %p, Alignment %p.\n", + r->LowestStartingAddress, (void *)r->Alignment ); + + if (r->Alignment) + { + if (*ret || (r->Alignment & (r->Alignment - 1)) || r->Alignment - 1 < granularity_mask) + { + WARN( "Invalid alignment %lu.\n", r->Alignment ); + return STATUS_INVALID_PARAMETER; + } + align = r->Alignment; + } + + limit = (ULONG_PTR)r->HighestEndingAddress; + if (limit && (*ret || limit > (ULONG_PTR)user_space_limit || ((limit + 1) & (page_mask - 1)))) + { This would most likely need to check against the user space limit of the target process.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1025#note_13826
Alexandre Julliard (@julliard) commented about dlls/wow64/virtual.c:
+ } + else + { + buf = (MEM_ADDRESS_REQUIREMENTS *)((char *)params64 + count * sizeof(*params64)); + } + for (i = 0; i < count; ++i) + { + if (params64[i].Type == MemExtendedParameterAddressRequirements) + { + MEM_ADDRESS_REQUIREMENTS32 *p = (MEM_ADDRESS_REQUIREMENTS32 *)params[i].Pointer; + + buf->LowestStartingAddress = (void *)(ULONG_PTR)p->LowestStartingAddress; + if (p->HighestEndingAddress) + { + if (p->HighestEndingAddress > highest_user_address) return STATUS_INVALID_PARAMETER; + buf->HighestEndingAddress = (void *)(ULONG_PTR)p->HighestEndingAddress; Same here.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1025#note_13827
Also the alignment support should be a separate MR, this one is already too large. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1025#note_13828
On Wed Nov 9 11:06:28 2022 +0000, Alexandre Julliard wrote:
This would most likely need to check against the user space limit of the target process. I was actually performing some additional ad-hoc tests while doing this. I am attaching the patch which extends the test for different target process (including 32 -> 64 and vice versa). To run that I copy 32 bit ntdll_test.exe as ntdll_test32.exe along 64 bit ntdll_test.exe (to be run), and ntdll_test64.exe to the 32 bit test directory.
There is a test in lines 538-543 which succeeds, in particular, with 32 bit target process and 64 bit source process. So it would be trivial to check the limit in the destination process in the same function, but, as weird as it sounds, it looks like this validation is done against source process on Windows for some reason. [0001-ntdll-tests-Test-NtAllocateVirtualMemoryEx-with-othe.patch](/uploads/b565c53433605b63eccccacb00120132/0001-ntdll-tests-Test-NtAllocateVirtualMemoryEx-with-othe.patch) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1025#note_13845
On Wed Nov 9 11:06:29 2022 +0000, Alexandre Julliard wrote:
Same here. Yes, here is actually the difference with Windows when wow64 is enabled (in the additional attached test that are lines 494-498 and 650-654. The check ("`if (p->HighestEndingAddress > ...`) looks correct to me in the view of the additional test above. But the limit is wrong for 32 bit source / 64 target. The difference is only when memory is allocated from 32 bit process in 64 bit process: Windows (and Wine without wow64 with my patches) allocates within the full 64 bit destination address space and returns truncated pointer to the caller. 64 bit to 32 bit allocation works like on Windows.
Unfortunately I don't yet see a clean way to do it other than maybe performing a separate wow64 APC call for other process so wow64 part is specifically handled in the target process for this case. Or, otherwise, maybe there is some way to tell NtAllocateVirtualMemoryEx that it is being called from wow64. Although I am not sure such way exists, as I understand NtAllocate functions should not care about that and this logic should be isolated in wow64 thunk. I was thinking that maybe we can leave this bit alone for now until something depends on that working exactly like on Windows? Given the caller gets truncated (unusable) pointer is seems to me such way of allocation looks rather broken on Windows and it is not much likely any app wants to allocate target process memory without caring about resulting address which it is not going to get correctly. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1025#note_13846
I was actually performing some additional ad-hoc tests while doing this. I am attaching the patch which extends the test for different target process (including 32 -> 64 and vice versa). To run that I copy 32 bit ntdll_test.exe as ntdll_test32.exe along 64 bit ntdll_test.exe (to be run), and ntdll_test64.exe to the 32 bit test directory.
A more meaningful case would be to test large address aware/unaware apps of the same bitness. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1025#note_13850
On 11/9/22 11:20, Alexandre Julliard (@julliard) wrote:
I was actually performing some additional ad-hoc tests while doing this. I am attaching the patch which extends the test for different target process (including 32 -> 64 and vice versa). To run that I copy 32 bit ntdll_test.exe as ntdll_test32.exe along 64 bit ntdll_test.exe (to be run), and ntdll_test64.exe to the 32 bit test directory. A more meaningful case would be to test large address aware/unaware apps of the same bitness.
I just ad-hoc tested that on top of my additional test by copying 32 bit ntdll_test.exe to ntdll_test64.exe in 32 bit test, manually setting large address aware flag on either executable, printing lpMaximumApplicationAddress from system info to make sure that all went correctly. It is the same: no failures introduced (besides unrelated part with broken return address of cource which is specific to 32 -> 64 where NtFreeVirtualMemory just succeeds this way of course). Should I prepare that in some easier reproducible form?
On 11/9/22 11:46, Paul Gofman wrote:
On 11/9/22 11:20, Alexandre Julliard (@julliard) wrote:
I was actually performing some additional ad-hoc tests while doing this. I am attaching the patch which extends the test for different target process (including 32 -> 64 and vice versa). To run that I copy 32 bit ntdll_test.exe as ntdll_test32.exe along 64 bit ntdll_test.exe (to be run), and ntdll_test64.exe to the 32 bit test directory. A more meaningful case would be to test large address aware/unaware apps of the same bitness.
I just ad-hoc tested that on top of my additional test by copying 32 bit ntdll_test.exe to ntdll_test64.exe in 32 bit test, manually setting large address aware flag on either executable, printing lpMaximumApplicationAddress from system info to make sure that all went correctly. It is the same: no failures introduced (besides unrelated part with broken return address of cource which is specific to 32 -> 64 where NtFreeVirtualMemory just succeeds this way of course).
Should I prepare that in some easier reproducible form?
I also just realized that I can actually easily straighten the HighestUserAddress setting part in wow64 vs other process by just not doing that if process is not the current process pseudo handle: that will be then set in invoke_system_apc() regardless of whether the call is processed in the same process or goes to another one. So I guess I'll resend with that after retesting and leave the validation part (which seems to match Windows) intact?
participants (5)
-
Alexandre Julliard (＠julliard) -
Marvin -
Paul Gofman -
Paul Gofman
-
Paul Gofman (＠gofman)