[PATCH v3 3/5] ntdll: Clarify NtAllocateVirtualMemory zero_bits parameter semantics

Rémi Bernon rbernon at codeweavers.com
Tue May 28 02:39:49 CDT 2019


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.

This patch adds a new exported __wine_allocate_virtual_memory function
that has a separate alignment parameter which is now used instead of
the zero_bits parameter.

Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
---
 dlls/commdlg.dll16/filedlg.c   |  8 ++++++--
 dlls/ntdll/directory.c         |  4 ++--
 dlls/ntdll/heap.c              |  7 ++++---
 dlls/ntdll/ntdll.spec          |  1 +
 dlls/ntdll/ntdll_misc.h        |  3 +++
 dlls/ntdll/server.c            |  9 +++++----
 dlls/ntdll/signal_arm.c        | 13 +++++++------
 dlls/ntdll/signal_arm64.c      | 17 +++++++++--------
 dlls/ntdll/signal_i386.c       | 17 +++++++++--------
 dlls/ntdll/signal_powerpc.c    | 13 +++++++------
 dlls/ntdll/signal_x86_64.c     | 17 +++++++++--------
 dlls/ntdll/thread.c            |  4 ++--
 dlls/ntdll/virtual.c           | 30 ++++++++++++++++++++++++------
 include/wine/server_protocol.h |  1 +
 14 files changed, 89 insertions(+), 55 deletions(-)

diff --git a/dlls/commdlg.dll16/filedlg.c b/dlls/commdlg.dll16/filedlg.c
index 5b72bfab100..d86ecd7d41c 100644
--- a/dlls/commdlg.dll16/filedlg.c
+++ b/dlls/commdlg.dll16/filedlg.c
@@ -504,13 +504,17 @@ struct hook_proc
 
 static LPOFNHOOKPROC alloc_hook( LPOFNHOOKPROC16 hook16 )
 {
+    extern NTSTATUS CDECL __wine_allocate_virtual_memory( HANDLE process, PVOID *ret, ULONG zero_bits,
+                                               SIZE_T *size_ptr, ULONG type, ULONG protect,
+                                               ULONG alignment );
+
     static struct hook_proc *hooks;
     static unsigned int count;
     SIZE_T size = 0x1000;
     unsigned int i;
 
-    if (!hooks && NtAllocateVirtualMemory( GetCurrentProcess(), (void **)&hooks, 12, &size,
-                                           MEM_COMMIT, PAGE_EXECUTE_READWRITE ))
+    if (!hooks && __wine_allocate_virtual_memory( GetCurrentProcess(), (void **)&hooks, 0, &size,
+                                                  MEM_COMMIT, PAGE_EXECUTE_READWRITE, 12 ))
         return NULL;
 
     for (i = 0; i < count; i++)
diff --git a/dlls/ntdll/directory.c b/dlls/ntdll/directory.c
index bbdbbe9781f..68c268ea9a9 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 (__wine_allocate_virtual_memory( GetCurrentProcess(), &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 );
+        __wine_allocate_virtual_memory( GetCurrentProcess(), &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 cccaaee1d45..b25640d4cc6 100644
--- a/dlls/ntdll/heap.c
+++ b/dlls/ntdll/heap.c
@@ -726,8 +726,9 @@ 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 (__wine_allocate_virtual_memory( NtCurrentProcess(), &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 +1522,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 (!__wine_allocate_virtual_memory( NtCurrentProcess(), &ptr, 0, &size, MEM_COMMIT, PAGE_READWRITE, 4 ))
         {
             heap->pending_free = ptr;
             heap->pending_pos = 0;
diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec
index 050ebc76410..29ff3a017ae 100644
--- a/dlls/ntdll/ntdll.spec
+++ b/dlls/ntdll/ntdll.spec
@@ -1514,6 +1514,7 @@
 
 # Virtual memory
 @ cdecl __wine_locked_recvmsg(long ptr long)
+@ cdecl __wine_allocate_virtual_memory(long ptr long ptr long long long)
 
 # Version
 @ cdecl wine_get_version() NTDLL_wine_get_version
diff --git a/dlls/ntdll/ntdll_misc.h b/dlls/ntdll/ntdll_misc.h
index 2d83f541bd5..a629ea8751c 100644
--- a/dlls/ntdll/ntdll_misc.h
+++ b/dlls/ntdll/ntdll_misc.h
@@ -168,6 +168,9 @@ extern NTSTATUS nt_to_unix_file_name_attr( const OBJECT_ATTRIBUTES *attr, ANSI_S
                                            UINT disposition ) DECLSPEC_HIDDEN;
 
 /* virtual memory */
+extern NTSTATUS CDECL __wine_allocate_virtual_memory( HANDLE process, 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/server.c b/dlls/ntdll/server.c
index 094b5309500..f7352e73cd6 100644
--- a/dlls/ntdll/server.c
+++ b/dlls/ntdll/server.c
@@ -429,10 +429,11 @@ BOOL invoke_apc( const apc_call_t *call, apc_result_t *result )
         size = call->virtual_alloc.size;
         if ((ULONG_PTR)addr == call->virtual_alloc.addr && size == call->virtual_alloc.size)
         {
-            result->virtual_alloc.status = NtAllocateVirtualMemory( NtCurrentProcess(), &addr,
-                                                                    call->virtual_alloc.zero_bits, &size,
-                                                                    call->virtual_alloc.op_type,
-                                                                    call->virtual_alloc.prot );
+            result->virtual_alloc.status = __wine_allocate_virtual_memory( NtCurrentProcess(), &addr,
+                                                                           call->virtual_alloc.zero_bits, &size,
+                                                                           call->virtual_alloc.op_type,
+                                                                           call->virtual_alloc.prot,
+                                                                           call->virtual_alloc.alignment );
             result->virtual_alloc.addr = wine_server_client_ptr( addr );
             result->virtual_alloc.size = size;
         }
diff --git a/dlls/ntdll/signal_arm.c b/dlls/ntdll/signal_arm.c
index e01c8ce2193..5c567fcf125 100644
--- a/dlls/ntdll/signal_arm.c
+++ b/dlls/ntdll/signal_arm.c
@@ -967,22 +967,23 @@ 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 = __wine_allocate_virtual_memory( NtCurrentProcess(), (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 94520c95ced..4f0ab02b719 100644
--- a/dlls/ntdll/signal_arm64.c
+++ b/dlls/ntdll/signal_arm64.c
@@ -871,24 +871,25 @@ 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 = __wine_allocate_virtual_memory( NtCurrentProcess(), (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 b4e88d125f1..719c3ce7281 100644
--- a/dlls/ntdll/signal_i386.c
+++ b/dlls/ntdll/signal_i386.c
@@ -2312,25 +2312,26 @@ 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 = __wine_allocate_virtual_memory( NtCurrentProcess(), &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 86398d8f54f..05355db8b71 100644
--- a/dlls/ntdll/signal_powerpc.c
+++ b/dlls/ntdll/signal_powerpc.c
@@ -1018,22 +1018,23 @@ 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 = __wine_allocate_virtual_memory( NtCurrentProcess(), (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 c2151f78c63..c1dc6df1364 100644
--- a/dlls/ntdll/signal_x86_64.c
+++ b/dlls/ntdll/signal_x86_64.c
@@ -3263,24 +3263,25 @@ 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 = __wine_allocate_virtual_memory( NtCurrentProcess(), (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/thread.c b/dlls/ntdll/thread.c
index 46de839400d..a9f542b77fc 100644
--- a/dlls/ntdll/thread.c
+++ b/dlls/ntdll/thread.c
@@ -184,8 +184,8 @@ void thread_init(void)
 
     addr = NULL;
     size = sizeof(*peb);
-    NtAllocateVirtualMemory( NtCurrentProcess(), &addr, 1, &size,
-                             MEM_COMMIT | MEM_TOP_DOWN, PAGE_READWRITE );
+    __wine_allocate_virtual_memory( NtCurrentProcess(), &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 78973a8cda4..bac1ea47f0d 100644
--- a/dlls/ntdll/virtual.c
+++ b/dlls/ntdll/virtual.c
@@ -2455,16 +2455,18 @@ void virtual_set_large_address_space(void)
 
 
 /***********************************************************************
- *             NtAllocateVirtualMemory   (NTDLL.@)
- *             ZwAllocateVirtualMemory   (NTDLL.@)
+ *             __wine_allocate_virtual_memory   (NTDLL.@)
+ *
+ * Same as NtAllocateVirtualMemory but with an alignment parameter
  */
-NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_bits,
-                                         SIZE_T *size_ptr, ULONG type, ULONG protect )
+NTSTATUS CDECL __wine_allocate_virtual_memory( HANDLE process, 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( zero_bits );
+    SIZE_T mask = get_mask( alignment );
     NTSTATUS status = STATUS_SUCCESS;
     BOOL is_dos_memory = FALSE;
     struct file_view *view;
@@ -2473,7 +2475,11 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_
     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)
+    {
+        FIXME("Unimplemented zero_bits handling\n");
+        return STATUS_INVALID_PARAMETER_3;
+    }
 
     if (process != NtCurrentProcess())
     {
@@ -2488,6 +2494,7 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_
         call.virtual_alloc.zero_bits = zero_bits;
         call.virtual_alloc.op_type   = type;
         call.virtual_alloc.prot      = protect;
+        call.virtual_alloc.alignment = alignment;
         status = server_queue_process_apc( process, &call, &result );
         if (status != STATUS_SUCCESS) return status;
 
@@ -2590,6 +2597,17 @@ NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_
 }
 
 
+/***********************************************************************
+ *             NtAllocateVirtualMemory   (NTDLL.@)
+ *             ZwAllocateVirtualMemory   (NTDLL.@)
+ */
+NTSTATUS WINAPI NtAllocateVirtualMemory( HANDLE process, PVOID *ret, ULONG zero_bits,
+                                         SIZE_T *size_ptr, ULONG type, ULONG protect )
+{
+    return __wine_allocate_virtual_memory( process, ret, zero_bits, size_ptr, type, protect, 0 );
+}
+
+
 /***********************************************************************
  *             NtFreeVirtualMemory   (NTDLL.@)
  *             ZwFreeVirtualMemory   (NTDLL.@)
diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h
index a45d092deb1..a81edbcabb6 100644
--- a/include/wine/server_protocol.h
+++ b/include/wine/server_protocol.h
@@ -485,6 +485,7 @@ typedef union
         mem_size_t       size;
         unsigned int     zero_bits;
         unsigned int     prot;
+        unsigned int     alignment;
     } virtual_alloc;
     struct
     {
-- 
2.20.1




More information about the wine-devel mailing list