UefiCpuPkg PiSmmCpuDxeSmm: Fixed #double fault on #page fault for IA32

When StackGuard is enabled on IA32, the #double fault exception
is reported instead of #page fault.

This issue does not exist on X64, or IA32 without StackGuard.

The fix at e4435f710cea2d2f10cd7343d545920867780086 was incomplete.

It is because AllocateCodePages() is used to allocate buffer for
GDT and TSS, the code pages will be set to RO in SetMemMapAttributes().
But IA32 Stack Guard need use task switch to switch stack that need
write GDT and TSS, so AllocateCodePages() could not be used.

This patch uses AllocatePages() instead of AllocateCodePages() to
allocate buffer for GDT and TSS if StackGuard is enabled on IA32.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
index 3c68c970245fba7369dde77e2adefac026f58b09..4c1499939b1b4d0eeb76ca620fadbc12d1b486f6 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
@@ -1,7 +1,7 @@
 /** @file\r
   SMM CPU misc functions for Ia32 arch specific.\r
   \r
 /** @file\r
   SMM CPU misc functions for Ia32 arch specific.\r
   \r

-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>\r

 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r


@@ -77,7 +77,12 @@ InitGdt (
 \r
     GdtTssTableSize = (gcSmiGdtr.Limit + 1 + TSS_SIZE * 2 + 7) & ~7; // 8 bytes aligned\r
     mGdtBufferSize = GdtTssTableSize * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus;\r
 \r
     GdtTssTableSize = (gcSmiGdtr.Limit + 1 + TSS_SIZE * 2 + 7) & ~7; // 8 bytes aligned\r
     mGdtBufferSize = GdtTssTableSize * gSmmCpuPrivate->SmmCoreEntryContext.NumberOfCpus;\r

-    GdtTssTables = (UINT8*)AllocateCodePages (EFI_SIZE_TO_PAGES (mGdtBufferSize));\r
+    //\r
+    // IA32 Stack Guard need use task switch to switch stack that need\r
+    // write GDT and TSS, so AllocateCodePages() could not be used here\r
+    // as code pages will be set to RO. \r
+    //\r
+    GdtTssTables = (UINT8*)AllocatePages (EFI_SIZE_TO_PAGES (mGdtBufferSize));\r

     ASSERT (GdtTssTables != NULL);\r
     mGdtBuffer = (UINTN)GdtTssTables;\r
     GdtTableStepSize = GdtTssTableSize;\r
     ASSERT (GdtTssTables != NULL);\r
     mGdtBuffer = (UINTN)GdtTssTables;\r
     GdtTableStepSize = GdtTssTableSize;\r


@@ -126,61 +131,6 @@ InitGdt (
   return GdtTssTables;\r
 }\r
 \r
   return GdtTssTables;\r
 }\r
 \r

-/**\r
-  This function sets GDT/IDT buffer to be RO and XP.\r
-**/\r
-VOID\r
-PatchGdtIdtMap (\r
-  VOID\r
-  )\r
-{\r
-  EFI_PHYSICAL_ADDRESS       BaseAddress;\r
-  UINTN                      Size;\r
-\r
-  //\r
-  // GDT\r
-  //\r
-  DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - GDT:\n"));\r
-\r
-  BaseAddress = mGdtBuffer;\r
-  Size = ALIGN_VALUE(mGdtBufferSize, SIZE_4KB);\r
-  if (!FeaturePcdGet (PcdCpuSmmStackGuard)) {\r
-    //\r
-    // Do not set RO for IA32 when stack guard feature is enabled.\r
-    // Stack Guard need use task switch to switch stack.\r
-    // It need write GDT and TSS.\r
-    //\r
-    SmmSetMemoryAttributes (\r
-      BaseAddress,\r
-      Size,\r
-      EFI_MEMORY_RO\r
-      );\r
-  }\r
-  SmmSetMemoryAttributes (\r
-    BaseAddress,\r
-    Size,\r
-    EFI_MEMORY_XP\r
-    );\r
-\r
-  //\r
-  // IDT\r
-  //\r
-  DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - IDT:\n"));\r
-\r
-  BaseAddress = gcSmiIdtr.Base;\r
-  Size = ALIGN_VALUE(gcSmiIdtr.Limit + 1, SIZE_4KB);\r
-  SmmSetMemoryAttributes (\r
-    BaseAddress,\r
-    Size,\r
-    EFI_MEMORY_RO\r
-    );\r
-  SmmSetMemoryAttributes (\r
-    BaseAddress,\r
-    Size,\r
-    EFI_MEMORY_XP\r
-    );\r
-}\r
-\r

 /**\r
   Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.\r
 \r
 /**\r
   Transfer AP to safe hlt-loop after it finished restore CPU features on S3 patch.\r
 \r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index ef32f17676658ecae614bf6c03743e69626c0336..cbaa513244d5a78d7a3b6b87c7115ab691001c4d 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1,7 +1,7 @@
 /** @file\r
 Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.\r
 \r
 /** @file\r
 Agent Module to load other modules to deploy SMM Entry Vector for X86 CPU.\r
 \r

-Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>\r

 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>\r
 \r
 This program and the accompanying materials\r
 Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>\r
 \r
 This program and the accompanying materials\r


@@ -509,14 +509,6 @@ InitGdt (
   OUT UINTN  *GdtStepSize\r
   );\r
 \r
   OUT UINTN  *GdtStepSize\r
   );\r
 \r

-/**\r
-  This function sets GDT/IDT buffer to be RO and XP.\r
-**/\r
-VOID\r
-PatchGdtIdtMap (\r
-  VOID\r
-  );\r
-\r

 /**\r
 \r
   Register the SMM Foundation entry point.\r
 /**\r
 \r
   Register the SMM Foundation entry point.\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 2d7dba59bf30370241c06f22bb4e690a9533846d..16664f304cde4b8fd88f3df3303bf3bdd6971b93 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1,6 +1,6 @@
 /** @file\r
 \r
 /** @file\r
 \r

-Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>\r

 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r


@@ -768,6 +768,53 @@ PatchSmmSaveStateMap (
     );\r
 }\r
 \r
     );\r
 }\r
 \r

+/**\r
+  This function sets GDT/IDT buffer to be RO and XP.\r
+**/\r
+VOID\r
+PatchGdtIdtMap (\r
+  VOID\r
+  )\r
+{\r
+  EFI_PHYSICAL_ADDRESS       BaseAddress;\r
+  UINTN                      Size;\r
+\r
+  //\r
+  // GDT\r
+  //\r
+  DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - GDT:\n"));\r
+\r
+  BaseAddress = mGdtBuffer;\r
+  Size = ALIGN_VALUE(mGdtBufferSize, SIZE_4KB);\r
+  //\r
+  // The range should have been set to RO\r
+  // if it is allocated with EfiRuntimeServicesCode.\r
+  //\r
+  SmmSetMemoryAttributes (\r
+    BaseAddress,\r
+    Size,\r
+    EFI_MEMORY_XP\r
+    );\r
+\r
+  //\r
+  // IDT\r
+  //\r
+  DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - IDT:\n"));\r
+\r
+  BaseAddress = gcSmiIdtr.Base;\r
+  Size = ALIGN_VALUE(gcSmiIdtr.Limit + 1, SIZE_4KB);\r
+  SmmSetMemoryAttributes (\r
+    BaseAddress,\r
+    Size,\r
+    EFI_MEMORY_RO\r
+    );\r
+  SmmSetMemoryAttributes (\r
+    BaseAddress,\r
+    Size,\r
+    EFI_MEMORY_XP\r
+    );\r
+}\r
+\r

 /**\r
   This function sets memory attribute according to MemoryAttributesTable.\r
 **/\r
 /**\r
   This function sets memory attribute according to MemoryAttributesTable.\r
 **/\r

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
index 9d26e44a9acd58c1f279694d0afa3eac2498a544..6a5d453242ff43b362451aaa2a94a51066bc225e 100644 (file)
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
@@ -1,7 +1,7 @@
 /** @file\r
   SMM CPU misc functions for x64 arch specific.\r
   \r
 /** @file\r
   SMM CPU misc functions for x64 arch specific.\r
   \r

-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>\r
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>\r

 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r
 This program and the accompanying materials\r
 are licensed and made available under the terms and conditions of the BSD License\r
 which accompanies this distribution.  The full text of the license may be found at\r


@@ -95,54 +95,6 @@ InitGdt (
   return GdtTssTables;\r
 }\r
 \r
   return GdtTssTables;\r
 }\r
 \r

-/**\r
-  This function sets GDT/IDT buffer to be RO and XP.\r
-**/\r
-VOID\r
-PatchGdtIdtMap (\r
-  VOID\r
-  )\r
-{\r
-  EFI_PHYSICAL_ADDRESS       BaseAddress;\r
-  UINTN                      Size;\r
-\r
-  //\r
-  // GDT\r
-  //\r
-  DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - GDT:\n"));\r
-\r
-  BaseAddress = mGdtBuffer;\r
-  Size = ALIGN_VALUE(mGdtBufferSize, SIZE_4KB);\r
-  SmmSetMemoryAttributes (\r
-    BaseAddress,\r
-    Size,\r
-    EFI_MEMORY_RO\r
-    );\r
-  SmmSetMemoryAttributes (\r
-    BaseAddress,\r
-    Size,\r
-    EFI_MEMORY_XP\r
-    );\r
-\r
-  //\r
-  // IDT\r
-  //\r
-  DEBUG ((DEBUG_INFO, "PatchGdtIdtMap - IDT:\n"));\r
-\r
-  BaseAddress = gcSmiIdtr.Base;\r
-  Size = ALIGN_VALUE(gcSmiIdtr.Limit + 1, SIZE_4KB);\r
-  SmmSetMemoryAttributes (\r
-    BaseAddress,\r
-    Size,\r
-    EFI_MEMORY_RO\r
-    );\r
-  SmmSetMemoryAttributes (\r
-    BaseAddress,\r
-    Size,\r
-    EFI_MEMORY_XP\r
-    );\r
-}\r
-\r

 /**\r
   Get Protected mode code segment from current GDT table.\r
 \r
 /**\r
   Get Protected mode code segment from current GDT table.\r
 \r