SysCall: Refactored UserStackTop to allocate it anew for each CallRing3.

MdeModulePkg/Core/Dxe/DxeMain.h

@@ -231,14 +231,12 @@ typedef struct {
   VOID                                    *HiiData;
   BOOLEAN                                 IsUserImage;
   UINTN                                   UserPageTable;
-  UINTN                                   UserStackTop;
 } LOADED_IMAGE_PRIVATE_DATA;
 
 typedef struct {
   VOID        *CoreWrapper;
   VOID        *UserSpaceDriver;
   UINTN       UserPageTable;
-  UINTN       UserStackTop;
   UINT8       NumberOfCalls;
   LIST_ENTRY  Link;
 } USER_SPACE_DRIVER;
@@ -2790,9 +2788,7 @@ FreeProtocolsList (
 UINTN
 EFIAPI
 InitializeUserPageTable (
-  IN LOADED_IMAGE_PRIVATE_DATA  *Image,
+  IN LOADED_IMAGE_PRIVATE_DATA  *Image
-  IN UINTN                      UserStackBase,
-  IN UINTN                      UserStackSize
   );
 
 #endif

MdeModulePkg/Core/Dxe/Image/Image.c

@@ -1033,30 +1033,6 @@ CoreUnloadAndCloseImage (
   CoreFreePool (Image);
 }
 
-STATIC
-UINTN
-EFIAPI
-AllocateStack (
-  IN  UINTN  Size,
-  OUT UINTN  *Base
-  )
-{
-  UINTN  TopOfStack;
-
-  ASSERT (Base != NULL);
-  ASSERT (IS_ALIGNED (Size, EFI_PAGE_SIZE));
-
-  *Base = (UINTN)AllocatePages (EFI_SIZE_TO_PAGES (Size));
-  ASSERT (*Base != 0);
-  //
-  // Compute the top of the allocated stack. Pre-allocate a UINTN for safety.
-  //
-  TopOfStack = *Base + Size - CPU_STACK_ALIGNMENT;
-  TopOfStack = ALIGN_VALUE (TopOfStack, CPU_STACK_ALIGNMENT);
-
-  return TopOfStack;
-}
-
 /**
   Loads an EFI image into memory and returns a handle to the image.
 
@@ -1132,7 +1108,6 @@ CoreLoadImageCommon (
   UEFI_IMAGE_LOADER_IMAGE_CONTEXT ImageContext;
   UINT8                           ImageOrigin;
   EFI_FV_FILE_ATTRIBUTES          FileAttributes;
-  UINTN                           UserStackBase;
 
   SecurityStatus = EFI_SUCCESS;
 
@@ -1470,14 +1445,7 @@ CoreLoadImageCommon (
   ProtectUefiImage (&Image->Info, ImageOrigin, &ImageContext, Image->IsUserImage);
 
   if ((gRing3Data != NULL) && Image->IsUserImage) {
-    Image->UserStackTop = AllocateStack (STACK_SIZE, &UserStackBase);
+    Image->UserPageTable = InitializeUserPageTable (Image);
-    SetUefiImageMemoryAttributes (UserStackBase, STACK_SIZE, EFI_MEMORY_XP | EFI_MEMORY_USER);
-
-    Image->UserPageTable = InitializeUserPageTable (
-                             Image,
-                             UserStackBase,
-                             STACK_SIZE
-                             );
   }
 
   RegisterMemoryProfileImage (
@@ -1742,7 +1710,6 @@ CoreStartImage (
         UserDriver->CoreWrapper     = NULL;
         UserDriver->UserSpaceDriver = (VOID *)Image->EntryPoint;
         UserDriver->UserPageTable   = Image->UserPageTable;
-        UserDriver->UserStackTop    = Image->UserStackTop;
         UserDriver->NumberOfCalls   = 0;
 
         InsertTailList (&gUserSpaceDriversHead, &UserDriver->Link);

MdeModulePkg/Core/Dxe/SysCall/BootServices.c

@@ -547,7 +547,6 @@ CallBootService (
         NewDriver->CoreWrapper     = CoreArgList[Index + 1];
         NewDriver->UserSpaceDriver = UserArgList[Index + 1];
         NewDriver->UserPageTable   = UserDriver->UserPageTable;
-        NewDriver->UserStackTop    = UserDriver->UserStackTop;
         NewDriver->NumberOfCalls   = 0;
 
         InsertTailList (&gUserSpaceDriversHead, &NewDriver->Link);

MdeModulePkg/Core/Dxe/SysCall/Initialization.c

@@ -180,9 +180,7 @@ InitializeRing3 (
 UINTN
 EFIAPI
 InitializeUserPageTable (
-  IN LOADED_IMAGE_PRIVATE_DATA  *Image,
+  IN LOADED_IMAGE_PRIVATE_DATA  *Image
-  IN UINTN                      UserStackBase,
-  IN UINTN                      UserStackSize
   )
 {
   UINTN                      UserPageTable;
@@ -198,7 +196,7 @@ InitializeUserPageTable (
   MakeUserPageTableTemplate (&UserPageTable, &UserPageTableSize);
 
   //
-  // Map gRing3Data, gRing3Interfaces, UserStackBase, DxeRing3
+  // Map gRing3Data, gRing3Interfaces, DxeRing3
   //
   gCpu->SetUserMemoryAttributes (
           gCpu,
@@ -216,14 +214,6 @@ InitializeUserPageTable (
           EFI_MEMORY_XP | EFI_MEMORY_USER
           );
 
-  gCpu->SetUserMemoryAttributes (
-          gCpu,
-          UserPageTable,
-          UserStackBase,
-          UserStackSize,
-          EFI_MEMORY_XP | EFI_MEMORY_USER
-          );
-
   SectionAddress = mDxeRing3->StartAddress;
   for (Index = 0; Index < mDxeRing3->NumSegments; Index++) {
     ImageRecordSegment = &mDxeRing3->Segments[Index];

MdeModulePkg/Core/Dxe/SysCall/SupportedProtocols.c

@@ -19,6 +19,30 @@ CallRing3 (
   IN UINTN            UserStackTop
   );
 
+STATIC
+UINTN
+EFIAPI
+AllocateStack (
+  IN  UINTN  Size,
+  OUT UINTN  *Base
+  )
+{
+  UINTN  TopOfStack;
+
+  ASSERT (Base != NULL);
+  ASSERT (IS_ALIGNED (Size, EFI_PAGE_SIZE));
+
+  *Base = (UINTN)AllocatePages (EFI_SIZE_TO_PAGES (Size));
+  ASSERT (*Base != 0);
+  //
+  // Compute the top of the allocated stack. Pre-allocate a UINTN for safety.
+  //
+  TopOfStack = *Base + Size - CPU_STACK_ALIGNMENT;
+  TopOfStack = ALIGN_VALUE (TopOfStack, CPU_STACK_ALIGNMENT);
+
+  return TopOfStack;
+}
+
 EFI_STATUS
 EFIAPI
 GoToRing3 (
@@ -28,32 +52,22 @@ GoToRing3 (
   ...
   )
 {
-  EFI_STATUS            Status;
+  EFI_STATUS       Status;
-  RING3_CALL_DATA       *Input;
+  RING3_CALL_DATA  *Input;
-  VA_LIST               Marker;
+  VA_LIST          Marker;
-  UINTN                 Index;
+  UINTN            Index;
-  EFI_PHYSICAL_ADDRESS  Ring3Pages;
+  UINTN            UserStackTop;
-  UINT32                PagesNumber;
+  UINTN            UserStackBase;
 
   if (UserDriver->NumberOfCalls > MAX_CALL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
-  PagesNumber = (UINT32)EFI_SIZE_TO_PAGES (sizeof (RING3_CALL_DATA) + Number * sizeof (UINTN));
+  UserStackTop  = AllocateStack (STACK_SIZE, &UserStackBase);
+  UserStackTop -= ALIGN_VALUE (sizeof (RING3_CALL_DATA) + Number * sizeof (UINTN), CPU_STACK_ALIGNMENT);
 
-  Status = CoreAllocatePages (
+  Input = (RING3_CALL_DATA *)UserStackTop;
-             AllocateAnyPages,
-             EfiRing3MemoryType,
-             PagesNumber,
-             &Ring3Pages
-             );
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
 
-  Input = (RING3_CALL_DATA *)(UINTN)Ring3Pages;
-
-  AllowSupervisorAccessToUserMemory ();
   Input->NumberOfArguments = Number;
   Input->EntryPoint        = EntryPoint;
 
@@ -62,21 +76,31 @@ GoToRing3 (
     Input->Arguments[Index] = VA_ARG (Marker, UINTN);
   }
   VA_END (Marker);
-  ForbidSupervisorAccessToUserMemory ();
+
-  //
+  SetUefiImageMemoryAttributes (UserStackBase, STACK_SIZE, EFI_MEMORY_XP | EFI_MEMORY_USER);
-  // TODO: Allocate new stacks (only for EFI_FILE_PROTOCOL instances?),
+
-  // because UserDriver can be interrupted and interrupt handler may call the same UserDriver again.
+  gCpu->SetUserMemoryAttributes (
-  //
+          gCpu,
+          UserDriver->UserPageTable,
+          UserStackBase,
+          STACK_SIZE,
+          EFI_MEMORY_XP | EFI_MEMORY_USER
+          );
+
   ++UserDriver->NumberOfCalls;
+  //
+  // Reserve space on stack for 4 arguments (X64 NOOPT prerequisite).
+  //
+  UserStackTop -= ALIGN_VALUE (8*4, CPU_STACK_ALIGNMENT);
 
   Status = CallRing3 (
              Input,
-             UserDriver->UserStackTop
+             UserStackTop
              );
 
   --UserDriver->NumberOfCalls;
 
-  CoreFreePages (Ring3Pages, PagesNumber);
+  CoreFreePages (UserStackBase, EFI_SIZE_TO_PAGES (STACK_SIZE));
 
   return Status;
 }
@@ -763,7 +787,6 @@ CoreFileOpen (
   NewDriver                  = AllocatePool (sizeof (USER_SPACE_DRIVER));
   NewDriver->CoreWrapper     = NewFile;
   NewDriver->UserPageTable   = UserDriver->UserPageTable;
-  NewDriver->UserStackTop    = UserDriver->UserStackTop;
   NewDriver->NumberOfCalls   = 0;
 
   AllowSupervisorAccessToUserMemory ();
@@ -865,7 +888,6 @@ CoreSimpleFileSystemOpenVolume (
   NewDriver                  = AllocatePool (sizeof (USER_SPACE_DRIVER));
   NewDriver->CoreWrapper     = File;
   NewDriver->UserPageTable   = UserDriver->UserPageTable;
-  NewDriver->UserStackTop    = UserDriver->UserStackTop;
   NewDriver->NumberOfCalls   = 0;
 
   AllowSupervisorAccessToUserMemory ();