[ros-diffs] [ion] 54285: [KERNEL32]: Fix multiple issues in BaseCreateStack: - StackLimit was set incorrectly. - Code was not using BaseStaticServerdata, but querying NT instead. - Fix memory leak in failure c...

ion at svn.reactos.org ion at svn.reactos.org
Thu Nov 3 06:46:24 UTC 2011


Author: ion
Date: Thu Nov  3 06:46:22 2011
New Revision: 54285

URL: http://svn.reactos.org/svn/reactos?rev=54285&view=rev
Log:
[KERNEL32]: Fix multiple issues in BaseCreateStack:
- StackLimit was set incorrectly.
- Code was not using BaseStaticServerdata, but querying NT instead.
- Fix memory leak in failure case.
- StackCommit and StackReserved values were not aligned correctly.
- Windows Server 2003+ feature of "Guaranteed Stack Commit Size" was not respected.
- Some math was screwy.
- Failure to get NT headers was not handled.

Modified:
    trunk/reactos/dll/win32/kernel32/client/fiber.c
    trunk/reactos/dll/win32/kernel32/client/proc.c
    trunk/reactos/dll/win32/kernel32/client/thread.c
    trunk/reactos/dll/win32/kernel32/client/utils.c
    trunk/reactos/dll/win32/kernel32/include/kernel32.h

Modified: trunk/reactos/dll/win32/kernel32/client/fiber.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/fiber.c?rev=54285&r1=54284&r2=54285&view=diff
==============================================================================
--- trunk/reactos/dll/win32/kernel32/client/fiber.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/client/fiber.c [iso-8859-1] Thu Nov  3 06:46:22 2011
@@ -201,7 +201,7 @@
     }
 
     /* Create the stack for the fiber */
-    Status = BasepCreateStack(NtCurrentProcess(),
+    Status = BaseCreateStack(NtCurrentProcess(),
                               dwStackCommitSize,
                               dwStackReserveSize,
                               &InitialTeb);

Modified: trunk/reactos/dll/win32/kernel32/client/proc.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/proc.c?rev=54285&r1=54284&r2=54285&view=diff
==============================================================================
--- trunk/reactos/dll/win32/kernel32/client/proc.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/client/proc.c [iso-8859-1] Thu Nov  3 06:46:22 2011
@@ -187,7 +187,7 @@
     DPRINT("BasepCreateFirstThread. hProcess: %lx\n", ProcessHandle);
 
     /* Create the Thread's Stack */
-    BasepCreateStack(ProcessHandle,
+    BaseCreateStack(ProcessHandle,
                      SectionImageInfo->MaximumStackSize,
                      SectionImageInfo->CommittedStackSize,
                      &InitialTeb);

Modified: trunk/reactos/dll/win32/kernel32/client/thread.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/thread.c?rev=54285&r1=54284&r2=54285&view=diff
==============================================================================
--- trunk/reactos/dll/win32/kernel32/client/thread.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/client/thread.c [iso-8859-1] Thu Nov  3 06:46:22 2011
@@ -137,7 +137,7 @@
     ClientId.UniqueProcess = hProcess;
 
     /* Create the Stack */
-    Status = BasepCreateStack(hProcess,
+    Status = BaseCreateStack(hProcess,
                               dwStackSize,
                               dwCreationFlags & STACK_SIZE_PARAM_IS_A_RESERVATION ?
                               dwStackSize : 0,

Modified: trunk/reactos/dll/win32/kernel32/client/utils.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/client/utils.c?rev=54285&r1=54284&r2=54285&view=diff
==============================================================================
--- trunk/reactos/dll/win32/kernel32/client/utils.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/client/utils.c [iso-8859-1] Thu Nov  3 06:46:22 2011
@@ -247,60 +247,64 @@
  */
 NTSTATUS
 WINAPI
-BasepCreateStack(HANDLE hProcess,
+BaseCreateStack(HANDLE hProcess,
                  SIZE_T StackReserve,
                  SIZE_T StackCommit,
                  PINITIAL_TEB InitialTeb)
 {
     NTSTATUS Status;
-    SYSTEM_BASIC_INFORMATION SystemBasicInfo;
     PIMAGE_NT_HEADERS Headers;
-    ULONG_PTR Stack = 0;
-    BOOLEAN UseGuard = FALSE;
-
-    DPRINT("BasepCreateStack (hProcess: %lx, Max: %lx, Current: %lx)\n",
+    ULONG_PTR Stack;
+    BOOLEAN UseGuard;
+    ULONG PageSize, Dummy, AllocationGranularity;
+    SIZE_T StackReserveHeader, StackCommitHeader, GuardPageSize, GuaranteedStackCommit;
+    DPRINT("BaseCreateStack (hProcess: %lx, Max: %lx, Current: %lx)\n",
             hProcess, StackReserve, StackCommit);
 
-    /* Get some memory information */
-    Status = NtQuerySystemInformation(SystemBasicInformation,
-                                      &SystemBasicInfo,
-                                      sizeof(SYSTEM_BASIC_INFORMATION),
-                                      NULL);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT1("Failure to query system info\n");
-        return Status;
-    }
-
-    /* Use the Image Settings if we are dealing with the current Process */
-    if (hProcess == NtCurrentProcess())
-    {
-        /* Get the Image Headers */
-        Headers = RtlImageNtHeader(NtCurrentPeb()->ImageBaseAddress);
-
-        /* If we didn't get the parameters, find them ourselves */
-        StackReserve = (StackReserve) ?
-                        StackReserve : Headers->OptionalHeader.SizeOfStackReserve;
-        StackCommit = (StackCommit) ?
-                       StackCommit : Headers->OptionalHeader.SizeOfStackCommit;
-    }
-    else
-    {
-        /* Use the System Settings if needed */
-        StackReserve = (StackReserve) ? StackReserve :
-                                        SystemBasicInfo.AllocationGranularity;
-        StackCommit = (StackCommit) ? StackCommit : SystemBasicInfo.PageSize;
-    }
-
-    /* Align everything to Page Size */
-    StackReserve = ROUND_UP(StackReserve, SystemBasicInfo.AllocationGranularity);
-    StackCommit = ROUND_UP(StackCommit, SystemBasicInfo.PageSize);
-    #if 1 // FIXME: Remove once Guard Page support is here
+    /* Read page size */
+    PageSize = BaseStaticServerData->SysInfo.PageSize;
+    AllocationGranularity = BaseStaticServerData->SysInfo.AllocationGranularity;
+
+    /* Get the Image Headers */
+    Headers = RtlImageNtHeader(NtCurrentPeb()->ImageBaseAddress);
+    if (!Headers) return STATUS_INVALID_IMAGE_FORMAT;
+
+    StackCommitHeader = Headers->OptionalHeader.SizeOfStackCommit;
+    StackReserveHeader = Headers->OptionalHeader.SizeOfStackReserve;
+
+    if (!StackReserve) StackReserve = StackReserveHeader;
+
+    if (!StackCommit)
+    {
+        StackCommit = StackCommitHeader;
+    }
+    else if (StackCommit >= StackReserve)
+    {
+        StackReserve = ROUND_UP(StackCommit, 1024 * 1024);
+    }
+    
+    StackCommit = ROUND_UP(StackCommit, PageSize);
+    StackReserve = ROUND_UP(StackReserve, AllocationGranularity);
+    
+    GuaranteedStackCommit = NtCurrentTeb()->GuaranteedStackBytes;
+    if ((GuaranteedStackCommit) && (StackCommit < GuaranteedStackCommit))
+    {
+        StackCommit = GuaranteedStackCommit;
+    }
+
+    if (StackCommit >= StackReserve)
+    {
+        StackReserve = ROUND_UP(StackCommit, 1024 * 1024);
+    }
+
+    StackCommit = ROUND_UP(StackCommit, PageSize);
+    StackReserve = ROUND_UP(StackReserve, AllocationGranularity);
+    
+    /* ROS Hack until we support guard page stack expansion */
     StackCommit = StackReserve;
-    #endif
-    DPRINT("StackReserve: %lx, StackCommit: %lx\n", StackReserve, StackCommit);
 
     /* Reserve memory for the stack */
+    Stack = 0;
     Status = ZwAllocateVirtualMemory(hProcess,
                                      (PVOID*)&Stack,
                                      0,
@@ -309,7 +313,7 @@
                                      PAGE_READWRITE);
     if (!NT_SUCCESS(Status))
     {
-        DPRINT1("Failure to reserve stack\n");
+        DPRINT1("Failure to reserve stack: %lx\n", Status);
         return Status;
     }
 
@@ -325,11 +329,15 @@
     /* Check if we will need a guard page */
     if (StackReserve > StackCommit)
     {
-        Stack -= SystemBasicInfo.PageSize;
-        StackCommit += SystemBasicInfo.PageSize;
+        Stack -= PageSize;
+        StackCommit += PageSize;
         UseGuard = TRUE;
     }
-
+    else
+    {
+        UseGuard = FALSE;
+    }
+    
     /* Allocate memory for the stack */
     Status = ZwAllocateVirtualMemory(hProcess,
                                      (PVOID*)&Stack,
@@ -340,6 +348,8 @@
     if (!NT_SUCCESS(Status))
     {
         DPRINT1("Failure to allocate stack\n");
+        GuardPageSize = 0;
+        ZwFreeVirtualMemory(hProcess, (PVOID*)&Stack, &GuardPageSize, MEM_RELEASE);
         return Status;
     }
 
@@ -349,10 +359,8 @@
     /* Create a guard page */
     if (UseGuard)
     {
-        SIZE_T GuardPageSize = SystemBasicInfo.PageSize;
-        ULONG Dummy;
-
-        /* Attempt maximum space possible */
+        /* Set the guard page */
+        GuardPageSize = PAGE_SIZE;
         Status = ZwProtectVirtualMemory(hProcess,
                                         (PVOID*)&Stack,
                                         &GuardPageSize,
@@ -360,12 +368,13 @@
                                         &Dummy);
         if (!NT_SUCCESS(Status))
         {
-            DPRINT1("Failure to create guard page\n");
+            DPRINT1("Failure to set guard page\n");
             return Status;
         }
 
         /* Update the Stack Limit keeping in mind the Guard Page */
-        InitialTeb->StackLimit = (PVOID)((ULONG_PTR)InitialTeb->StackLimit - GuardPageSize);
+        InitialTeb->StackLimit = (PVOID)((ULONG_PTR)InitialTeb->StackLimit +
+                                         GuardPageSize);
     }
 
     /* We are done! */

Modified: trunk/reactos/dll/win32/kernel32/include/kernel32.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/include/kernel32.h?rev=54285&r1=54284&r2=54285&view=diff
==============================================================================
--- trunk/reactos/dll/win32/kernel32/include/kernel32.h [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/include/kernel32.h [iso-8859-1] Thu Nov  3 06:46:22 2011
@@ -173,7 +173,7 @@
                              
 NTSTATUS
 WINAPI
-BasepCreateStack(HANDLE hProcess,
+BaseCreateStack(HANDLE hProcess,
                  SIZE_T StackReserve,
                  SIZE_T StackCommit,
                  PINITIAL_TEB InitialTeb);




More information about the Ros-diffs mailing list