From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=65.50.211.136; helo=terminus.zytor.com; envelope-from=pcacjr@zytor.com; receiver=edk2-devel@lists.01.org Received: from terminus.zytor.com (terminus.zytor.com [65.50.211.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 3333E21B00DC1 for ; Thu, 16 Nov 2017 14:10:11 -0800 (PST) Received: from terminus.zytor.com (localhost [127.0.0.1]) by terminus.zytor.com (8.15.2/8.15.2) with ESMTP id vAGMDHo3010047; Thu, 16 Nov 2017 14:13:17 -0800 Received: (from apache@localhost) by terminus.zytor.com (8.15.2/8.15.2/Submit) id vAGMDEti010031; Thu, 16 Nov 2017 20:13:14 -0200 X-Authentication-Warning: terminus.zytor.com: apache set sender to pcacjr@zytor.com using -f Received: from 201.47.212.245 (SquirrelMail authenticated user pcacjr) by www.zytor.com with HTTP; Thu, 16 Nov 2017 20:13:16 -0200 Message-ID: In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503AA1E9D1@shsmsx102.ccr.corp.intel.com> References: <11840074660da43fd43fac88cff851f1ccc31143.1510778784.git.pcacjr@zytor.com> <74D8A39837DF1E4DA445A8C0B3885C503AA1E9D1@shsmsx102.ccr.corp.intel.com> Date: Thu, 16 Nov 2017 20:13:16 -0200 From: "Paulo Alcantara" To: "Yao, Jiewen" Cc: "Paulo Alcantara" , "edk2-devel@lists.01.org" , "Laszlo Ersek" , "Dong, Eric" User-Agent: SquirrelMail/1.4.22-19.fc24 MIME-Version: 1.0 X-Priority: 3 (Normal) Importance: Normal X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham autolearn_force=no version=3.4.1 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on terminus.zytor.com Subject: Re: [RFC v2 1/3] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add stack trace support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Nov 2017 22:10:11 -0000 Content-Type: text/plain;charset=iso-8859-1 Content-Transfer-Encoding: 8bit Hi Jiewen, On Wed, November 15, 2017 11:57 pm, Yao, Jiewen wrote: > Hi Paulo > Thanks to bring this cool feature. > > I have same feeling as Jeff Fan. It is great! > > I have some questions, hope you can clarify. > 1) Would you please let us know which tool change is validated? Such as > MSVC, or GCC? GCC only. This should not work on MSVC because it relies on frame pointers. > 2) Would you please let us know which phase is validated? Such as PEI > PreMemory, PEI PostMemory, DXE, SMM? DXE. I'm currently testing it by placing a random "CpuBreakpoint ()" in PartitionDxe driver. > 3) Would you please let us know if there is any special condition > validated? Such as code is in an interrupt handler, SMM initialization > code, thunk code during mode switch, etc. > I ask this because I have seen lots of discussion on sanity check, to > avoid double exception. At the moment I'm only ensuring proper aligned RSP and ESP values. But we still need to validate the RIP values, etc. as Andrew and Brian suggested. > 4) We supported some security feature to create a hole in normal memory > map, such as PageGuard, StackGuard, SMM communication protection, etc. > Accessing those memory directly will cause #PF immediately. > Would you please let us know how we detect that case, to avoid RIP or RSP > fail to the non-present page? Sorry. I have no idea :-( I'd hope to get some help from you guys. > 5) May I know why we check RIP < ImageBase? Is that legal or illegal if > RIP > ImageBase+ImageSize, but RIP in another PE/COFF image? That check was a wrong assumption that I had in the beginning. RIP may point to either lower or higher addresses where the other PE/COFF images are located. Fixed it in v3. Sorry for the delay in the response. I'm only able to work on this in my free time. Thanks! Paulo >> + // >> + // Check if RIP is within another PE/COFF image base address >> + // >> + if (Rip < ImageBase) { >> + // >> + // Search for the respective PE/COFF image based on RIP >> + // > > Thank you > Yao Jiewen > > >> -----Original Message----- >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of >> Paulo >> Alcantara >> Sent: Thursday, November 16, 2017 9:18 AM >> To: edk2-devel@lists.01.org >> Cc: Laszlo Ersek ; Dong, Eric >> Subject: [edk2] [RFC v2 1/3] UefiCpuPkg/CpuExceptionHandlerLib/X64: Add >> stack >> trace support >> >> This patch adds stack trace support during a X64 CPU exception. >> >> It will dump out back trace, stack contents as well as image module >> names that were part of the call stack. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Cc: Eric Dong >> Cc: Laszlo Ersek >> Signed-off-by: Paulo Alcantara >> --- >> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | >> 369 >> +++++++++++++++++++- >> 1 file changed, 367 insertions(+), 2 deletions(-) >> >> diff --git >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c >> index 65f0cff680..11cd7c9e1c 100644 >> --- >> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c >> +++ >> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c >> @@ -14,6 +14,11 @@ >> >> #include "CpuExceptionCommon.h" >> >> +// >> +// Unknown PDB file name >> +// >> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 >> *mUnknownPdbFileName = "????"; >> + >> /** >> Return address map of exception handler template so that C code can >> generate >> exception tables. >> @@ -242,6 +247,350 @@ DumpCpuContext ( >> ); >> } >> >> +/** >> + Get absolute path and file name of PDB file in PE/COFF image. >> + >> + @param[in] ImageBase Base address of PE/COFF image. >> + @param[out] PdbAbsoluteFilePath Absolute path of PDB file. >> + @param[out] PdbFileName File name of PDB file. >> +**/ >> +STATIC >> +VOID >> +GetPdbFileName ( >> + IN UINTN ImageBase, >> + OUT CHAR8 **PdbAbsoluteFilePath, >> + OUT CHAR8 **PdbFileName >> + ) >> +{ >> + VOID *PdbPointer; >> + CHAR8 *Str; >> + >> + // >> + // Get PDB file name from PE/COFF image >> + // >> + PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)ImageBase); >> + if (PdbPointer == NULL) { >> + // >> + // No PDB file name found. Set it to an unknown file name. >> + // >> + *PdbFileName = (CHAR8 *)mUnknownPdbFileName; >> + if (PdbAbsoluteFilePath != NULL) { >> + *PdbAbsoluteFilePath = NULL; >> + } >> + } else { >> + // >> + // Get file name portion out of PDB file in PE/COFF image >> + // >> + Str = (CHAR8 *)((UINTN)PdbPointer + >> + AsciiStrLen ((CHAR8 *)PdbPointer) - sizeof *Str); >> + for (; *Str != '/' && *Str != '\\'; Str--) { >> + ; >> + } >> + >> + // >> + // Set PDB file name (also skip trailing path separator: '/' or >> '\\') >> + // >> + *PdbFileName = Str + 1; >> + >> + if (PdbAbsoluteFilePath != NULL) { >> + // >> + // Set absolute file path of PDB file >> + // >> + *PdbAbsoluteFilePath = PdbPointer; >> + } >> + } >> +} >> + >> +/** >> + Dump stack contents. >> + >> + @param[in] CurrentRsp Current stack pointer address. >> + @param[in] UnwondStacksCount Count of unwond stack frames. >> +**/ >> +STATIC >> +VOID >> +DumpStackContents ( >> + IN UINT64 CurrentRsp, >> + IN INTN UnwondStacksCount >> + ) >> +{ >> + // >> + // Check for proper stack pointer alignment >> + // >> + if (((UINTN)CurrentRsp & (CPU_STACK_ALIGNMENT - 1)) != 0) { >> + InternalPrintMessage ("!!!! Unaligned stack pointer. !!!!\n"); >> + return; >> + } >> + >> + // >> + // Dump out stack contents >> + // >> + InternalPrintMessage ("\nStack dump:\n"); >> + while (UnwondStacksCount-- > 0) { >> + InternalPrintMessage ( >> + "0x%016lx: %016lx %016lx\n", >> + CurrentRsp, >> + *(UINT64 *)CurrentRsp, >> + *(UINT64 *)((UINTN)CurrentRsp + 8) >> + ); >> + >> + // >> + // Point to next stack >> + // >> + CurrentRsp += CPU_STACK_ALIGNMENT; >> + } >> +} >> + >> +/** >> + Dump all image module names from call stack. >> + >> + @param[in] SystemContext Pointer to EFI_SYSTEM_CONTEXT. >> +**/ >> +STATIC >> +VOID >> +DumpImageModuleNames ( >> + IN EFI_SYSTEM_CONTEXT SystemContext >> + ) >> +{ >> + EFI_STATUS Status; >> + UINT64 Rip; >> + UINTN ImageBase; >> + VOID *EntryPoint; >> + CHAR8 *PdbAbsoluteFilePath; >> + CHAR8 *PdbFileName; >> + UINT64 Rbp; >> + >> + // >> + // Set current RIP address >> + // >> + Rip = SystemContext.SystemContextX64->Rip; >> + >> + // >> + // Set current frame pointer address >> + // >> + Rbp = SystemContext.SystemContextX64->Rbp; >> + >> + // >> + // Check for proper frame pointer alignment >> + // >> + if (((UINTN)Rbp & (CPU_STACK_ALIGNMENT - 1)) != 0) { >> + InternalPrintMessage ("!!!! Unaligned frame pointer. !!!!\n"); >> + return; >> + } >> + >> + // >> + // Get initial PE/COFF image base address from current RIP >> + // >> + ImageBase = PeCoffSearchImageBase (Rip); >> + if (ImageBase == 0) { >> + InternalPrintMessage ("!!!! Could not find image module names. >> !!!!"); >> + return; >> + } >> + >> + // >> + // Get initial PE/COFF image's entry point >> + // >> + Status = PeCoffLoaderGetEntryPoint ((VOID *)ImageBase, &EntryPoint); >> + if (EFI_ERROR (Status)) { >> + EntryPoint = NULL; >> + } >> + >> + // >> + // Get file name and absolute path of initial PDB file >> + // >> + GetPdbFileName (ImageBase, &PdbAbsoluteFilePath, &PdbFileName); >> + >> + // >> + // Print out initial image module name (if any) >> + // >> + if (PdbAbsoluteFilePath != NULL) { >> + InternalPrintMessage ( >> + "\n%a (ImageBase=0x%016lx, EntryPoint=0x%016lx):\n", >> + PdbFileName, >> + ImageBase, >> + (UINTN)EntryPoint >> + ); >> + InternalPrintMessage ("%a\n", PdbAbsoluteFilePath); >> + } >> + >> + // >> + // Walk through call stack and find next module names >> + // >> + for (;;) { >> + // >> + // Set RIP with return address from current stack frame >> + // >> + Rip = *(UINT64 *)((UINTN)Rbp + 8); >> + >> + // >> + // If RIP is zero, then stop unwinding the stack >> + // >> + if (Rip == 0) { >> + break; >> + } >> + >> + // >> + // Check if RIP is within another PE/COFF image base address >> + // >> + if (Rip < ImageBase) { >> + // >> + // Search for the respective PE/COFF image based on RIP >> + // >> + ImageBase = PeCoffSearchImageBase (Rip); >> + if (ImageBase == 0) { >> + // >> + // Stop stack trace >> + // >> + break; >> + } >> + >> + // >> + // Get PE/COFF image's entry point >> + // >> + Status = PeCoffLoaderGetEntryPoint ((VOID *)ImageBase, >> &EntryPoint); >> + if (EFI_ERROR (Status)) { >> + EntryPoint = NULL; >> + } >> + >> + // >> + // Get file name and absolute path of PDB file >> + // >> + GetPdbFileName (ImageBase, &PdbAbsoluteFilePath, &PdbFileName); >> + >> + // >> + // Print out image module name (if any) >> + // >> + if (PdbAbsoluteFilePath != NULL) { >> + InternalPrintMessage ( >> + "%a (ImageBase=0x%016lx, EntryPoint=0x%016lx):\n", >> + PdbFileName, >> + ImageBase, >> + (UINTN)EntryPoint >> + ); >> + InternalPrintMessage ("%a\n", PdbAbsoluteFilePath); >> + } >> + } >> + >> + // >> + // Unwind the stack >> + // >> + Rbp = *(UINT64 *)(UINTN)Rbp; >> + } >> +} >> + >> +/** >> + Dump stack trace. >> + >> + @param[in] SystemContext Pointer to EFI_SYSTEM_CONTEXT. >> + @param[out] UnwondStacksCount Count of unwond stack frames. >> +**/ >> +STATIC >> +VOID >> +DumpStackTrace ( >> + IN EFI_SYSTEM_CONTEXT SystemContext, >> + OUT INTN *UnwondStacksCount >> + ) >> +{ >> + UINT64 Rip; >> + UINT64 Rbp; >> + UINTN ImageBase; >> + CHAR8 *PdbFileName; >> + >> + // >> + // Set current RIP address >> + // >> + Rip = SystemContext.SystemContextX64->Rip; >> + >> + // >> + // Set current frame pointer address >> + // >> + Rbp = SystemContext.SystemContextX64->Rbp; >> + >> + // >> + // Get initial PE/COFF image base address from current RIP >> + // >> + ImageBase = PeCoffSearchImageBase (Rip); >> + if (ImageBase == 0) { >> + InternalPrintMessage ("!!!! Could not find backtrace information. >> !!!!"); >> + return; >> + } >> + >> + // >> + // Get PDB file name from initial PE/COFF image >> + // >> + GetPdbFileName (ImageBase, NULL, &PdbFileName); >> + >> + // >> + // Initialize count of unwond stacks >> + // >> + *UnwondStacksCount = 1; >> + >> + // >> + // Print out back trace >> + // >> + InternalPrintMessage ("\nCall trace:\n"); >> + >> + for (;;) { >> + // >> + // Print stack frame in the following format: >> + // >> + // # @ + (RBP) in [ | ????] >> + // >> + InternalPrintMessage ( >> + "%d 0x%016lx @ 0x%016lx+0x%x (0x%016lx) in %a\n", >> + *UnwondStacksCount - 1, >> + Rip, >> + ImageBase, >> + Rip - ImageBase - 1, >> + Rbp, >> + PdbFileName >> + ); >> + >> + // >> + // Set RIP with return address from current stack frame >> + // >> + Rip = *(UINT64 *)((UINTN)Rbp + 8); >> + >> + // >> + // If RIP is zero, then stop unwinding the stack >> + // >> + if (Rip == 0) { >> + break; >> + } >> + >> + // >> + // Check if RIP is within another PE/COFF image base address >> + // >> + if (Rip < ImageBase) { >> + // >> + // Search for the respective PE/COFF image based on RIP >> + // >> + ImageBase = PeCoffSearchImageBase (Rip); >> + if (ImageBase == 0) { >> + // >> + // Stop stack trace >> + // >> + break; >> + } >> + >> + // >> + // Get PDB file name >> + // >> + GetPdbFileName (ImageBase, NULL, &PdbFileName); >> + } >> + >> + // >> + // Unwind the stack >> + // >> + Rbp = *(UINT64 *)(UINTN)Rbp; >> + >> + // >> + // Increment count of unwond stacks >> + // >> + (*UnwondStacksCount)++; >> + } >> +} >> + >> /** >> Display CPU information. >> >> @@ -254,9 +603,25 @@ DumpImageAndCpuContent ( >> IN EFI_SYSTEM_CONTEXT SystemContext >> ) >> { >> + INTN UnwondStacksCount; >> + >> + // >> + // Dump CPU context >> + // >> DumpCpuContext (ExceptionType, SystemContext); >> + >> + // >> + // Dump stack trace >> + // >> + DumpStackTrace (SystemContext, &UnwondStacksCount); >> + >> + // >> + // Dump image module names >> + // >> + DumpImageModuleNames (SystemContext); >> + >> // >> - // Dump module image base and module entry point by RIP >> + // Dump stack contents >> // >> - DumpModuleImageInfo (SystemContext.SystemContextX64->Rip); >> + DumpStackContents (SystemContext.SystemContextX64->Rsp, >> UnwondStacksCount); >> } >> -- >> 2.14.3 >> >> _______________________________________________ >> edk2-devel mailing list >> edk2-devel@lists.01.org >> https://lists.01.org/mailman/listinfo/edk2-devel > > -- Paulo Alcantara, HP Inc. Speaking for myself only.