From ef051451367fa5455d75b38c407ca352a43b4849 Mon Sep 17 00:00:00 2001 From: Oliver Smith-Denny Date: Wed, 2 Aug 2023 11:59:28 -0700 Subject: ArmPkg: DefaultExceptionHandlerLib: Do Not Allocate Memory If gST->ConOut is available when Arm's DefaultExceptionHandler is running, AsciiPrint will get called to attempt to print to ConOut, in addition to the serial output. AsciiPrint calls AsciiInternalPrint in UefiLibPrint.c which in turn calls AllocatePool to allocate a buffer to convert the Ascii input string to a Unicode string to pass to ConOut->OutputString. Per the comment on DefaultExceptionHandler, we should not be allocating memory in the exception handler, as this can cause the exception handler to fail if we had a memory exception or the system state is such that we cannot allocate memory. It has been observed on ArmVirtQemu that exceptions generated in the memory handling code will fail to output the stack dump and CPU state that is critical to debugging because the AllocatePool will fail. This patch fixes the Arm and AARCH64 DefaultExceptionHandlers to not allocate memory when ConOut is available and instead use stack memory to convert the Ascii string needed for SerialPortWrite to the Unicode string needed for ConOut->OutputString. Correspondingly, ArmVirtQemu can now output the stack dump and CPU state when hitting an exception in memory code. Signed-off-by: Oliver Smith-Denny --- .../AArch64/DefaultExceptionHandler.c | 32 ++++++++++++++-------- .../Arm/DefaultExceptionHandler.c | 24 +++++++++++++--- 2 files changed, 41 insertions(+), 15 deletions(-) (limited to 'ArmPkg') diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c index f2bca5d..a39896d 100644 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c @@ -22,6 +22,12 @@ #include #include +// +// Maximum number of characters to print to serial (UINT8s) and to console if +// available (as UINT16s) +// +#define MAX_PRINT_CHARS 100 + STATIC CHAR8 *gExceptionTypeString[] = { "Synchronous", "IRQ", @@ -188,18 +194,14 @@ DefaultExceptionHandler ( IN OUT EFI_SYSTEM_CONTEXT SystemContext ) { - CHAR8 Buffer[100]; - UINTN CharCount; - INT32 Offset; + CHAR8 Buffer[MAX_PRINT_CHARS]; + CHAR16 UnicodeBuffer[MAX_PRINT_CHARS]; + UINTN CharCount; + INT32 Offset; if (mRecursiveException) { STATIC CHAR8 CONST Message[] = "\nRecursive exception occurred while dumping the CPU state\n"; - SerialPortWrite ((UINT8 *)Message, sizeof Message - 1); - if (gST->ConOut != NULL) { - AsciiPrint (Message); - } - CpuDeadLoop (); } @@ -207,9 +209,10 @@ DefaultExceptionHandler ( CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n\n%a Exception at 0x%016lx\n", gExceptionTypeString[ExceptionType], SystemContext.SystemContextAArch64->ELR); SerialPortWrite ((UINT8 *)Buffer, CharCount); - if (gST->ConOut != NULL) { - AsciiPrint (Buffer); - } + + // Prepare a unicode buffer for ConOut, if applicable, in case the buffer + // gets reused. + UnicodeSPrintAsciiFormat (UnicodeBuffer, MAX_PRINT_CHARS, Buffer); DEBUG_CODE_BEGIN (); CHAR8 *Pdb, *PrevPdb; @@ -330,6 +333,13 @@ DefaultExceptionHandler ( )); } + // Attempt to print that we had a synchronous exception to ConOut. We do + // this after the serial logging as ConOut's logging is more complex and we + // aren't guaranteed to succeed. + if (gST->ConOut != NULL) { + gST->ConOut->OutputString (gST->ConOut, UnicodeBuffer); + } + ASSERT (FALSE); CpuDeadLoop (); } diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c index 13b321e..accad64 100644 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c @@ -24,6 +24,12 @@ #include // +// Maximum number of characters to print to serial (UINT8s) and to console if +// available (as UINT16s) +// +#define MAX_PRINT_CHARS 100 + +// // The number of elements in a CHAR8 array, including the terminating NUL, that // is meant to hold the string rendering of the CPSR. // @@ -198,7 +204,8 @@ DefaultExceptionHandler ( IN OUT EFI_SYSTEM_CONTEXT SystemContext ) { - CHAR8 Buffer[100]; + CHAR8 Buffer[MAX_PRINT_CHARS]; + CHAR16 UnicodeBuffer[MAX_PRINT_CHARS]; UINTN CharCount; UINT32 DfsrStatus; UINT32 IfsrStatus; @@ -216,9 +223,10 @@ DefaultExceptionHandler ( SystemContext.SystemContextArm->CPSR ); SerialPortWrite ((UINT8 *)Buffer, CharCount); - if (gST->ConOut != NULL) { - AsciiPrint (Buffer); - } + + // Prepare a unicode buffer for ConOut, if applicable, as Buffer is used + // below. + UnicodeSPrintAsciiFormat (UnicodeBuffer, MAX_PRINT_CHARS, Buffer); DEBUG_CODE_BEGIN (); CHAR8 *Pdb; @@ -289,6 +297,14 @@ DefaultExceptionHandler ( } DEBUG ((DEBUG_ERROR, "\n")); + + // Attempt to print that we had a synchronous exception to ConOut. We do + // this after the serial logging as ConOut's logging is more complex and we + // aren't guaranteed to succeed. + if (gST->ConOut != NULL) { + gST->ConOut->OutputString (gST->ConOut, UnicodeBuffer); + } + ASSERT (FALSE); CpuDeadLoop (); // may return if executing under a debugger -- cgit v1.1