From 5c919d20d92336d9bb08b77c9c5f6ca08784e8d5 Mon Sep 17 00:00:00 2001 From: Mikhail Krichanov Date: Mon, 27 Jan 2025 11:50:49 +0300 Subject: [PATCH] Ring3: Fixed memory leaks and passed UserArguments to CallBootService through User stack for ARM, AARCH64. --- .../AArch64/ExceptionSupport.S | 7 +-- .../Dxe/SysCall/AARCH64/InitializeAARCH64.c | 28 +++++----- .../Core/Dxe/SysCall/ARM/InitializeARM.c | 51 +++++++------------ MdeModulePkg/Core/Dxe/SysCall/BootServices.c | 11 +++- MdePkg/Include/Protocol/DebugSupport.h | 13 +++-- 5 files changed, 52 insertions(+), 58 deletions(-) diff --git a/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S b/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S index 78dc83f39f..481eb2bd82 100644 --- a/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S +++ b/ArmPkg/Library/ArmExceptionLib/AArch64/ExceptionSupport.S @@ -91,7 +91,7 @@ UINT64 FAR; 0x320 // Fault Address Register UINT64 Type; 0x328 // Exception Type UINT64 TTBR0; 0x330 // User Page Table - UINT64 Padding;0x338 // Alignment requirement + UINT64 SP_EL0; 0x338 // User Stack Pointer */ GCC_ASM_EXPORT(ExceptionHandlersEnd) @@ -101,7 +101,7 @@ GCC_ASM_EXPORT(CommonCExceptionHandler) #define GP_CONTEXT_SIZE (32 * 8) #define FP_CONTEXT_SIZE (32 * 16) -#define SYS_CONTEXT_SIZE ( 8 * 8) // 7 SYS regs + Alignment requirement (ie: the stack must be aligned on 0x10) +#define SYS_CONTEXT_SIZE ( 8 * 8) // 8 SYS regs + Alignment requirement (ie: the stack must be aligned on 0x10) ASM_FUNC_ALIGN(ExceptionHandlerBase, 4096) @@ -270,7 +270,8 @@ ASM_PFX(CommonExceptionEntry): 1:mrs x2, ttbr0_el1 b 3f 2:mrs x2, ttbr0_el2 -3:stp x2, xzr, [x28, #0x30] +3:mrs x3, sp_el0 + stp x2, x3, [x28, #0x30] // Push FP regs to Stack. stp q0, q1, [x28, #-FP_CONTEXT_SIZE]! diff --git a/MdeModulePkg/Core/Dxe/SysCall/AARCH64/InitializeAARCH64.c b/MdeModulePkg/Core/Dxe/SysCall/AARCH64/InitializeAARCH64.c index 832a44d73d..3541667e50 100644 --- a/MdeModulePkg/Core/Dxe/SysCall/AARCH64/InitializeAARCH64.c +++ b/MdeModulePkg/Core/Dxe/SysCall/AARCH64/InitializeAARCH64.c @@ -30,34 +30,32 @@ SysCallBootService ( IN EFI_SYSTEM_CONTEXT Context ) { - EFI_STATUS Status; - EFI_PHYSICAL_ADDRESS Physical; + EFI_STATUS Status; + UINTN *UserArguments; ArmEnableInterrupts (); - Status = CoreAllocatePages ( - AllocateAnyPages, - EfiRing3MemoryType, - EFI_SIZE_TO_PAGES (7 * sizeof (UINTN)), - &Physical - ); - if (EFI_ERROR (Status)) { - return Status; - } + UserArguments = (UINTN *)(Context.SystemContextAArch64->SP_EL0 - 7 * sizeof (UINTN)); AllowSupervisorAccessToUserMemory (); - CopyMem ((VOID *)Physical, (VOID *)&(Context.SystemContextAArch64->X1), 7 * sizeof (UINTN)); + // + // First 6 arguments are passed through X2-X7 and copied to Core stack, + // all the others are on User stack. + // + CopyMem ( + (VOID *)UserArguments, + (VOID *)&(Context.SystemContextAArch64->X1), + 7 * sizeof (UINTN) + ); ForbidSupervisorAccessToUserMemory (); Status = CallBootService ( Context.SystemContextAArch64->X0, Context.SystemContextAArch64->X1, - (UINTN *)Physical, + UserArguments, Context.SystemContextAArch64->SP ); - CoreFreePages (Physical, EFI_SIZE_TO_PAGES (7 * sizeof (UINTN))); - ArmDisableInterrupts (); return Status; diff --git a/MdeModulePkg/Core/Dxe/SysCall/ARM/InitializeARM.c b/MdeModulePkg/Core/Dxe/SysCall/ARM/InitializeARM.c index 7cdd051817..66804b9d71 100644 --- a/MdeModulePkg/Core/Dxe/SysCall/ARM/InitializeARM.c +++ b/MdeModulePkg/Core/Dxe/SysCall/ARM/InitializeARM.c @@ -29,10 +29,10 @@ SysCallBootService ( IN EFI_SYSTEM_CONTEXT Context ) { - EFI_STATUS Status; - EFI_PHYSICAL_ADDRESS Physical; - UINT8 Type; - UINT8 NumberOfArguments; + EFI_STATUS Status; + UINT8 Type; + UINT8 NumberOfArguments; + UINTN *UserArguments; ArmEnableInterrupts (); @@ -47,16 +47,6 @@ SysCallBootService ( ++NumberOfArguments; } - Status = CoreAllocatePages ( - AllocateAnyPages, - EfiRing3MemoryType, - EFI_SIZE_TO_PAGES ((NumberOfArguments + 1) * sizeof (UINTN)), - &Physical - ); - if (EFI_ERROR (Status)) { - return Status; - } - AllowSupervisorAccessToUserMemory (); if (Type == SysCallFreePages) { // @@ -64,41 +54,34 @@ SysCallBootService ( // [SP] == Memory // Memory is passed as 2 words on stack and aligned on 8 bytes. // - CopyMem ((VOID *)(UINTN)Physical, (VOID *)&(Context.SystemContextArm->R1), 2 * sizeof (UINTN)); + UserArguments = (UINTN *)(Context.SystemContextArm->SP - 2 * sizeof (UINTN)); + CopyMem ( - (VOID *)((UINTN)Physical + 2 * sizeof (UINTN)), - (VOID *)Context.SystemContextArm->SP, + (VOID *)UserArguments, + (VOID *)&(Context.SystemContextArm->R1), 2 * sizeof (UINTN) ); } else { // - // First 2 arguments are passed through R2-R3 and copied to SysCall Stack. + // First 2 arguments are passed through R2-R3 and copied to Core stack, + // all the others are on User stack. // - CopyMem ((VOID *)(UINTN)Physical, (VOID *)&(Context.SystemContextArm->R1), 3 * sizeof (UINTN)); + UserArguments = (UINTN *)(Context.SystemContextArm->SP - 3 * sizeof (UINTN)); - if (NumberOfArguments > 2) { - // - // All remaining arguments are on User Stack. - // - CopyMem ( - (VOID *)((UINTN)Physical + 3 * sizeof (UINTN)), - (VOID *)Context.SystemContextArm->SP, - (NumberOfArguments - 2) * sizeof (UINTN) - ); - } + CopyMem ( + (VOID *)UserArguments, + (VOID *)&(Context.SystemContextArm->R1), + 3 * sizeof (UINTN) + ); } ForbidSupervisorAccessToUserMemory (); Status = CallBootService ( Type, NumberOfArguments, - (UINTN *)(UINTN)Physical, + UserArguments, Context.SystemContextArm->SP_EL1 ); - // - // TODO: Fix memory leak for ReturnToCore(). - // - CoreFreePages (Physical, EFI_SIZE_TO_PAGES ((NumberOfArguments + 1) * sizeof (UINTN))); ArmDisableInterrupts (); diff --git a/MdeModulePkg/Core/Dxe/SysCall/BootServices.c b/MdeModulePkg/Core/Dxe/SysCall/BootServices.c index 32f9f966d1..7faf5dfb19 100644 --- a/MdeModulePkg/Core/Dxe/SysCall/BootServices.c +++ b/MdeModulePkg/Core/Dxe/SysCall/BootServices.c @@ -396,7 +396,15 @@ CallBootService ( switch (Type) { case SysCallReturnToCore: - ReturnToCore (Arguments[1], ReturnSP); + // + // Argument 1: EFI_STATUS Status + // Argument 2: UINTN ReturnSP + // + Status = (EFI_STATUS)Arguments[1]; + + FreePool (Arguments); + + ReturnToCore (Status, ReturnSP); break; case SysCallLocateProtocol: // @@ -1510,5 +1518,6 @@ CallBootService ( break; } + FreePool (Arguments); return EFI_UNSUPPORTED; } diff --git a/MdePkg/Include/Protocol/DebugSupport.h b/MdePkg/Include/Protocol/DebugSupport.h index c65fe48e1b..99b5f9a6cb 100644 --- a/MdePkg/Include/Protocol/DebugSupport.h +++ b/MdePkg/Include/Protocol/DebugSupport.h @@ -594,11 +594,14 @@ typedef struct { UINT64 V30[2]; UINT64 V31[2]; - UINT64 ELR; // Exception Link Register - UINT64 SPSR; // Saved Processor Status Register - UINT64 FPSR; // Floating Point Status Register - UINT64 ESR; // Exception syndrome register - UINT64 FAR; // Fault Address Register + UINT64 ELR; // Exception Link Register + UINT64 SPSR; // Saved Processor Status Register + UINT64 FPSR; // Floating Point Status Register + UINT64 ESR; // Exception syndrome register + UINT64 FAR; // Fault Address Register + UINT64 Type; // Exception Type + UINT64 TTBR0; // User Page Table + UINT64 SP_EL0; // User Stack Pointer } EFI_SYSTEM_CONTEXT_AARCH64; ///