public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "duntan" <dun.tan@intel.com>
To: devel@edk2.groups.io
Cc: Eric Dong <eric.dong@intel.com>, Ray Ni <ray.ni@intel.com>,
	Rahul Kumar <rahul1.kumar@intel.com>,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: [Patch V3 08/11] UefiCpuPkg/PiSmmCpuDxeSmm: Clear CR0.WP before modify page table
Date: Fri, 21 Apr 2023 16:36:25 +0800	[thread overview]
Message-ID: <20230421083628.1408-9-dun.tan@intel.com> (raw)
In-Reply-To: <20230421083628.1408-1-dun.tan@intel.com>

Clear CR0.WP before modify smm page table. Currently, there is
an assumption that smm pagetable is always RW before ReadyToLock.
However, when AMD SEV is enabled, FvbServicesSmm driver calls
MemEncryptSevClearMmioPageEncMask to clear AddressEncMask bit
in smm page table for this range:
[PcdOvmfFdBaseAddress,PcdOvmfFdBaseAddress+PcdOvmfFirmwareFdSize]
If page slpit happens in this process, new memory for smm page
table is allocated. Then the newly allocated page table memory
is marked as RO in smm page table in this FvbServicesSmm driver,
which may lead to PF if smm code doesn't clear CR0.WP before
modify smm page table when ReadyToLock.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 11 +++++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c             |  5 +++++
 2 files changed, 16 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index eb3547247d..110a8f3d81 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -992,6 +992,8 @@ SetMemMapAttributes (
   IA32_MAP_ENTRY                        *Map;
   UINTN                                 Count;
   UINT64                                MemoryAttribute;
+  BOOLEAN                               WpEnabled;
+  BOOLEAN                               CetEnabled;
 
   Count     = 0;
   Map       = NULL;
@@ -1028,6 +1030,8 @@ SetMemMapAttributes (
     MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
   }
 
+  DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+
   MemoryMap = MemoryMapStart;
   for (Index = 0; Index < MemoryMapEntryCount; Index++) {
     DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n", MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
@@ -1055,6 +1059,7 @@ SetMemMapAttributes (
     MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
   }
 
+  EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
   FreePool (Map);
 
   PatchSmmSaveStateMap ();
@@ -1361,9 +1366,13 @@ SetUefiMemMapAttributes (
   UINTN                  MemoryMapEntryCount;
   UINTN                  Index;
   EFI_MEMORY_DESCRIPTOR  *Entry;
+  BOOLEAN                WpEnabled;
+  BOOLEAN                CetEnabled;
 
   DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));
 
+  DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+
   if (mUefiMemoryMap != NULL) {
     MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;
     MemoryMap           = mUefiMemoryMap;
@@ -1442,6 +1451,8 @@ SetUefiMemMapAttributes (
     }
   }
 
+  EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+
   //
   // Do not free mUefiMemoryAttributesTable, it will be checked in IsSmmCommBufferForbiddenAddress().
   //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 1b0b6673e1..5625ba0cac 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -574,6 +574,8 @@ InitPaging (
   BOOLEAN   Nx;
   IA32_CR4  Cr4;
   BOOLEAN   Enable5LevelPaging;
+  BOOLEAN   WpEnabled;
+  BOOLEAN   CetEnabled;
 
   Cr4.UintN          = AsmReadCr4 ();
   Enable5LevelPaging = (BOOLEAN)(Cr4.Bits.LA57 == 1);
@@ -620,6 +622,7 @@ InitPaging (
     NumberOfPdptEntries = 4;
   }
 
+  DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
   //
   // Go through page table and change 2MB-page into 4KB-page.
   //
@@ -800,6 +803,8 @@ InitPaging (
     } // end for PML4
   } // end for PML5
 
+  EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+
   //
   // Flush TLB
   //
-- 
2.39.1.windows.1


  parent reply	other threads:[~2023-04-21  8:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-21  8:36 [Patch V3 00/11] Use CpuPageTableLib to create and update smm page table duntan
2023-04-21  8:36 ` [Patch V3 01/11] OvmfPkg: Add CpuPageTableLib required by PiSmmCpuDxe duntan
2023-04-21  8:36 ` [Patch V3 02/11] UefiPayloadPkg: " duntan
2023-04-21  8:36 ` [Patch V3 03/11] OvmfPkg:Remove code that apply AddressEncMask to non-leaf entry duntan
2023-04-21 14:26   ` Lendacky, Thomas
2023-04-21 14:53     ` Lendacky, Thomas
2023-04-24  3:38       ` [edk2-devel] " duntan
2023-04-24  9:54     ` Gerd Hoffmann
2023-04-25  2:51       ` Ni, Ray
2023-04-26  7:58         ` Min Xu
2023-04-21  8:36 ` [Patch V3 04/11] UefiCpuPkg: Use CpuPageTableLib to convert SMM paging attribute duntan
2023-04-21  8:36 ` [Patch V3 05/11] UefiCpuPkg/PiSmmCpuDxeSmm: Avoid setting non-present range to RO/NX duntan
2023-04-21  8:36 ` [Patch V3 06/11] UefiCpuPkg: Extern mSmmShadowStackSize in PiSmmCpuDxeSmm.h duntan
2023-04-21  8:36 ` [Patch V3 07/11] UefiCpuPkg/PiSmmCpuDxeSmm: Add 2 function to disable/enable CR0.WP duntan
2023-04-21  8:36 ` duntan [this message]
2023-04-21  8:36 ` [Patch V3 09/11] UefiCpuPkg: Refinement to current smm page table generation code duntan
2023-04-21  8:36 ` [Patch V3 10/11] UefiCpuPkg: Refinement to code about updating smm page table duntan
2023-04-21  8:36 ` [Patch V3 11/11] UefiCpuPkg/PiSmmCpuDxeSmm: Remove unnecessary function duntan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230421083628.1408-9-dun.tan@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox