From dd71687e442f71065bb0d8c93e5c58ec636ceb1d Mon Sep 17 00:00:00 2001 From: Mikhail Krichanov Date: Fri, 24 Jan 2025 18:44:34 +0300 Subject: [PATCH] SysCall: Refactored UserStackTop to allocate it anew for each CallRing3. --- MdeModulePkg/Core/Dxe/DxeMain.h | 6 +- MdeModulePkg/Core/Dxe/Image/Image.c | 35 +-------- MdeModulePkg/Core/Dxe/SysCall/BootServices.c | 1 - .../Core/Dxe/SysCall/Initialization.c | 14 +--- .../Core/Dxe/SysCall/SupportedProtocols.c | 78 ++++++++++++------- 5 files changed, 54 insertions(+), 80 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h index dc0a89e9fd..77270ce692 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.h +++ b/MdeModulePkg/Core/Dxe/DxeMain.h @@ -230,14 +230,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; @@ -2778,9 +2776,7 @@ FreeProtocolsList ( UINTN EFIAPI InitializeUserPageTable ( - IN LOADED_IMAGE_PRIVATE_DATA *Image, - IN UINTN UserStackBase, - IN UINTN UserStackSize + IN LOADED_IMAGE_PRIVATE_DATA *Image ); #endif diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c index 26d00b8efb..77acc106cc 100644 --- a/MdeModulePkg/Core/Dxe/Image/Image.c +++ b/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); - SetUefiImageMemoryAttributes (UserStackBase, STACK_SIZE, EFI_MEMORY_XP | EFI_MEMORY_USER); - - Image->UserPageTable = InitializeUserPageTable ( - Image, - UserStackBase, - STACK_SIZE - ); + Image->UserPageTable = InitializeUserPageTable (Image); } 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); diff --git a/MdeModulePkg/Core/Dxe/SysCall/BootServices.c b/MdeModulePkg/Core/Dxe/SysCall/BootServices.c index 988fb7c9c9..32f9f966d1 100644 --- a/MdeModulePkg/Core/Dxe/SysCall/BootServices.c +++ b/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); diff --git a/MdeModulePkg/Core/Dxe/SysCall/Initialization.c b/MdeModulePkg/Core/Dxe/SysCall/Initialization.c index a6e23206e7..20949c0019 100644 --- a/MdeModulePkg/Core/Dxe/SysCall/Initialization.c +++ b/MdeModulePkg/Core/Dxe/SysCall/Initialization.c @@ -180,9 +180,7 @@ InitializeRing3 ( UINTN EFIAPI InitializeUserPageTable ( - IN LOADED_IMAGE_PRIVATE_DATA *Image, - IN UINTN UserStackBase, - IN UINTN UserStackSize + IN LOADED_IMAGE_PRIVATE_DATA *Image ) { 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]; diff --git a/MdeModulePkg/Core/Dxe/SysCall/SupportedProtocols.c b/MdeModulePkg/Core/Dxe/SysCall/SupportedProtocols.c index 91820e3713..c09503d717 100644 --- a/MdeModulePkg/Core/Dxe/SysCall/SupportedProtocols.c +++ b/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; - RING3_CALL_DATA *Input; - VA_LIST Marker; - UINTN Index; - EFI_PHYSICAL_ADDRESS Ring3Pages; - UINT32 PagesNumber; + EFI_STATUS Status; + RING3_CALL_DATA *Input; + VA_LIST Marker; + UINTN Index; + UINTN UserStackTop; + 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 ( - AllocateAnyPages, - EfiRing3MemoryType, - PagesNumber, - &Ring3Pages - ); - if (EFI_ERROR (Status)) { - return Status; - } + Input = (RING3_CALL_DATA *)UserStackTop; - 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 (); - // - // TODO: Allocate new stacks (only for EFI_FILE_PROTOCOL instances?), - // because UserDriver can be interrupted and interrupt handler may call the same UserDriver again. - // + + SetUefiImageMemoryAttributes (UserStackBase, STACK_SIZE, EFI_MEMORY_XP | EFI_MEMORY_USER); + + 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 ();