[RFC v5 5/8] UefiCpuPkg/CpuExceptionHandlerLib: Ensure valid frame/stack pointers

[Paulo Alcantara <paulo@paulo.ac>]

Validate all possible memory dereferences during stack traces in IA32
and X64 CPU exceptions.

Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Requested-by: Brian Johnson <brian.johnson@hpe.com>
Requested-by: Jiewen Yao <jiewen.yao@intel.com>
Signed-off-by: Paulo Alcantara <paulo@paulo.ac>
---
 UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c | 149 +++++++++++++++++++-
 UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c  |  75 +++++++++-
 2 files changed, 216 insertions(+), 8 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
index c5d6ea0939..3b92512b92 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
@@ -14,6 +14,11 @@
 
 #include "CpuExceptionCommon.h"
 
+//
+// IA32 Segment Selector bit definitions
+//
+#define IA32_SEGSEL_TI BIT2
+
 /**
   Return address map of exception handler template so that C code can generate
   exception tables.
@@ -398,6 +403,97 @@ DumpCpuContext (
     );
 }
 
+/**
+  Check if a logical address is valid.
+
+  @param[in]  SystemContext      Pointer to EFI_SYSTEM_CONTEXT.
+  @param[in]  SegmentSelector    Segment selector.
+  @param[in]  Offset             Offset or logical address.
+**/
+STATIC
+BOOLEAN
+IsLogicalAddressValid (
+  IN  EFI_SYSTEM_CONTEXT   SystemContext,
+  IN  UINT16               SegmentSelector,
+  IN  UINTN                Offset
+  )
+{
+  IA32_SEGMENT_DESCRIPTOR  *SegmentDescriptor;
+  UINT32                   SegDescBase;
+  UINT32                   SegDescLimit;
+  UINT64                   SegDescLimitInBytes;
+
+  //
+  // Check for valid input parameters
+  //
+  if (SegmentSelector == 0 || Offset == 0) {
+    return FALSE;
+  }
+
+  //
+  // Look for a segment descriptor in a GDT or LDT table depending on TI
+  // (Table Indicator) bit in segment selector.
+  //
+  if ((SegmentSelector & IA32_SEGSEL_TI) == 0) {
+    //
+    // Get segment descriptor from GDT table
+    //
+    SegmentDescriptor =
+      (IA32_SEGMENT_DESCRIPTOR *)(
+        (UINTN)SystemContext.SystemContextIa32->Gdtr[0] +
+        (SegmentSelector & ~7)
+        );
+  } else {
+    //
+    // Get segment descriptor from LDT table
+    //
+    SegmentDescriptor =
+      (IA32_SEGMENT_DESCRIPTOR *)(
+        (UINTN)SystemContext.SystemContextIa32->Ldtr +
+        (SegmentSelector & ~7)
+        );
+  }
+
+  //
+  // Get segment descriptor's base address
+  //
+  SegDescBase = SegmentDescriptor->Bits.BaseLow |
+    (SegmentDescriptor->Bits.BaseMid << 16) |
+    (SegmentDescriptor->Bits.BaseHigh << 24);
+
+  //
+  // Get segment descriptor's limit
+  //
+  SegDescLimit = SegmentDescriptor->Bits.LimitLow |
+    (SegmentDescriptor->Bits.LimitHigh << 16);
+
+  //
+  // Calculate segment descriptor's limit in bytes
+  //
+  if (SegmentDescriptor->Bits.G == 1) {
+    SegDescLimitInBytes = (UINT64)SegDescLimit * SIZE_4KB + (SIZE_4KB - 1);
+  } else {
+    SegDescLimitInBytes = SegDescLimit;
+  }
+
+  //
+  // Make sure to not access beyond a segment limit boundary
+  //
+  if ((UINT64)Offset + SegDescBase > SegDescLimitInBytes) {
+    return FALSE;
+  }
+
+  //
+  // Check if the translated logical address (or linear address) is valid
+  //
+  return IsLinearAddressValid (
+    SystemContext.SystemContextIa32->Cr0,
+    SystemContext.SystemContextIa32->Cr3,
+    SystemContext.SystemContextIa32->Cr4,
+    Offset + SegDescBase
+    );
+}
+
 /**
   Dump stack trace.
 
@@ -470,6 +566,20 @@ DumpStacktrace (
   InternalPrintMessage ("\nCall trace:\n");
 
   for (;;) {
+    //
+    // Check for valid frame pointer
+    //
+    if (!IsLogicalAddressValid (SystemContext,
+                                SystemContext.SystemContextIa32->Ss,
+                                (UINTN)Ebp + 4) ||
+        !IsLogicalAddressValid (SystemContext,
+                                SystemContext.SystemContextIa32->Ss,
+                                (UINTN)Ebp)) {
+      InternalPrintMessage ("%a: attempted to dereference an invalid frame "
+                            "pointer at 0x%08x\n", __FUNCTION__, Ebp);
+      break;
+    }
+
     //
     // Print stack frame in the following format:
     //
@@ -610,6 +720,16 @@ DumpImageModuleNames (
   // Walk through call stack and find next module names
   //
   for (;;) {
+    if (!IsLogicalAddressValid (SystemContext,
+                                SystemContext.SystemContextIa32->Ss,
+                                (UINTN)Ebp) ||
+        !IsLogicalAddressValid (SystemContext,
+                                SystemContext.SystemContextIa32->Ss,
+                                (UINTN)Ebp + 4)) {
+      InternalPrintMessage ("%a: attempted to dereference an invalid frame "
+                            "pointer at 0x%08x\n", __FUNCTION__, Ebp);
+    }
+
     //
     // Set EIP with return address from current stack frame
     //
@@ -673,16 +793,23 @@ DumpImageModuleNames (
 /**
   Dump stack contents.
 
-  @param[in]  CurrentEsp         Current stack pointer address.
+  @param[in]  SystemContext       Pointer to EFI_SYSTEM_CONTEXT.
   @param[in]  UnwoundStacksCount  Count of unwound stack frames.
 **/
 STATIC
 VOID
 DumpStackContents (
-  IN UINT32  CurrentEsp,
-  IN INTN    UnwoundStacksCount
+  IN  EFI_SYSTEM_CONTEXT  SystemContext,
+  IN  INTN                UnwoundStacksCount
   )
 {
+  UINT32 CurrentEsp;
+
+  //
+  // Get current stack pointer
+  //
+  CurrentEsp = SystemContext.SystemContextIa32->Esp;
+
   //
   // Check for proper stack alignment
   //
@@ -696,6 +823,20 @@ DumpStackContents (
   //
   InternalPrintMessage ("\nStack dump:\n");
   while (UnwoundStacksCount-- > 0) {
+    //
+    // Check for a valid stack pointer address
+    //
+    if (!IsLogicalAddressValid (SystemContext,
+                                SystemContext.SystemContextIa32->Ss,
+                                (UINTN)CurrentEsp) ||
+        !IsLogicalAddressValid (SystemContext,
+                                SystemContext.SystemContextIa32->Ss,
+                                (UINTN)CurrentEsp + 4)) {
+      InternalPrintMessage ("%a: attempted to dereference an invalid stack "
+                            "pointer at 0x%08x\n", __FUNCTION__, CurrentEsp);
+      break;
+    }
+
     InternalPrintMessage (
       "0x%08x: %08x %08x\n",
       CurrentEsp,
@@ -742,5 +883,5 @@ DumpImageAndCpuContent (
   //
   // Dump stack contents
   //
-  DumpStackContents (SystemContext.SystemContextIa32->Esp, UnwoundStacksCount);
+  DumpStackContents (SystemContext, UnwoundStacksCount);
 }
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
index 523dce95c9..c81f4c00eb 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c
@@ -401,16 +401,26 @@ DumpCpuContext (
 /**
   Dump stack contents.
 
-  @param[in]  CurrentRsp         Current stack pointer address.
+  @param[in]  SystemContext       Pointer to EFI_SYSTEM_CONTEXT.
   @param[in]  UnwoundStacksCount  Count of unwound stack frames.
 **/
 STATIC
 VOID
 DumpStackContents (
-  IN UINT64  CurrentRsp,
-  IN INTN    UnwoundStacksCount
+  IN  EFI_SYSTEM_CONTEXT  SystemContext,
+  IN  INTN                UnwoundStacksCount
   )
 {
+  UINT64  CurrentRsp;
+  UINTN   Cr0;
+  UINTN   Cr3;
+  UINTN   Cr4;
+
+  //
+  // Get current stack pointer
+  //
+  CurrentRsp = SystemContext.SystemContextX64->Rsp;
+
   //
   // Check for proper stack pointer alignment
   //
@@ -419,11 +429,28 @@ DumpStackContents (
     return;
   }
 
+  //
+  // Get system control registers
+  //
+  Cr0 = SystemContext.SystemContextX64->Cr0;
+  Cr3 = SystemContext.SystemContextX64->Cr3;
+  Cr4 = SystemContext.SystemContextX64->Cr4;
+
   //
   // Dump out stack contents
   //
   InternalPrintMessage ("\nStack dump:\n");
   while (UnwoundStacksCount-- > 0) {
+    //
+    // Check for a valid stack pointer address
+    //
+    if (!IsLinearAddressValid (Cr0, Cr3, Cr4, (UINTN)CurrentRsp) ||
+        !IsLinearAddressValid (Cr0, Cr3, Cr4, (UINTN)CurrentRsp + 8)) {
+      InternalPrintMessage ("%a: attempted to dereference an invalid stack "
+                            "pointer at 0x%016lx\n", __FUNCTION__, CurrentRsp);
+      break;
+    }
+
     InternalPrintMessage (
       "0x%016lx: %016lx %016lx\n",
       CurrentRsp,
@@ -459,6 +486,9 @@ DumpImageModuleNames (
   CHAR8       *PdbFileName;
   UINT64      Rbp;
   UINTN       LastImageBase;
+  UINTN       Cr0;
+  UINTN       Cr3;
+  UINTN       Cr4;
 
   //
   // Set current RIP address
@@ -527,10 +557,27 @@ DumpImageModuleNames (
     InternalPrintMessage ("%a\n", PdbAbsoluteFilePath);
   }
 
+  //
+  // Get system control registers
+  //
+  Cr0 = SystemContext.SystemContextX64->Cr0;
+  Cr3 = SystemContext.SystemContextX64->Cr3;
+  Cr4 = SystemContext.SystemContextX64->Cr4;
+
   //
   // Walk through call stack and find next module names
   //
   for (;;) {
+    //
+    // Check for a valid frame pointer
+    //
+    if (!IsLinearAddressValid (Cr0, Cr3, Cr4, (UINTN)Rbp + 8) ||
+        !IsLinearAddressValid (Cr0, Cr3, Cr4, (UINTN)Rbp)) {
+      InternalPrintMessage ("%a: attempted to dereference an invalid frame "
+                            "pointer at 0x%016lx\n", __FUNCTION__, Rbp);
+      break;
+    }
+
     //
     // Set RIP with return address from current stack frame
     //
@@ -617,6 +664,9 @@ DumpStacktrace (
   UINT64  Rbp;
   UINTN   ImageBase;
   CHAR8   *PdbFileName;
+  UINTN   Cr0;
+  UINTN   Cr3;
+  UINTN   Cr4;
 
   //
   // Set current RIP address
@@ -656,12 +706,29 @@ DumpStacktrace (
   //
   *UnwoundStacksCount = 1;
 
+  //
+  // Get system control registers
+  //
+  Cr0 = SystemContext.SystemContextX64->Cr0;
+  Cr3 = SystemContext.SystemContextX64->Cr3;
+  Cr4 = SystemContext.SystemContextX64->Cr4;
+
   //
   // Print out back trace
   //
   InternalPrintMessage ("\nCall trace:\n");
 
   for (;;) {
+    //
+    // Check for valid frame pointer
+    //
+    if (!IsLinearAddressValid (Cr0, Cr3, Cr4, (UINTN)Rbp + 8) ||
+        !IsLinearAddressValid (Cr0, Cr3, Cr4, (UINTN)Rbp)) {
+      InternalPrintMessage ("%a: attempted to dereference an invalid frame "
+                            "pointer at 0x%016lx\n", __FUNCTION__, Rbp);
+      break;
+    }
+
     //
     // Print stack frame in the following format:
     //
@@ -749,5 +816,5 @@ DumpImageAndCpuContent (
   //
   // Dump stack contents
   //
-  DumpStackContents (SystemContext.SystemContextX64->Rsp, UnwoundStacksCount);
+  DumpStackContents (SystemContext, UnwoundStacksCount);
 }
-- 
2.14.3