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

[Huw Davies]

On Tue, May 28, 2019 at 09:39:49AM +0200, Rémi Bernon wrote:
> 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++)

We really don't want to start introducing Wine specific apis unless we
really have to.  In this case we'll just live with the default
alignment and as Alexandre mentioned here:
https://www.winehq.org/pipermail/wine-devel/2019-March/142211.html
you may as well switch to VirtualAlloc().


> 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 );
>      }
>  

It may make sense to introduce an internal ntdll helper which allocs
with smaller alignments than 64k, but it won't need to do this
cross-process.

Huw.