[KERNEL32][RTL] Diverse fixes/improvements for thread stack creation code. (#802)

- kernel32!BaseCreateStack() is compatible with ntdll!RtlpCreateUserStack(). - When checking whether a stack guard page can be added, its size has to be accounted for in the checking logic. - We have to satisfy the PEB::MinimumStackCommit constraint. - We cannot use PEB::GuaranteedStackBytes in BaseCreateStack() since it is nowhere initialized (default is 0). It gets initialized to a non-zero value when the user manually calls SetThreadStackGuarantee(). https://www.installsetupconfig.com/win32programming/windowsthreadsprocessapis7_6.html - RtlpCreateUserStack(): Fix memory leak in failure case. - RtlpFreeUserStack() doesn't need to return anything. See also commit 1bc59379 (r59868). CORE-11319
2025-04-04 12:39:35 +00:00 · 2016-05-26 04:44:25 +02:00 · 2016-05-26 04:44:25 +02:00 · effdb6f232
commit effdb6f232
parent 4895f21efa
4 changed files with 102 additions and 75 deletions
--- a/dll/win32/kernel32/client/thread.c
+++ b/dll/win32/kernel32/client/thread.c
@ -165,9 +165,10 @@ CreateRemoteThread(IN HANDLE hProcess,

    /* Create the Stack */
    Status = BaseCreateStack(hProcess,
-                             dwStackSize,
-                             dwCreationFlags & STACK_SIZE_PARAM_IS_A_RESERVATION ?
-                             dwStackSize : 0,
+                             (dwCreationFlags & STACK_SIZE_PARAM_IS_A_RESERVATION) ?
+                                0 : dwStackSize,
+                             (dwCreationFlags & STACK_SIZE_PARAM_IS_A_RESERVATION) ?
+                                dwStackSize : 0,
                             &InitialTeb);
    if (!NT_SUCCESS(Status))
    {
--- a/dll/win32/kernel32/client/utils.c
+++ b/dll/win32/kernel32/client/utils.c
@ -346,22 +346,25 @@ BaseFormatObjectAttributes(OUT POBJECT_ATTRIBUTES ObjectAttributes,
 }

 /*
- * Creates a stack for a thread or fiber
+ * Creates a stack for a thread or fiber.
+ * NOTE: Adapted from sdk/lib/rtl/thread.c:RtlpCreateUserStack().
 */
 NTSTATUS
 WINAPI
-BaseCreateStack(HANDLE hProcess,
-                 SIZE_T StackCommit,
-                 SIZE_T StackReserve,
-                 PINITIAL_TEB InitialTeb)
+BaseCreateStack(
+    _In_ HANDLE hProcess,
+    _In_opt_ SIZE_T StackCommit,
+    _In_opt_ SIZE_T StackReserve,
+    _Out_ PINITIAL_TEB InitialTeb)
 {
    NTSTATUS Status;
    PIMAGE_NT_HEADERS Headers;
    ULONG_PTR Stack;
    BOOLEAN UseGuard;
-    ULONG PageSize, Dummy, AllocationGranularity;
-    SIZE_T StackReserveHeader, StackCommitHeader, GuardPageSize, GuaranteedStackCommit;
-    DPRINT("BaseCreateStack (hProcess: %p, Max: %lx, Current: %lx)\n",
+    ULONG PageSize, AllocationGranularity, Dummy;
+    SIZE_T MinimumStackCommit, GuardPageSize;
+
+    DPRINT("BaseCreateStack(hProcess: 0x%p, Max: 0x%lx, Current: 0x%lx)\n",
            hProcess, StackReserve, StackCommit);

    /* Read page size */
@ -372,34 +375,38 @@ BaseCreateStack(HANDLE hProcess,
    Headers = RtlImageNtHeader(NtCurrentPeb()->ImageBaseAddress);
    if (!Headers) return STATUS_INVALID_IMAGE_FORMAT;

-    StackCommitHeader = Headers->OptionalHeader.SizeOfStackCommit;
-    StackReserveHeader = Headers->OptionalHeader.SizeOfStackReserve;
+    if (StackReserve == 0)
+        StackReserve = Headers->OptionalHeader.SizeOfStackReserve;

-    if (!StackReserve) StackReserve = StackReserveHeader;
-
-    if (!StackCommit)
+    if (StackCommit == 0)
    {
-        StackCommit = StackCommitHeader;
+        StackCommit = Headers->OptionalHeader.SizeOfStackCommit;
    }
+    /* Check if the commit is higher than the reserve */
    else if (StackCommit >= StackReserve)
    {
+        /* Grow the reserve beyond the commit, up to 1MB alignment */
        StackReserve = ROUND_UP(StackCommit, 1024 * 1024);
    }

+    /* Align everything to Page Size */
    StackCommit = ROUND_UP(StackCommit, PageSize);
    StackReserve = ROUND_UP(StackReserve, AllocationGranularity);

-    GuaranteedStackCommit = NtCurrentTeb()->GuaranteedStackBytes;
-    if ((GuaranteedStackCommit) && (StackCommit < GuaranteedStackCommit))
+    MinimumStackCommit = NtCurrentPeb()->MinimumStackCommit;
+    if ((MinimumStackCommit != 0) && (StackCommit < MinimumStackCommit))
    {
-        StackCommit = GuaranteedStackCommit;
+        StackCommit = MinimumStackCommit;
    }

+    /* Check if the commit is higher than the reserve */
    if (StackCommit >= StackReserve)
    {
+        /* Grow the reserve beyond the commit, up to 1MB alignment */
        StackReserve = ROUND_UP(StackCommit, 1024 * 1024);
    }

+    /* Align everything to Page Size */
    StackCommit = ROUND_UP(StackCommit, PageSize);
    StackReserve = ROUND_UP(StackReserve, AllocationGranularity);

@ -423,11 +430,11 @@ BaseCreateStack(HANDLE hProcess,
    InitialTeb->PreviousStackBase = NULL;
    InitialTeb->PreviousStackLimit = NULL;

-    /* Update the Stack Position */
+    /* Update the stack position */
    Stack += StackReserve - StackCommit;

-    /* Check if we will need a guard page */
-    if (StackReserve > StackCommit)
+    /* Check if we can add a guard page */
+    if (StackReserve >= StackCommit + PageSize)
    {
        Stack -= PageSize;
        StackCommit += PageSize;
@ -456,11 +463,10 @@ BaseCreateStack(HANDLE hProcess,
    /* Now set the current Stack Limit */
    InitialTeb->StackLimit = (PVOID)Stack;

-    /* Create a guard page */
+    /* Create a guard page if needed */
    if (UseGuard)
    {
-        /* Set the guard page */
-        GuardPageSize = PAGE_SIZE;
+        GuardPageSize = PageSize;
        Status = NtProtectVirtualMemory(hProcess,
                                        (PVOID*)&Stack,
                                        &GuardPageSize,
@ -481,10 +487,14 @@ BaseCreateStack(HANDLE hProcess,
    return STATUS_SUCCESS;
 }

+/*
+ * NOTE: Adapted from sdk/lib/rtl/thread.c:RtlpFreeUserStack().
+ */
 VOID
 WINAPI
-BaseFreeThreadStack(IN HANDLE hProcess,
-                    IN PINITIAL_TEB InitialTeb)
+BaseFreeThreadStack(
+    _In_ HANDLE hProcess,
+    _In_ PINITIAL_TEB InitialTeb)
 {
    SIZE_T Dummy = 0;

@ -501,10 +511,10 @@ BaseFreeThreadStack(IN HANDLE hProcess,
 VOID
 WINAPI
 BaseInitializeContext(IN PCONTEXT Context,
-                       IN PVOID Parameter,
-                       IN PVOID StartAddress,
-                       IN PVOID StackAddress,
-                       IN ULONG ContextType)
+                      IN PVOID Parameter,
+                      IN PVOID StartAddress,
+                      IN PVOID StackAddress,
+                      IN ULONG ContextType)
 {
 #ifdef _M_IX86
    ULONG ContextFlags;
--- a/dll/win32/kernel32/include/kernel32.h
+++ b/dll/win32/kernel32/include/kernel32.h
@ -183,10 +183,17 @@ BaseFormatObjectAttributes(OUT POBJECT_ATTRIBUTES ObjectAttributes,

 NTSTATUS
 WINAPI
-BaseCreateStack(HANDLE hProcess,
-                 SIZE_T StackReserve,
-                 SIZE_T StackCommit,
-                 PINITIAL_TEB InitialTeb);
+BaseCreateStack(
+    _In_ HANDLE hProcess,
+    _In_opt_ SIZE_T StackCommit,
+    _In_opt_ SIZE_T StackReserve,
+    _Out_ PINITIAL_TEB InitialTeb);
+
+VOID
+WINAPI
+BaseFreeThreadStack(
+    _In_ HANDLE hProcess,
+    _In_ PINITIAL_TEB InitialTeb);

 VOID
 WINAPI
@ -233,11 +240,6 @@ WINAPI
 BaseThreadStartup(LPTHREAD_START_ROUTINE lpStartAddress,
                  LPVOID lpParameter);

-VOID
-WINAPI
-BaseFreeThreadStack(IN HANDLE hProcess,
-                    IN PINITIAL_TEB InitialTeb);
-
 __declspec(noreturn)
 VOID
 WINAPI
--- a/sdk/lib/rtl/thread.c
+++ b/sdk/lib/rtl/thread.c
@ -20,7 +20,7 @@

 NTSTATUS
 NTAPI
-RtlpCreateUserStack(IN HANDLE hProcess,
+RtlpCreateUserStack(IN HANDLE ProcessHandle,
                    IN SIZE_T StackReserve OPTIONAL,
                    IN SIZE_T StackCommit OPTIONAL,
                    IN ULONG StackZeroBits OPTIONAL,
@ -29,10 +29,10 @@ RtlpCreateUserStack(IN HANDLE hProcess,
    NTSTATUS Status;
    SYSTEM_BASIC_INFORMATION SystemBasicInfo;
    PIMAGE_NT_HEADERS Headers;
-    ULONG_PTR Stack = 0;
-    BOOLEAN UseGuard = FALSE;
+    ULONG_PTR Stack;
+    BOOLEAN UseGuard;
    ULONG Dummy;
-    SIZE_T GuardPageSize;
+    SIZE_T MinimumStackCommit, GuardPageSize;

    /* Get some memory information */
    Status = ZwQuerySystemInformation(SystemBasicInformation,
@ -42,26 +42,34 @@ RtlpCreateUserStack(IN HANDLE hProcess,
    if (!NT_SUCCESS(Status)) return Status;

    /* Use the Image Settings if we are dealing with the current Process */
-    if (hProcess == NtCurrentProcess())
+    if (ProcessHandle == NtCurrentProcess())
    {
        /* Get the Image Headers */
        Headers = RtlImageNtHeader(NtCurrentPeb()->ImageBaseAddress);
        if (!Headers) return STATUS_INVALID_IMAGE_FORMAT;

        /* If we didn't get the parameters, find them ourselves */
-        if (!StackReserve) StackReserve = Headers->OptionalHeader.
-                                          SizeOfStackReserve;
-        if (!StackCommit) StackCommit = Headers->OptionalHeader.
-                                        SizeOfStackCommit;
+        if (StackReserve == 0)
+            StackReserve = Headers->OptionalHeader.SizeOfStackReserve;
+        if (StackCommit == 0)
+            StackCommit = Headers->OptionalHeader.SizeOfStackCommit;
+
+        MinimumStackCommit = NtCurrentPeb()->MinimumStackCommit;
+        if ((MinimumStackCommit != 0) && (StackCommit < MinimumStackCommit))
+        {
+            StackCommit = MinimumStackCommit;
+        }
    }
    else
    {
        /* Use the System Settings if needed */
-        if (!StackReserve) StackReserve = SystemBasicInfo.AllocationGranularity;
-        if (!StackCommit) StackCommit = SystemBasicInfo.PageSize;
+        if (StackReserve == 0)
+            StackReserve = SystemBasicInfo.AllocationGranularity;
+        if (StackCommit == 0)
+            StackCommit = SystemBasicInfo.PageSize;
    }

-    /* Check if the commit is higher than the reserve*/
+    /* Check if the commit is higher than the reserve */
    if (StackCommit >= StackReserve)
    {
        /* Grow the reserve beyond the commit, up to 1MB alignment */
@ -69,11 +77,12 @@ RtlpCreateUserStack(IN HANDLE hProcess,
    }

    /* Align everything to Page Size */
-    StackReserve = ROUND_UP(StackReserve, SystemBasicInfo.AllocationGranularity);
    StackCommit = ROUND_UP(StackCommit, SystemBasicInfo.PageSize);
+    StackReserve = ROUND_UP(StackReserve, SystemBasicInfo.AllocationGranularity);

    /* Reserve memory for the stack */
-    Status = ZwAllocateVirtualMemory(hProcess,
+    Stack = 0;
+    Status = ZwAllocateVirtualMemory(ProcessHandle,
                                     (PVOID*)&Stack,
                                     StackZeroBits,
                                     &StackReserve,
@ -82,41 +91,48 @@ RtlpCreateUserStack(IN HANDLE hProcess,
    if (!NT_SUCCESS(Status)) return Status;

    /* Now set up some basic Initial TEB Parameters */
-    InitialTeb->PreviousStackBase = NULL;
-    InitialTeb->PreviousStackLimit = NULL;
    InitialTeb->AllocatedStackBase = (PVOID)Stack;
    InitialTeb->StackBase = (PVOID)(Stack + StackReserve);
+    InitialTeb->PreviousStackBase = NULL;
+    InitialTeb->PreviousStackLimit = NULL;

-    /* Update the Stack Position */
+    /* Update the stack position */
    Stack += StackReserve - StackCommit;

-    /* Check if we will need a guard page */
-    if (StackReserve > StackCommit)
+    /* Check if we can add a guard page */
+    if (StackReserve >= StackCommit + SystemBasicInfo.PageSize)
    {
-        /* Remove a page to set as guard page */
        Stack -= SystemBasicInfo.PageSize;
        StackCommit += SystemBasicInfo.PageSize;
        UseGuard = TRUE;
    }
+    else
+    {
+        UseGuard = FALSE;
+    }

    /* Allocate memory for the stack */
-    Status = ZwAllocateVirtualMemory(hProcess,
+    Status = ZwAllocateVirtualMemory(ProcessHandle,
                                     (PVOID*)&Stack,
                                     0,
                                     &StackCommit,
                                     MEM_COMMIT,
                                     PAGE_READWRITE);
-    if (!NT_SUCCESS(Status)) return Status;
+    if (!NT_SUCCESS(Status))
+    {
+        GuardPageSize = 0;
+        ZwFreeVirtualMemory(ProcessHandle, (PVOID*)&Stack, &GuardPageSize, MEM_RELEASE);
+        return Status;
+    }

    /* Now set the current Stack Limit */
    InitialTeb->StackLimit = (PVOID)Stack;

-    /* Create a guard page */
+    /* Create a guard page if needed */
    if (UseGuard)
    {
-        /* Attempt maximum space possible */
        GuardPageSize = SystemBasicInfo.PageSize;
-        Status = ZwProtectVirtualMemory(hProcess,
+        Status = ZwProtectVirtualMemory(ProcessHandle,
                                        (PVOID*)&Stack,
                                        &GuardPageSize,
                                        PAGE_GUARD | PAGE_READWRITE,
@ -132,23 +148,21 @@ RtlpCreateUserStack(IN HANDLE hProcess,
    return STATUS_SUCCESS;
 }

-NTSTATUS
+VOID
 NTAPI
-RtlpFreeUserStack(IN HANDLE Process,
+RtlpFreeUserStack(IN HANDLE ProcessHandle,
                  IN PINITIAL_TEB InitialTeb)
 {
    SIZE_T Dummy = 0;
-    NTSTATUS Status;

    /* Free the Stack */
-    Status = ZwFreeVirtualMemory(Process,
-                                 &InitialTeb->AllocatedStackBase,
-                                 &Dummy,
-                                 MEM_RELEASE);
+    ZwFreeVirtualMemory(ProcessHandle,
+                        &InitialTeb->AllocatedStackBase,
+                        &Dummy,
+                        MEM_RELEASE);

    /* Clear the initial TEB */
-    RtlZeroMemory(InitialTeb, sizeof(INITIAL_TEB));
-    return Status;
+    RtlZeroMemory(InitialTeb, sizeof(*InitialTeb));
 }

 /* FUNCTIONS ***************************************************************/