Shadow stack will stop update after CET disable (DisableCet in
DisableReadOnlyPageWriteProtect), but normal smi stack will be
continue updated with the function return and enter
(DisableReadOnlyPageWriteProtect & EnableReadOnlyPageWriteProtect),
thus leading stack mismatch after CET re-enabled (EnableCet in
EnableReadOnlyPageWriteProtect).
Normal smi stack and shadow stack must be matched when CET enable,
otherwise CP Exception will happen, which is caused by a near RET
instruction (See SDM Vol 3, 6.15-Control Protection Exception).
With above requirement, CET feature enable & disable must be in the
same function to avoid stack mismatch issue.
Cc: Eric Dong <eric.d...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Zeng Star <star.z...@intel.com>
Cc: Gerd Hoffmann <kra...@redhat.com>
Cc: Rahul Kumar <rahul1.ku...@intel.com>
Signed-off-by: Jiaxin Wu <jiaxin...@intel.com>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 10 +-
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 104 ++++++++++++++-------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 19 +++-
3 files changed, 91 insertions(+), 42 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 654935dc76..daa843b057 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1554,26 +1554,24 @@ SmmWaitForApArrival (
/**
Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1.
@param[out] WpEnabled If Cr0.WP is enabled.
- @param[out] CetEnabled If CET is enabled.
+
**/
VOID
DisableReadOnlyPageWriteProtect (
- OUT BOOLEAN *WpEnabled,
- OUT BOOLEAN *CetEnabled
+ OUT BOOLEAN *WpEnabled
);
/**
Enable Write Protect on pages marked as read-only.
@param[out] WpEnabled If Cr0.WP should be enabled.
- @param[out] CetEnabled If CET should be enabled.
+
**/
VOID
EnableReadOnlyPageWriteProtect (
- BOOLEAN WpEnabled,
- BOOLEAN CetEnabled
+ BOOLEAN WpEnabled
);
#endif
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 6f49866615..2c198a161a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -42,61 +42,44 @@ BOOLEAN mIsReadOnlyPageTable = FALSE;
/**
Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1.
@param[out] WpEnabled If Cr0.WP is enabled.
- @param[out] CetEnabled If CET is enabled.
+
**/
VOID
DisableReadOnlyPageWriteProtect (
- OUT BOOLEAN *WpEnabled,
- OUT BOOLEAN *CetEnabled
+ OUT BOOLEAN *WpEnabled
)
{
IA32_CR0 Cr0;
- *CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
- Cr0.UintN = AsmReadCr0 ();
- *WpEnabled = (Cr0.Bits.WP != 0) ? TRUE : FALSE;
+ Cr0.UintN = AsmReadCr0 ();
+ *WpEnabled = (Cr0.Bits.WP != 0) ? TRUE : FALSE;
if (*WpEnabled) {
- if (*CetEnabled) {
- //
- // CET must be disabled if WP is disabled. Disable CET before clearing
CR0.WP.
- //
- DisableCet ();
- }
-
Cr0.Bits.WP = 0;
AsmWriteCr0 (Cr0.UintN);
}
}
/**
Enable Write Protect on pages marked as read-only.
@param[out] WpEnabled If Cr0.WP should be enabled.
- @param[out] CetEnabled If CET should be enabled.
+
**/
VOID
EnableReadOnlyPageWriteProtect (
- BOOLEAN WpEnabled,
- BOOLEAN CetEnabled
+ BOOLEAN WpEnabled
)
{
IA32_CR0 Cr0;
if (WpEnabled) {
Cr0.UintN = AsmReadCr0 ();
Cr0.Bits.WP = 1;
AsmWriteCr0 (Cr0.UintN);
-
- if (CetEnabled) {
- //
- // re-enable CET.
- //
- EnableCet ();
- }
}
}
/**
Initialize a buffer pool for page table use only.
@@ -157,13 +140,29 @@ InitializePageTablePool (
//
// If page table memory has been marked as RO, mark the new pool pages as
read-only.
//
if (mIsReadOnlyPageTable) {
- DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+ //
+ // CET must be disabled if WP is disabled.
+ //
+ CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
+ if (CetEnabled) {
+ DisableCet ();
+ }
+
+ DisableReadOnlyPageWriteProtect (&WpEnabled);
+
SmmSetMemoryAttributes ((EFI_PHYSICAL_ADDRESS)(UINTN)Buffer,
EFI_PAGES_TO_SIZE (PoolPages), EFI_MEMORY_RO);
- EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+
+ //
+ // Enable the WP and restore CET to enable
+ //
+ EnableReadOnlyPageWriteProtect (WpEnabled);
+ if (CetEnabled) {
+ EnableCet ();
+ }
}
return TRUE;
}
@@ -1055,11 +1054,19 @@ SetMemMapAttributes (
Status = PageTableParse (PageTable, mPagingMode, Map, &Count);
}
ASSERT_RETURN_ERROR (Status);
- DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+ //
+ // CET must be disabled if WP is disabled.
+ //
+ CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
+ if (CetEnabled) {
+ DisableCet ();
+ }
+
+ DisableReadOnlyPageWriteProtect (&WpEnabled);
MemoryMap = MemoryMapStart;
for (Index = 0; Index < MemoryMapEntryCount; Index++) {
DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n",
MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
if (MemoryMap->Type == EfiRuntimeServicesCode) {
@@ -1085,11 +1092,18 @@ SetMemMapAttributes (
);
MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
}
- EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+ //
+ // Enable the WP and restore CET to enable
+ //
+ EnableReadOnlyPageWriteProtect (WpEnabled);
+ if (CetEnabled) {
+ EnableCet ();
+ }
+
FreePool (Map);
PatchSmmSaveStateMap ();
PatchGdtIdtMap ();
@@ -1399,11 +1413,19 @@ SetUefiMemMapAttributes (
PERF_FUNCTION_BEGIN ();
DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));
- DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+ //
+ // CET must be disabled if WP is disabled.
+ //
+ CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
+ if (CetEnabled) {
+ DisableCet ();
+ }
+
+ DisableReadOnlyPageWriteProtect (&WpEnabled);
if (mUefiMemoryMap != NULL) {
MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;
MemoryMap = mUefiMemoryMap;
for (Index = 0; Index < MemoryMapEntryCount; Index++) {
@@ -1479,11 +1501,17 @@ SetUefiMemMapAttributes (
Entry = NEXT_MEMORY_DESCRIPTOR (Entry,
mUefiMemoryAttributesTable->DescriptorSize);
}
}
- EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+ //
+ // Enable the WP and restore CET to enable
+ //
+ EnableReadOnlyPageWriteProtect (WpEnabled);
+ if (CetEnabled) {
+ EnableCet ();
+ }
//
// Do not free mUefiMemoryAttributesTable, it will be checked in
IsSmmCommBufferForbiddenAddress().
//
@@ -1881,23 +1909,31 @@ SetPageTableAttributes (
PERF_FUNCTION_BEGIN ();
DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n"));
//
- // Disable write protection, because we need mark page table to be write
protected.
- // We need *write* page table memory, to mark itself to be *read only*.
+ // CET must be disabled if WP is disabled.
//
- DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+ CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
+ if (CetEnabled) {
+ DisableCet ();
+ }
+
+ DisableReadOnlyPageWriteProtect (&WpEnabled);
// Set memory used by page table as Read Only.
DEBUG ((DEBUG_INFO, "Start...\n"));
EnablePageTableProtection ();
//
- // Enable write protection, after page table attribute updated.
+ // Enable the WP and restore CET to enable
//
- EnableReadOnlyPageWriteProtect (TRUE, CetEnabled);
+ EnableReadOnlyPageWriteProtect (WpEnabled);
+ if (CetEnabled) {
+ EnableCet ();
+ }
+
mIsReadOnlyPageTable = TRUE;
//
// Flush TLB after mark all page table pool as read only.
//
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 7ac3c66f91..71fdf5f34a 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -604,11 +604,20 @@ InitPaging (
Limit = BASE_4GB;
} else {
Limit = (IsRestrictedMemoryAccess ()) ? LShiftU64 (1,
mPhysicalAddressBits) : BASE_4GB;
}
- DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
+ //
+ // CET must be disabled if WP is disabled.
+ //
+ CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
+ if (CetEnabled) {
+ DisableCet ();
+ }
+
+ DisableReadOnlyPageWriteProtect (&WpEnabled);
+
//
// [0, 4k] may be non-present.
//
PreviousAddress = ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) !=
0) ? BASE_4KB : 0;
@@ -670,11 +679,17 @@ InitPaging (
//
Status = ConvertMemoryPageAttributes (PageTable, mPagingMode,
PreviousAddress, Limit - PreviousAddress, MemoryAttrMask, TRUE, NULL);
ASSERT_RETURN_ERROR (Status);
}
- EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
+ //
+ // Enable the WP and restore CET to enable
+ //
+ EnableReadOnlyPageWriteProtect (WpEnabled);
+ if (CetEnabled) {
+ EnableCet ();
+ }
//
// Flush TLB
//
CpuFlushTlb ();
--
2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110619): https://edk2.groups.io/g/devel/message/110619
Mute This Topic: https://groups.io/mt/102362300/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
