Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CP Exception when CET enable #4867

Merged
merged 1 commit into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 47 additions & 12 deletions UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
Original file line number Diff line number Diff line change
Expand Up @@ -1553,27 +1553,62 @@ SmmWaitForApArrival (
);

/**
Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1.
Write unprotect read-only pages if Cr0.Bits.WP is 1.

@param[out] WriteProtect If Cr0.Bits.WP is enabled.

@param[out] WpEnabled If Cr0.WP is enabled.
@param[out] CetEnabled If CET is enabled.
**/
VOID
DisableReadOnlyPageWriteProtect (
OUT BOOLEAN *WpEnabled,
OUT BOOLEAN *CetEnabled
SmmWriteUnprotectReadOnlyPage (
OUT BOOLEAN *WriteProtect
);

/**
Enable Write Protect on pages marked as read-only.
Write protect read-only pages.

@param[in] WriteProtect If Cr0.Bits.WP should be enabled.

@param[out] WpEnabled If Cr0.WP should be enabled.
@param[out] CetEnabled If CET should be enabled.
**/
VOID
EnableReadOnlyPageWriteProtect (
BOOLEAN WpEnabled,
BOOLEAN CetEnabled
SmmWriteProtectReadOnlyPage (
IN BOOLEAN WriteProtect
);

///
/// Define macros to encapsulate the write unprotect/protect
/// read-only pages.
/// Below pieces of logic are defined as macros and not functions
/// because "CET" feature disable & enable must be in the same
/// function to avoid shadow stack and normal SMI stack mismatch,
/// thus WRITE_UNPROTECT_RO_PAGES () must be called pair with
/// WRITE_PROTECT_RO_PAGES () in same function.
///
/// @param[in,out] Wp A BOOLEAN variable local to the containing
/// function, carrying write protection status from
/// WRITE_UNPROTECT_RO_PAGES() to
/// WRITE_PROTECT_RO_PAGES().
///
/// @param[in,out] Cet A BOOLEAN variable local to the containing
/// function, carrying control flow integrity
/// enforcement status from
/// WRITE_UNPROTECT_RO_PAGES() to
/// WRITE_PROTECT_RO_PAGES().
///
#define WRITE_UNPROTECT_RO_PAGES(Wp, Cet) \
do { \
Cet = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0); \
if (Cet) { \
DisableCet (); \
} \
SmmWriteUnprotectReadOnlyPage (&Wp); \
} while (FALSE)

#define WRITE_PROTECT_RO_PAGES(Wp, Cet) \
do { \
SmmWriteProtectReadOnlyPage (Wp); \
if (Cet) { \
EnableCet (); \
} \
} while (FALSE)

#endif
73 changes: 30 additions & 43 deletions UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,60 +41,43 @@ PAGE_TABLE_POOL *mPageTablePool = NULL;
BOOLEAN mIsReadOnlyPageTable = FALSE;

/**
Disable Write Protect on pages marked as read-only if Cr0.Bits.WP is 1.
Write unprotect read-only pages if Cr0.Bits.WP is 1.

@param[out] WriteProtect If Cr0.Bits.WP is enabled.

@param[out] WpEnabled If Cr0.WP is enabled.
@param[out] CetEnabled If CET is enabled.
**/
VOID
DisableReadOnlyPageWriteProtect (
OUT BOOLEAN *WpEnabled,
OUT BOOLEAN *CetEnabled
SmmWriteUnprotectReadOnlyPage (
OUT BOOLEAN *WriteProtect
)
{
IA32_CR0 Cr0;

*CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 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.UintN = AsmReadCr0 ();
*WriteProtect = (Cr0.Bits.WP != 0);
if (*WriteProtect) {
Cr0.Bits.WP = 0;
AsmWriteCr0 (Cr0.UintN);
}
}

/**
Enable Write Protect on pages marked as read-only.
Write protect read-only pages.

@param[in] WriteProtect If Cr0.Bits.WP should be enabled.

@param[out] WpEnabled If Cr0.WP should be enabled.
@param[out] CetEnabled If CET should be enabled.
**/
VOID
EnableReadOnlyPageWriteProtect (
BOOLEAN WpEnabled,
BOOLEAN CetEnabled
SmmWriteProtectReadOnlyPage (
IN BOOLEAN WriteProtect
)
{
IA32_CR0 Cr0;

if (WpEnabled) {
if (WriteProtect) {
Cr0.UintN = AsmReadCr0 ();
Cr0.Bits.WP = 1;
AsmWriteCr0 (Cr0.UintN);

if (CetEnabled) {
//
// re-enable CET.
//
EnableCet ();
}
}
}

Expand All @@ -121,7 +104,7 @@ InitializePageTablePool (
)
{
VOID *Buffer;
BOOLEAN WpEnabled;
BOOLEAN WriteProtect;
BOOLEAN CetEnabled;

//
Expand Down Expand Up @@ -159,9 +142,11 @@ InitializePageTablePool (
// If page table memory has been marked as RO, mark the new pool pages as read-only.
//
if (mIsReadOnlyPageTable) {
DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
WRITE_UNPROTECT_RO_PAGES (WriteProtect, CetEnabled);

SmmSetMemoryAttributes ((EFI_PHYSICAL_ADDRESS)(UINTN)Buffer, EFI_PAGES_TO_SIZE (PoolPages), EFI_MEMORY_RO);
EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);

WRITE_PROTECT_RO_PAGES (WriteProtect, CetEnabled);
}

return TRUE;
Expand Down Expand Up @@ -1011,7 +996,7 @@ SetMemMapAttributes (
IA32_MAP_ENTRY *Map;
UINTN Count;
UINT64 MemoryAttribute;
BOOLEAN WpEnabled;
BOOLEAN WriteProtect;
BOOLEAN CetEnabled;

SmmGetSystemConfigurationTable (&gEdkiiPiSmmMemoryAttributesTableGuid, (VOID **)&MemoryAttributesTable);
Expand Down Expand Up @@ -1057,7 +1042,7 @@ SetMemMapAttributes (

ASSERT_RETURN_ERROR (Status);

DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
WRITE_UNPROTECT_RO_PAGES (WriteProtect, CetEnabled);

MemoryMap = MemoryMapStart;
for (Index = 0; Index < MemoryMapEntryCount; Index++) {
Expand Down Expand Up @@ -1087,7 +1072,8 @@ SetMemMapAttributes (
MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap, DescriptorSize);
}

EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
WRITE_PROTECT_RO_PAGES (WriteProtect, CetEnabled);

FreePool (Map);

PatchSmmSaveStateMap ();
Expand Down Expand Up @@ -1394,14 +1380,14 @@ SetUefiMemMapAttributes (
UINTN MemoryMapEntryCount;
UINTN Index;
EFI_MEMORY_DESCRIPTOR *Entry;
BOOLEAN WpEnabled;
BOOLEAN WriteProtect;
BOOLEAN CetEnabled;

PERF_FUNCTION_BEGIN ();

DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));

DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
WRITE_UNPROTECT_RO_PAGES (WriteProtect, CetEnabled);

if (mUefiMemoryMap != NULL) {
MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;
Expand Down Expand Up @@ -1481,7 +1467,7 @@ SetUefiMemMapAttributes (
}
}

EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
WRITE_PROTECT_RO_PAGES (WriteProtect, CetEnabled);

//
// Do not free mUefiMemoryAttributesTable, it will be checked in IsSmmCommBufferForbiddenAddress().
Expand Down Expand Up @@ -1872,7 +1858,7 @@ SetPageTableAttributes (
VOID
)
{
BOOLEAN WpEnabled;
BOOLEAN WriteProtect;
BOOLEAN CetEnabled;

if (!IfReadOnlyPageTableNeeded ()) {
Expand All @@ -1886,7 +1872,7 @@ SetPageTableAttributes (
// 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*.
//
DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
WRITE_UNPROTECT_RO_PAGES (WriteProtect, CetEnabled);

// Set memory used by page table as Read Only.
DEBUG ((DEBUG_INFO, "Start...\n"));
Expand All @@ -1895,7 +1881,8 @@ SetPageTableAttributes (
//
// Enable write protection, after page table attribute updated.
//
EnableReadOnlyPageWriteProtect (TRUE, CetEnabled);
WRITE_PROTECT_RO_PAGES (TRUE, CetEnabled);

mIsReadOnlyPageTable = TRUE;

//
Expand Down
7 changes: 4 additions & 3 deletions UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ InitPaging (
UINT64 Limit;
UINT64 PreviousAddress;
UINT64 MemoryAttrMask;
BOOLEAN WpEnabled;
BOOLEAN WriteProtect;
BOOLEAN CetEnabled;

PERF_FUNCTION_BEGIN ();
Expand All @@ -606,7 +606,8 @@ InitPaging (
Limit = (IsRestrictedMemoryAccess ()) ? LShiftU64 (1, mPhysicalAddressBits) : BASE_4GB;
}

DisableReadOnlyPageWriteProtect (&WpEnabled, &CetEnabled);
WRITE_UNPROTECT_RO_PAGES (WriteProtect, CetEnabled);

//
// [0, 4k] may be non-present.
//
Expand Down Expand Up @@ -672,7 +673,7 @@ InitPaging (
ASSERT_RETURN_ERROR (Status);
}

EnableReadOnlyPageWriteProtect (WpEnabled, CetEnabled);
WRITE_PROTECT_RO_PAGES (WriteProtect, CetEnabled);

//
// Flush TLB
Expand Down