[PATCH v13] ntdll: Clarify NtAllocateVirtualMemory zero_bits parameter semantics
From: Rémi Bernon <rbernon(a)codeweavers.com> This parameter was misinterpreted as an alignment parameter for the lower bits of the allocated memory region, although it is a constraint on the higher bits. Add a new internal ntdll virtual_alloc_aligned function that has a separate alignment parameter which is now used instead of the zero_bits parameter. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Huw Davies <huw(a)codeweavers.com> --- dlls/ntdll/directory.c | 4 +-- dlls/ntdll/heap.c | 5 ++-- dlls/ntdll/ntdll_misc.h | 2 ++ dlls/ntdll/signal_arm.c | 12 ++++----- dlls/ntdll/signal_arm64.c | 16 ++++++------ dlls/ntdll/signal_i386.c | 16 ++++++------ dlls/ntdll/signal_powerpc.c | 12 ++++----- dlls/ntdll/signal_x86_64.c | 16 ++++++------ dlls/ntdll/tests/virtual.c | 4 +-- dlls/ntdll/thread.c | 3 +-- dlls/ntdll/virtual.c | 49 ++++++++++++++++++++++++++----------- 11 files changed, 79 insertions(+), 60 deletions(-) diff --git a/dlls/ntdll/directory.c b/dlls/ntdll/directory.c index bbdbbe9781..6605999b82 100644 --- a/dlls/ntdll/directory.c +++ b/dlls/ntdll/directory.c @@ -1603,14 +1603,14 @@ static KERNEL_DIRENT *start_vfat_ioctl( int fd ) SIZE_T size = 2 * sizeof(*de) + page_size; void *addr = NULL; - if (NtAllocateVirtualMemory( GetCurrentProcess(), &addr, 1, &size, MEM_RESERVE, PAGE_READWRITE )) + if (virtual_alloc_aligned( &addr, 0, &size, MEM_RESERVE, PAGE_READWRITE, 1 )) return NULL; /* commit only the size needed for the dir entries */ /* this leaves an extra unaccessible page, which should make the kernel */ /* fail with -EFAULT before it stomps all over our memory */ de = addr; size = 2 * sizeof(*de); - NtAllocateVirtualMemory( GetCurrentProcess(), &addr, 1, &size, MEM_COMMIT, PAGE_READWRITE ); + virtual_alloc_aligned( &addr, 0, &size, MEM_COMMIT, PAGE_READWRITE, 1 ); } /* set d_reclen to 65535 to work around an AFS kernel bug */ diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c index cccaaee1d4..2d2caf551e 100644 --- a/dlls/ntdll/heap.c +++ b/dlls/ntdll/heap.c @@ -726,8 +726,7 @@ static void *allocate_large_block( HEAP *heap, DWORD flags, SIZE_T size ) LPVOID address = NULL; if (block_size < size) return NULL; /* overflow */ - if (NtAllocateVirtualMemory( NtCurrentProcess(), &address, 5, - &block_size, MEM_COMMIT, get_protection_type( flags ) )) + if (virtual_alloc_aligned( &address, 0, &block_size, MEM_COMMIT, get_protection_type( flags ), 5 )) { WARN("Could not allocate block for %08lx bytes\n", size ); return NULL; @@ -1521,7 +1520,7 @@ void heap_set_debug_flags( HANDLE handle ) void *ptr = NULL; SIZE_T size = MAX_FREE_PENDING * sizeof(*heap->pending_free); - if (!NtAllocateVirtualMemory( NtCurrentProcess(), &ptr, 4, &size, MEM_COMMIT, PAGE_READWRITE )) + if (!virtual_alloc_aligned( &ptr, 0, &size, MEM_COMMIT, PAGE_READWRITE, 4 )) { heap->pending_free = ptr; heap->pending_pos = 0; diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h index 3463ebd38a..f371a0ab42 100644 --- a/dlls/ntdll/ntdll_misc.h +++ b/dlls/ntdll/ntdll_misc.h @@ -168,6 +168,8 @@ extern NTSTATUS nt_to_unix_file_name_attr( const OBJECT_ATTRIBUTES *attr, ANSI_S UINT disposition ) DECLSPEC_HIDDEN; /* virtual memory */ +extern NTSTATUS virtual_alloc_aligned( PVOID *ret, ULONG zero_bits, SIZE_T *size_ptr, + ULONG type, ULONG protect, ULONG alignment ); extern NTSTATUS virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG zero_bits, SIZE_T commit_size, const LARGE_INTEGER *offset_ptr, SIZE_T *size_ptr, ULONG protect, pe_image_info_t *image_info ) DECLSPEC_HIDDEN; diff --git a/dlls/ntdll/signal_arm.c b/dlls/ntdll/signal_arm.c index e01c8ce219..cf7f2d1316 100644 --- a/dlls/ntdll/signal_arm.c +++ b/dlls/ntdll/signal_arm.c @@ -967,22 +967,22 @@ int CDECL __wine_set_signal_handler(unsigned int sig, wine_signal_handler wsh) */ NTSTATUS signal_alloc_thread( TEB **teb ) { - static size_t sigstack_zero_bits; + static size_t sigstack_alignment; SIZE_T size; NTSTATUS status; - if (!sigstack_zero_bits) + if (!sigstack_alignment) { size_t min_size = page_size; /* find the first power of two not smaller than min_size */ - while ((1u << sigstack_zero_bits) < min_size) sigstack_zero_bits++; + while ((1u << sigstack_alignment) < min_size) sigstack_alignment++; assert( sizeof(TEB) <= min_size ); } - size = 1 << sigstack_zero_bits; + size = 1 << sigstack_alignment; *teb = NULL; - if (!(status = NtAllocateVirtualMemory( NtCurrentProcess(), (void **)teb, sigstack_zero_bits, - &size, MEM_COMMIT | MEM_TOP_DOWN, PAGE_READWRITE ))) + if (!(status = virtual_alloc_aligned( (void **)teb, 0, &size, MEM_COMMIT | MEM_TOP_DOWN, + PAGE_READWRITE, sigstack_alignment ))) { (*teb)->Tib.Self = &(*teb)->Tib; (*teb)->Tib.ExceptionList = (void *)~0UL; diff --git a/dlls/ntdll/signal_arm64.c b/dlls/ntdll/signal_arm64.c index 94520c95ce..d2d43b34bc 100644 --- a/dlls/ntdll/signal_arm64.c +++ b/dlls/ntdll/signal_arm64.c @@ -871,24 +871,24 @@ int CDECL __wine_set_signal_handler(unsigned int sig, wine_signal_handler wsh) */ NTSTATUS signal_alloc_thread( TEB **teb ) { - static size_t sigstack_zero_bits; + static size_t sigstack_alignment; SIZE_T size; NTSTATUS status; - if (!sigstack_zero_bits) + if (!sigstack_alignment) { size_t min_size = teb_size + max( MINSIGSTKSZ, 8192 ); /* find the first power of two not smaller than min_size */ - sigstack_zero_bits = 12; - while ((1u << sigstack_zero_bits) < min_size) sigstack_zero_bits++; - signal_stack_size = (1 << sigstack_zero_bits) - teb_size; + sigstack_alignment = 12; + while ((1u << sigstack_alignment) < min_size) sigstack_alignment++; + signal_stack_size = (1 << sigstack_alignment) - teb_size; assert( sizeof(TEB) <= teb_size ); } - size = 1 << sigstack_zero_bits; + size = 1 << sigstack_alignment; *teb = NULL; - if (!(status = NtAllocateVirtualMemory( NtCurrentProcess(), (void **)teb, sigstack_zero_bits, - &size, MEM_COMMIT | MEM_TOP_DOWN, PAGE_READWRITE ))) + if (!(status = virtual_alloc_aligned( (void **)teb, 0, &size, MEM_COMMIT | MEM_TOP_DOWN, + PAGE_READWRITE, sigstack_alignment ))) { (*teb)->Tib.Self = &(*teb)->Tib; (*teb)->Tib.ExceptionList = (void *)~0UL; diff --git a/dlls/ntdll/signal_i386.c b/dlls/ntdll/signal_i386.c index b4e88d125f..b2925062bf 100644 --- a/dlls/ntdll/signal_i386.c +++ b/dlls/ntdll/signal_i386.c @@ -2312,25 +2312,25 @@ static void ldt_unlock(void) */ NTSTATUS signal_alloc_thread( TEB **teb ) { - static size_t sigstack_zero_bits; + static size_t sigstack_alignment; struct x86_thread_data *thread_data; SIZE_T size; void *addr = NULL; NTSTATUS status; - if (!sigstack_zero_bits) + if (!sigstack_alignment) { size_t min_size = teb_size + max( MINSIGSTKSZ, 8192 ); /* find the first power of two not smaller than min_size */ - sigstack_zero_bits = 12; - while ((1u << sigstack_zero_bits) < min_size) sigstack_zero_bits++; - signal_stack_mask = (1 << sigstack_zero_bits) - 1; - signal_stack_size = (1 << sigstack_zero_bits) - teb_size; + sigstack_alignment = 12; + while ((1u << sigstack_alignment) < min_size) sigstack_alignment++; + signal_stack_mask = (1 << sigstack_alignment) - 1; + signal_stack_size = (1 << sigstack_alignment) - teb_size; } size = signal_stack_mask + 1; - if (!(status = NtAllocateVirtualMemory( NtCurrentProcess(), &addr, sigstack_zero_bits, - &size, MEM_COMMIT | MEM_TOP_DOWN, PAGE_READWRITE ))) + if (!(status = virtual_alloc_aligned( &addr, 0, &size, MEM_COMMIT | MEM_TOP_DOWN, + PAGE_READWRITE, sigstack_alignment ))) { *teb = addr; (*teb)->Tib.Self = &(*teb)->Tib; diff --git a/dlls/ntdll/signal_powerpc.c b/dlls/ntdll/signal_powerpc.c index 86398d8f54..f23265445d 100644 --- a/dlls/ntdll/signal_powerpc.c +++ b/dlls/ntdll/signal_powerpc.c @@ -1018,22 +1018,22 @@ int CDECL __wine_set_signal_handler(unsigned int sig, wine_signal_handler wsh) */ NTSTATUS signal_alloc_thread( TEB **teb ) { - static size_t sigstack_zero_bits; + static size_t sigstack_alignment; SIZE_T size; NTSTATUS status; - if (!sigstack_zero_bits) + if (!sigstack_alignment) { size_t min_size = page_size; /* this is just for the TEB, we don't use a signal stack yet */ /* find the first power of two not smaller than min_size */ - while ((1u << sigstack_zero_bits) < min_size) sigstack_zero_bits++; + while ((1u << sigstack_alignment) < min_size) sigstack_alignment++; assert( sizeof(TEB) <= min_size ); } - size = 1 << sigstack_zero_bits; + size = 1 << sigstack_alignment; *teb = NULL; - if (!(status = NtAllocateVirtualMemory( NtCurrentProcess(), (void **)teb, sigstack_zero_bits, - &size, MEM_COMMIT | MEM_TOP_DOWN, PAGE_READWRITE ))) + if (!(status = virtual_alloc_aligned( (void **)teb, 0, &size, MEM_COMMIT | MEM_TOP_DOWN, + PAGE_READWRITE, sigstack_alignment ))) { (*teb)->Tib.Self = &(*teb)->Tib; (*teb)->Tib.ExceptionList = (void *)~0UL; diff --git a/dlls/ntdll/signal_x86_64.c b/dlls/ntdll/signal_x86_64.c index 2633b988b6..b024de7642 100644 --- a/dlls/ntdll/signal_x86_64.c +++ b/dlls/ntdll/signal_x86_64.c @@ -3263,24 +3263,24 @@ int CDECL __wine_set_signal_handler(unsigned int sig, wine_signal_handler wsh) */ NTSTATUS signal_alloc_thread( TEB **teb ) { - static size_t sigstack_zero_bits; + static size_t sigstack_alignment; SIZE_T size; NTSTATUS status; - if (!sigstack_zero_bits) + if (!sigstack_alignment) { size_t min_size = teb_size + max( MINSIGSTKSZ, 8192 ); /* find the first power of two not smaller than min_size */ - sigstack_zero_bits = 12; - while ((1u << sigstack_zero_bits) < min_size) sigstack_zero_bits++; - signal_stack_size = (1 << sigstack_zero_bits) - teb_size; + sigstack_alignment = 12; + while ((1u << sigstack_alignment) < min_size) sigstack_alignment++; + signal_stack_size = (1 << sigstack_alignment) - teb_size; assert( sizeof(TEB) <= teb_size ); } - size = 1 << sigstack_zero_bits; + size = 1 << sigstack_alignment; *teb = NULL; - if (!(status = NtAllocateVirtualMemory( NtCurrentProcess(), (void **)teb, sigstack_zero_bits, - &size, MEM_COMMIT | MEM_TOP_DOWN, PAGE_READWRITE ))) + if (!(status = virtual_alloc_aligned( (void **)teb, 0, &size, MEM_COMMIT | MEM_TOP_DOWN, + PAGE_READWRITE, sigstack_alignment ))) { (*teb)->Tib.Self = &(*teb)->Tib; (*teb)->Tib.ExceptionList = (void *)~0UL; diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index 195a54704f..ff10508a60 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -55,7 +55,6 @@ static void test_AllocateVirtualMemory(void) addr2 = (char *)addr1 + 0x1000; status = NtAllocateVirtualMemory(NtCurrentProcess(), &addr2, 12, &size, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE); - todo_wine ok(status == STATUS_CONFLICTING_ADDRESSES, "NtAllocateVirtualMemory returned %08x\n", status); if (status == STATUS_SUCCESS) { @@ -141,12 +140,11 @@ static void test_AllocateVirtualMemory(void) } else { - todo_wine ok(status == STATUS_SUCCESS || status == STATUS_NO_MEMORY, "NtAllocateVirtualMemory returned %08x\n", status); if (status == STATUS_SUCCESS) { - todo_wine + todo_wine_if((UINT_PTR)addr2 & ~zero_bits) ok(((UINT_PTR)addr2 & ~zero_bits) == 0, "NtAllocateVirtualMemory returned address %p\n", addr2); diff --git a/dlls/ntdll/thread.c b/dlls/ntdll/thread.c index 46de839400..96aa6be7f2 100644 --- a/dlls/ntdll/thread.c +++ b/dlls/ntdll/thread.c @@ -184,8 +184,7 @@ void thread_init(void) addr = NULL; size = sizeof(*peb); - NtAllocateVirtualMemory( NtCurrentProcess(), &addr, 1, &size, - MEM_COMMIT | MEM_TOP_DOWN, PAGE_READWRITE ); + virtual_alloc_aligned( &addr, 0, &size, MEM_COMMIT | MEM_TOP_DOWN, PAGE_READWRITE, 1 ); peb = addr; peb->FastPebLock = &peb_lock; diff --git a/dlls/ntdll/virtual.c b/dlls/ntdll/virtual.c index 78973a8cda..fa1448f489 100644 --- a/dlls/ntdll/virtual.c +++ b/dlls/ntdll/virtual.c @@ -1083,7 +1083,7 @@ static NTSTATUS map_fixed_area( void *base, size_t size, unsigned int vprot ) * The csVirtual section must be held by caller. */ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, size_t mask, - int top_down, unsigned int vprot ) + int top_down, unsigned int vprot, size_t zero_bits ) { void *ptr; NTSTATUS status; @@ -1101,6 +1101,9 @@ static NTSTATUS map_view( struct file_view **view_ret, void *base, size_t size, size_t view_size = size + mask + 1; struct alloc_area alloc; + if (zero_bits) + FIXME("Unimplemented zero_bits parameter value\n"); + alloc.size = size; alloc.mask = mask; alloc.top_down = top_down; @@ -1284,7 +1287,7 @@ static NTSTATUS allocate_dos_memory( struct file_view **view, unsigned int vprot if (addr != low_64k) { if (addr != (void *)-1) munmap( addr, dosmem_size - 0x10000 ); - return map_view( view, NULL, dosmem_size, 0xffff, 0, vprot ); + return map_view( view, NULL, dosmem_size, 0xffff, 0, vprot, 0 ); } } @@ -1388,11 +1391,11 @@ static NTSTATUS map_image( HANDLE hmapping, ACCESS_MASK access, int fd, SIZE_T m if (base >= (char *)address_space_start) /* make sure the DOS area remains free */ status = map_view( &view, base, total_size, mask, FALSE, SEC_IMAGE | SEC_FILE | - VPROT_COMMITTED | VPROT_READ | VPROT_EXEC | VPROT_WRITECOPY ); + VPROT_COMMITTED | VPROT_READ | VPROT_EXEC | VPROT_WRITECOPY, 0 ); if (status != STATUS_SUCCESS) status = map_view( &view, NULL, total_size, mask, FALSE, SEC_IMAGE | SEC_FILE | - VPROT_COMMITTED | VPROT_READ | VPROT_EXEC | VPROT_WRITECOPY ); + VPROT_COMMITTED | VPROT_READ | VPROT_EXEC | VPROT_WRITECOPY, 0 ); if (status != STATUS_SUCCESS) goto error; @@ -1713,7 +1716,7 @@ NTSTATUS virtual_map_section( HANDLE handle, PVOID *addr_ptr, ULONG zero_bits, S get_vprot_flags( protect, &vprot, sec_flags & SEC_IMAGE ); vprot |= sec_flags; if (!(sec_flags & SEC_RESERVE)) vprot |= VPROT_COMMITTED; - res = map_view( &view, *addr_ptr, size, mask, FALSE, vprot ); + res = map_view( &view, *addr_ptr, size, mask, FALSE, vprot, 0 ); if (res) { server_leave_uninterrupted_section( &csVirtual, &sigset ); @@ -1946,7 +1949,7 @@ NTSTATUS virtual_alloc_thread_stack( TEB *teb, SIZE_T reserve_size, SIZE_T commi server_enter_uninterrupted_section( &csVirtual, &sigset ); if ((status = map_view( &view, NULL, size + extra_size, 0xffff, 0, - VPROT_READ | VPROT_WRITE | VPROT_COMMITTED )) != STATUS_SUCCESS) + VPROT_READ | VPROT_WRITE | VPROT_COMMITTED, 0 )) != STATUS_SUCCESS) goto done; #ifdef VALGRIND_STACK_REGISTER @@ -2461,19 +2464,16 @@ void virtual_set_large_address_space(void) NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_bits, SIZE_T *size_ptr, ULONG type, ULONG protect ) { - void *base; - unsigned int vprot; SIZE_T size = *size_ptr; - SIZE_T mask = get_mask( zero_bits ); NTSTATUS status = STATUS_SUCCESS; - BOOL is_dos_memory = FALSE; - struct file_view *view; - sigset_t sigset; TRACE("%p %p %08lx %x %08x\n", process, *ret, size, type, protect ); if (!size) return STATUS_INVALID_PARAMETER; - if (!mask) return STATUS_INVALID_PARAMETER_3; + if (zero_bits > 21 && zero_bits < 32) return STATUS_INVALID_PARAMETER_3; +#ifndef _WIN64 + if (!is_wow64 && zero_bits >= 32) return STATUS_INVALID_PARAMETER_3; +#endif if (process != NtCurrentProcess()) { @@ -2499,6 +2499,27 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_ return result.virtual_alloc.status; } + return virtual_alloc_aligned( ret, zero_bits, size_ptr, type, protect, 0 ); +} + + +/*********************************************************************** + * virtual_alloc_aligned (NTDLL.@) + * + * Same as NtAllocateVirtualMemory but with an alignment parameter + */ +NTSTATUS virtual_alloc_aligned( PVOID *ret, ULONG zero_bits, SIZE_T *size_ptr, + ULONG type, ULONG protect, ULONG alignment ) +{ + void *base; + unsigned int vprot; + SIZE_T size = *size_ptr; + SIZE_T mask = get_mask( alignment ); + NTSTATUS status = STATUS_SUCCESS; + BOOL is_dos_memory = FALSE; + struct file_view *view; + sigset_t sigset; + /* Round parameters to a page boundary */ if (is_beyond_limit( 0, size, working_set_limit )) return STATUS_WORKING_SET_LIMIT_RANGE; @@ -2550,7 +2571,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_ 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, mask, type & MEM_TOP_DOWN, vprot ); + else status = map_view( &view, base, size, mask, type & MEM_TOP_DOWN, vprot, zero_bits ); if (status == STATUS_SUCCESS) base = view->base; } -- 2.17.1
participants (1)
-
Huw Davies