From c4c7fb388e7f86fa98417a706bb495fb3c3c910b Mon Sep 17 00:00:00 2001 From: Jiewen Yao Date: Thu, 4 Jan 2018 10:37:42 +0800 Subject: [PATCH] UefiCpuPkg/PiSmmCpu: Check GCD and UEFI mem attrib table. 1) UefiCpuPkg/PiSmmCpu: Check for untested memory in GCD It treats GCD untested memory as invalid SMM communication buffer. 2) UefiCpuPkg/PiSmmCpu: Check EFI_RUNTIME_RO in UEFI mem attrib table. It treats the UEFI runtime page with EFI_MEMORY_RO attribute as invalid SMM communication buffer. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jiewen Yao --- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 2 + UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 3 + UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 + .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 240 ++++++++++++++++-- 4 files changed, 218 insertions(+), 28 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c index 32ce5958c5..062b51b2e4 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c @@ -16,6 +16,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include "PiSmmCpuDxeSmm.h" +UINT8 mPhysicalAddressBits = 32; + /** Create PageTable for SMM use. diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h index b8658ba17e..9cf7c8b5b5 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h @@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include +#include #include #include @@ -44,6 +45,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include +#include #include #include #include @@ -419,6 +421,7 @@ extern SPIN_LOCK *mConfigSmmCodeAccessCheckLock; extern SPIN_LOCK *mMemoryMappedLock; extern EFI_SMRAM_DESCRIPTOR *mSmmCpuSmramRanges; extern UINTN mSmmCpuSmramRangeCount; +extern UINT8 mPhysicalAddressBits; // // Copy of the PcdPteMemoryEncryptionAddressOrMask diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf index 099792e6ce..3566430401 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf @@ -136,6 +136,7 @@ gEfiAcpi20TableGuid ## SOMETIMES_CONSUMES ## SystemTable gEfiAcpi10TableGuid ## SOMETIMES_CONSUMES ## SystemTable gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable + gEfiMemoryAttributesTableGuid ## CONSUMES ## SystemTable [FeaturePcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ## CONSUMES diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c index a535389c26..2496f93561 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c @@ -13,8 +13,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include "PiSmmCpuDxeSmm.h" -#define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \ - ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size))) +// +// attributes for reserved memory before it is promoted to system memory +// +#define EFI_MEMORY_PRESENT 0x0100000000000000ULL +#define EFI_MEMORY_INITIALIZED 0x0200000000000000ULL +#define EFI_MEMORY_TESTED 0x0400000000000000ULL #define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \ ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size))) @@ -23,6 +27,11 @@ EFI_MEMORY_DESCRIPTOR *mUefiMemoryMap; UINTN mUefiMemoryMapSize; UINTN mUefiDescriptorSize; +EFI_GCD_MEMORY_SPACE_DESCRIPTOR *mGcdMemSpace = NULL; +UINTN mGcdMemNumberOfDesc = 0; + +EFI_MEMORY_ATTRIBUTES_TABLE *mUefiMemoryAttributesTable = NULL; + PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = { {Page4K, SIZE_4KB, PAGING_4K_ADDRESS_MASK_64}, {Page2M, SIZE_2MB, PAGING_2M_ADDRESS_MASK_64}, @@ -380,6 +389,7 @@ ConvertMemoryPageAttributes ( PAGE_ATTRIBUTE SplitAttribute; RETURN_STATUS Status; BOOLEAN IsEntryModified; + EFI_PHYSICAL_ADDRESS MaximumSupportMemAddress; ASSERT (Attributes != 0); ASSERT ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) == 0); @@ -391,6 +401,17 @@ ConvertMemoryPageAttributes ( return RETURN_INVALID_PARAMETER; } + MaximumSupportMemAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)(LShiftU64 (1, mPhysicalAddressBits) - 1); + if (BaseAddress > MaximumSupportMemAddress) { + return RETURN_UNSUPPORTED; + } + if (Length > MaximumSupportMemAddress) { + return RETURN_UNSUPPORTED; + } + if ((Length != 0) && (BaseAddress > MaximumSupportMemAddress - (Length - 1))) { + return RETURN_UNSUPPORTED; + } + // DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) - %016lx, %016lx, %02lx\n", IsSet, BaseAddress, Length, Attributes)); if (IsSplitted != NULL) { @@ -964,6 +985,80 @@ MergeMemoryMapForNotPresentEntry ( return ; } +/** + This function caches the GCD memory map information. +**/ +VOID +GetGcdMemoryMap ( + VOID + ) +{ + UINTN NumberOfDescriptors; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR *MemSpaceMap; + EFI_STATUS Status; + UINTN Index; + + Status = gDS->GetMemorySpaceMap (&NumberOfDescriptors, &MemSpaceMap); + if (EFI_ERROR (Status)) { + return ; + } + + mGcdMemNumberOfDesc = 0; + for (Index = 0; Index < NumberOfDescriptors; Index++) { + if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved && + (MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) == + (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED) + ) { + mGcdMemNumberOfDesc++; + } + } + + mGcdMemSpace = AllocateZeroPool (mGcdMemNumberOfDesc * sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR)); + ASSERT (mGcdMemSpace != NULL); + if (mGcdMemSpace == NULL) { + mGcdMemNumberOfDesc = 0; + gBS->FreePool (MemSpaceMap); + return ; + } + + mGcdMemNumberOfDesc = 0; + for (Index = 0; Index < NumberOfDescriptors; Index++) { + if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved && + (MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) == + (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED) + ) { + CopyMem ( + &mGcdMemSpace[mGcdMemNumberOfDesc], + &MemSpaceMap[Index], + sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR) + ); + mGcdMemNumberOfDesc++; + } + } + + gBS->FreePool (MemSpaceMap); +} + +/** + Get UEFI MemoryAttributesTable. +**/ +VOID +GetUefiMemoryAttributesTable ( + VOID + ) +{ + EFI_STATUS Status; + EFI_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable; + UINTN MemoryAttributesTableSize; + + Status = EfiGetSystemConfigurationTable (&gEfiMemoryAttributesTableGuid, (VOID **)&MemoryAttributesTable); + if (!EFI_ERROR (Status)) { + MemoryAttributesTableSize = sizeof(EFI_MEMORY_ATTRIBUTES_TABLE) + MemoryAttributesTable->DescriptorSize * MemoryAttributesTable->NumberOfEntries; + mUefiMemoryAttributesTable = AllocateCopyPool (MemoryAttributesTableSize, MemoryAttributesTable); + ASSERT (mUefiMemoryAttributesTable != NULL); + } +} + /** This function caches the UEFI memory map information. **/ @@ -1023,6 +1118,16 @@ GetUefiMemoryMap ( ASSERT (mUefiMemoryMap != NULL); gBS->FreePool (MemoryMap); + + // + // Get additional information from GCD memory map. + // + GetGcdMemoryMap (); + + // + // Get UEFI memory attributes table. + // + GetUefiMemoryAttributesTable (); } /** @@ -1037,33 +1142,89 @@ SetUefiMemMapAttributes ( VOID ) { + EFI_STATUS Status; EFI_MEMORY_DESCRIPTOR *MemoryMap; UINTN MemoryMapEntryCount; UINTN Index; + EFI_MEMORY_DESCRIPTOR *Entry; DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n")); - if (mUefiMemoryMap == NULL) { - DEBUG ((DEBUG_INFO, "UefiMemoryMap - NULL\n")); - return ; - } - - MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize; - MemoryMap = mUefiMemoryMap; - for (Index = 0; Index < MemoryMapEntryCount; Index++) { - if (IsUefiPageNotPresent(MemoryMap)) { - DEBUG ((DEBUG_INFO, "UefiMemory protection: 0x%lx - 0x%lx\n", MemoryMap->PhysicalStart, MemoryMap->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages))); - SmmSetMemoryAttributes ( - MemoryMap->PhysicalStart, - EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages), - EFI_MEMORY_RP - ); + if (mUefiMemoryMap != NULL) { + MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize; + MemoryMap = mUefiMemoryMap; + for (Index = 0; Index < MemoryMapEntryCount; Index++) { + if (IsUefiPageNotPresent(MemoryMap)) { + Status = SmmSetMemoryAttributes ( + MemoryMap->PhysicalStart, + EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages), + EFI_MEMORY_RP + ); + DEBUG (( + DEBUG_INFO, + "UefiMemory protection: 0x%lx - 0x%lx %r\n", + MemoryMap->PhysicalStart, + MemoryMap->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages), + Status + )); + } + MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize); } - MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize); } + // + // Do not free mUefiMemoryMap, it will be checked in IsSmmCommBufferForbiddenAddress(). + // // - // Do free mUefiMemoryMap, it will be checked in IsSmmCommBufferForbiddenAddress(). + // Set untested memory as not present. + // + if (mGcdMemSpace != NULL) { + for (Index = 0; Index < mGcdMemNumberOfDesc; Index++) { + Status = SmmSetMemoryAttributes ( + mGcdMemSpace[Index].BaseAddress, + mGcdMemSpace[Index].Length, + EFI_MEMORY_RP + ); + DEBUG (( + DEBUG_INFO, + "GcdMemory protection: 0x%lx - 0x%lx %r\n", + mGcdMemSpace[Index].BaseAddress, + mGcdMemSpace[Index].BaseAddress + mGcdMemSpace[Index].Length, + Status + )); + } + } + // + // Do not free mGcdMemSpace, it will be checked in IsSmmCommBufferForbiddenAddress(). + // + + // + // Set UEFI runtime memory with EFI_MEMORY_RO as not present. + // + if (mUefiMemoryAttributesTable != NULL) { + Entry = (EFI_MEMORY_DESCRIPTOR *)(mUefiMemoryAttributesTable + 1); + for (Index = 0; Index < mUefiMemoryAttributesTable->NumberOfEntries; Index++) { + if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == EfiRuntimeServicesData) { + if ((Entry->Attribute & EFI_MEMORY_RO) != 0) { + Status = SmmSetMemoryAttributes ( + Entry->PhysicalStart, + EFI_PAGES_TO_SIZE((UINTN)Entry->NumberOfPages), + EFI_MEMORY_RP + ); + DEBUG (( + DEBUG_INFO, + "UefiMemoryAttribute protection: 0x%lx - 0x%lx %r\n", + Entry->PhysicalStart, + Entry->PhysicalStart + (UINT64)EFI_PAGES_TO_SIZE((UINTN)Entry->NumberOfPages), + Status + )); + } + } + Entry = NEXT_MEMORY_DESCRIPTOR (Entry, mUefiMemoryAttributesTable->DescriptorSize); + } + } + // + // Do not free mUefiMemoryAttributesTable, it will be checked in IsSmmCommBufferForbiddenAddress(). // } @@ -1083,21 +1244,44 @@ IsSmmCommBufferForbiddenAddress ( EFI_MEMORY_DESCRIPTOR *MemoryMap; UINTN MemoryMapEntryCount; UINTN Index; + EFI_MEMORY_DESCRIPTOR *Entry; - if (mUefiMemoryMap == NULL) { - return FALSE; + if (mUefiMemoryMap != NULL) { + MemoryMap = mUefiMemoryMap; + MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize; + for (Index = 0; Index < MemoryMapEntryCount; Index++) { + if (IsUefiPageNotPresent (MemoryMap)) { + if ((Address >= MemoryMap->PhysicalStart) && + (Address < MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)) ) { + return TRUE; + } + } + MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize); + } } - MemoryMap = mUefiMemoryMap; - MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize; - for (Index = 0; Index < MemoryMapEntryCount; Index++) { - if (IsUefiPageNotPresent (MemoryMap)) { - if ((Address >= MemoryMap->PhysicalStart) && - (Address < MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)) ) { + if (mGcdMemSpace != NULL) { + for (Index = 0; Index < mGcdMemNumberOfDesc; Index++) { + if ((Address >= mGcdMemSpace[Index].BaseAddress) && + (Address < mGcdMemSpace[Index].BaseAddress + mGcdMemSpace[Index].Length) ) { return TRUE; } } - MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize); + } + + if (mUefiMemoryAttributesTable != NULL) { + Entry = (EFI_MEMORY_DESCRIPTOR *)(mUefiMemoryAttributesTable + 1); + for (Index = 0; Index < mUefiMemoryAttributesTable->NumberOfEntries; Index++) { + if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == EfiRuntimeServicesData) { + if ((Entry->Attribute & EFI_MEMORY_RO) != 0) { + if ((Address >= Entry->PhysicalStart) && + (Address < Entry->PhysicalStart + LShiftU64 (Entry->NumberOfPages, EFI_PAGE_SHIFT))) { + return TRUE; + } + Entry = NEXT_MEMORY_DESCRIPTOR (Entry, mUefiMemoryAttributesTable->DescriptorSize); + } + } + } } return FALSE; }