[reactos] 01/01: [KERNEL32][RTL] Diverse fixes/improvements for thread stack creation code. (#802)
https://git.reactos.org/?p=reactos.git;a=commitdiff;h=effdb6f232aa4448179c2b...
commit effdb6f232aa4448179c2b0a0f70aaa20856357c Author: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org AuthorDate: Thu May 26 04:44:25 2016 +0200 Commit: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org CommitDate: Thu Aug 1 19:04:13 2019 +0200
[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/windowsthreadsprocessapi...
- RtlpCreateUserStack(): Fix memory leak in failure case. - RtlpFreeUserStack() doesn't need to return anything.
See also commit 1bc59379 (r59868).
CORE-11319 --- dll/win32/kernel32/client/thread.c | 7 +-- dll/win32/kernel32/client/utils.c | 68 ++++++++++++++++------------- dll/win32/kernel32/include/kernel32.h | 20 +++++---- sdk/lib/rtl/thread.c | 82 ++++++++++++++++++++--------------- 4 files changed, 102 insertions(+), 75 deletions(-)
diff --git a/dll/win32/kernel32/client/thread.c b/dll/win32/kernel32/client/thread.c index e64cd7230d3..3505d4bdae7 100644 --- 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)) { diff --git a/dll/win32/kernel32/client/utils.c b/dll/win32/kernel32/client/utils.c index e0654c9700c..6e3a3368ad5 100644 --- 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) StackReserve = StackReserveHeader; + if (StackReserve == 0) + StackReserve = Headers->OptionalHeader.SizeOfStackReserve;
- 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; diff --git a/dll/win32/kernel32/include/kernel32.h b/dll/win32/kernel32/include/kernel32.h index 005b5d05272..ef47d4d946c 100644 --- 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 diff --git a/sdk/lib/rtl/thread.c b/sdk/lib/rtl/thread.c index 535380593c9..df7541cdeac 100644 --- 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 ***************************************************************/
-
Hermès Bélusca-Maïto