From 16c121f411bb69c641079723e94fb4f80af3c66e Mon Sep 17 00:00:00 2001 From: Jiewen Yao Date: Thu, 23 Nov 2017 10:22:00 +0800 Subject: Add Check for MemoryAttributeTable. Cc: Michael A Kubacki Cc: Amy Chan Cc: Chasel Chiu Cc: Brett Wang Cc: Daocheng Bu Cc: Isaac W Oram Cc: Rangasai V Chaganty Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Jiewen Yao Reviewed-by: Amy Chan --- .../TestPointCheckLib/DxeTestPointCheckLib.c | 23 ++++++++++-- .../SmmCheckCommunicationBuffer.c | 32 ++++++++++++++--- .../TestPointCheckLib/SmmTestPointCheckLib.c | 42 +++++++++++++++++----- .../TestPointCheckLib/SmmTestPointCheckLib.inf | 1 + .../Library/TestPointCheckLib/TestPointInternal.h | 2 ++ 5 files changed, 85 insertions(+), 15 deletions(-) diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c index fdeff8243d..7a890bcd99 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.c @@ -23,6 +23,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include +#include #include #include "TestPointInternal.h" @@ -432,6 +433,8 @@ TestPointDxeSmmReadyToBootSmmPageProtection ( EFI_GCD_IO_SPACE_DESCRIPTOR *GcdIoMap; UINTN GcdMemoryMapNumberOfDescriptors; UINTN GcdIoMapNumberOfDescriptors; + EFI_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable; + UINTN MemoryAttributesTableSize; EFI_STATUS Status; UINTN CommSize; UINT8 *CommBuffer; @@ -452,7 +455,14 @@ TestPointDxeSmmReadyToBootSmmPageProtection ( TestPointDumpUefiMemoryMap (&UefiMemoryMap, &UefiMemoryMapSize, &UefiDescriptorSize, FALSE); TestPointDumpGcd (&GcdMemoryMap, &GcdMemoryMapNumberOfDescriptors, &GcdIoMap, &GcdIoMapNumberOfDescriptors, FALSE); - + + MemoryAttributesTable = NULL; + MemoryAttributesTableSize = 0; + Status = EfiGetSystemConfigurationTable (&gEfiMemoryAttributesTableGuid, &MemoryAttributesTable); + if (!EFI_ERROR (Status)) { + MemoryAttributesTableSize = sizeof(EFI_MEMORY_ATTRIBUTES_TABLE) + MemoryAttributesTable->DescriptorSize * MemoryAttributesTable->NumberOfEntries; + } + Status = gBS->LocateProtocol(&gEfiSmmCommunicationProtocolGuid, NULL, (VOID **)&SmmCommunication); if (EFI_ERROR(Status)) { DEBUG ((DEBUG_INFO, "TestPointDxeSmmReadyToBootSmmPageProtection: Locate SmmCommunication protocol - %r\n", Status)); @@ -463,7 +473,8 @@ TestPointDxeSmmReadyToBootSmmPageProtection ( sizeof(TEST_POINT_SMM_COMMUNICATION_UEFI_GCD_MAP_INFO) + UefiMemoryMapSize + GcdMemoryMapNumberOfDescriptors * sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR) + - GcdIoMapNumberOfDescriptors * sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR); + GcdIoMapNumberOfDescriptors * sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR) + + MemoryAttributesTableSize; Status = EfiGetSystemConfigurationTable( &gEdkiiPiSmmCommunicationRegionTableGuid, @@ -502,6 +513,9 @@ TestPointDxeSmmReadyToBootSmmPageProtection ( CommData->GcdMemoryMapSize = GcdMemoryMapNumberOfDescriptors * sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR); CommData->GcdIoMapOffset = CommData->GcdMemoryMapOffset + CommData->GcdMemoryMapSize; CommData->GcdIoMapSize = GcdIoMapNumberOfDescriptors * sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR); + CommData->UefiMemoryAttributeTableOffset = CommData->GcdIoMapOffset + CommData->GcdIoMapSize; + CommData->UefiMemoryAttributeTableSize = MemoryAttributesTableSize; + CopyMem ( (VOID *)((UINTN)CommData + CommData->UefiMemoryMapOffset), UefiMemoryMap, @@ -517,6 +531,11 @@ TestPointDxeSmmReadyToBootSmmPageProtection ( GcdIoMap, CommData->GcdIoMapSize ); + CopyMem ( + (VOID *)((UINTN)CommData + CommData->UefiMemoryAttributeTableOffset), + MemoryAttributesTable, + CommData->UefiMemoryAttributeTableSize + ); CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + CommHeader->MessageLength; Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize); diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmCheckCommunicationBuffer.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmCheckCommunicationBuffer.c index 027354c0bc..acc61ceaf7 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmCheckCommunicationBuffer.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmCheckCommunicationBuffer.c @@ -16,6 +16,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include +#include #define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \ ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size))) @@ -58,9 +59,10 @@ IsUefiPageNotPresent ( EFI_STATUS TestPointCheckSmmCommunicationBuffer ( - IN EFI_MEMORY_DESCRIPTOR *UefiMemoryMap, - IN UINTN UefiMemoryMapSize, - IN UINTN UefiDescriptorSize + IN EFI_MEMORY_DESCRIPTOR *UefiMemoryMap, + IN UINTN UefiMemoryMapSize, + IN UINTN UefiDescriptorSize, + IN EFI_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable ) { EFI_STATUS ReturnStatus; @@ -68,6 +70,7 @@ TestPointCheckSmmCommunicationBuffer ( EFI_MEMORY_DESCRIPTOR *MemoryMap; UINTN MemoryMapEntryCount; UINTN Index; + EFI_MEMORY_DESCRIPTOR *Entry; DEBUG ((DEBUG_INFO, "==== TestPointCheckSmmCommunicationBuffer - Enter\n")); @@ -89,7 +92,28 @@ TestPointCheckSmmCommunicationBuffer ( } MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, UefiDescriptorSize); } - + + if (MemoryAttributesTable != NULL) { + Entry = (EFI_MEMORY_DESCRIPTOR *)(MemoryAttributesTable + 1); + for (Index = 0; Index < MemoryAttributesTable->NumberOfEntries; Index++) { + if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == EfiRuntimeServicesData) { + if ((Entry->Attribute & EFI_MEMORY_RO) != 0) { + DEBUG ((DEBUG_INFO, "UEFI MemoryAttributeTable Checking 0x%lx - 0x%x\n", Entry->PhysicalStart, EFI_PAGES_TO_SIZE(Entry->NumberOfPages))); + Status = TestPointCheckPageTable ( + Entry->PhysicalStart, + EFI_PAGES_TO_SIZE((UINTN)Entry->NumberOfPages), + FALSE, + TRUE + ); + if (EFI_ERROR(Status)) { + ReturnStatus = Status; + } + } + } + Entry = NEXT_MEMORY_DESCRIPTOR (Entry, MemoryAttributesTable->DescriptorSize); + } + } + if (EFI_ERROR (ReturnStatus)) { TestPointLibAppendErrorString ( PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV, diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c index 2b914231ce..25b86e8b24 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.c @@ -17,8 +17,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include +#include #include #include +#include #include "TestPointInternal.h" @@ -46,9 +48,10 @@ TestPointCheckSmmPaging ( EFI_STATUS TestPointCheckSmmCommunicationBuffer ( - IN EFI_MEMORY_DESCRIPTOR *UefiMemoryMap, - IN UINTN UefiMemoryMapSize, - IN UINTN UefiDescriptorSize + IN EFI_MEMORY_DESCRIPTOR *UefiMemoryMap, + IN UINTN UefiMemoryMapSize, + IN UINTN UefiDescriptorSize, + IN EFI_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable ); VOID @@ -77,6 +80,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED EFI_GCD_IO_SPACE_DESCRIPTOR *mGcdIoMap; GLOBAL_REMOVE_IF_UNREFERENCED UINTN mGcdMemoryMapNumberOfDescriptors; GLOBAL_REMOVE_IF_UNREFERENCED UINTN mGcdIoMapNumberOfDescriptors; +EFI_MEMORY_ATTRIBUTES_TABLE *mUefiMemoryAttributesTable; + GLOBAL_REMOVE_IF_UNREFERENCED ADAPTER_INFO_PLATFORM_TEST_POINT_STRUCT mTestPointStruct = { PLATFORM_TEST_POINT_VERSION, PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV, @@ -164,7 +169,10 @@ TestPointSmmReadyToLockSecureSmmCommunicationBuffer ( VOID ) { - + EFI_STATUS Status; + EFI_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable; + UINTN MemoryAttributesTableSize; + if ((mFeatureImplemented[6] & TEST_POINT_BYTE6_SMM_READY_TO_LOCK_SECURE_SMM_COMMUNICATION_BUFFER) == 0) { return EFI_SUCCESS; } @@ -176,6 +184,13 @@ TestPointSmmReadyToLockSecureSmmCommunicationBuffer ( // TestPointDumpUefiMemoryMap (&mUefiMemoryMap, &mUefiMemoryMapSize, &mUefiDescriptorSize, TRUE); TestPointDumpGcd (&mGcdMemoryMap, &mGcdMemoryMapNumberOfDescriptors, &mGcdIoMap, &mGcdIoMapNumberOfDescriptors, TRUE); + + Status = EfiGetSystemConfigurationTable (&gEfiMemoryAttributesTableGuid, &MemoryAttributesTable); + if (!EFI_ERROR (Status)) { + MemoryAttributesTableSize = sizeof(EFI_MEMORY_ATTRIBUTES_TABLE) + MemoryAttributesTable->DescriptorSize * MemoryAttributesTable->NumberOfEntries; + mUefiMemoryAttributesTable = AllocateCopyPool (MemoryAttributesTableSize, MemoryAttributesTable); + ASSERT (mUefiMemoryAttributesTable != NULL); + } // // Defer the validation to TestPointSmmReadyToBootSecureSmmCommunicationBuffer, because page table setup later. // @@ -217,7 +232,7 @@ TestPointSmmReadyToBootSmmPageProtection ( if (mUefiMemoryMap != NULL) { Result = TRUE; - Status = TestPointCheckSmmCommunicationBuffer (mUefiMemoryMap, mUefiMemoryMapSize, mUefiDescriptorSize); + Status = TestPointCheckSmmCommunicationBuffer (mUefiMemoryMap, mUefiMemoryMapSize, mUefiDescriptorSize, mUefiMemoryAttributesTable); if (EFI_ERROR(Status)) { Result = FALSE; } @@ -284,7 +299,7 @@ TestPointSmmReadyToBootSmmPageProtectionHandler ( DEBUG((DEBUG_ERROR, "TestPointSmmReadyToBootSmmPageProtectionHandler: UefiMemoryMapOffset invalid!\n")); goto Done; } - if (CommData->UefiMemoryMapSize > TempCommBufferSize - CommData->UefiMemoryMapOffset) { + if (CommData->UefiMemoryMapSize >= TempCommBufferSize - CommData->UefiMemoryMapOffset) { DEBUG((DEBUG_ERROR, "TestPointSmmReadyToBootSmmPageProtectionHandler: UefiMemoryMapSize invalid!\n")); goto Done; } @@ -292,7 +307,7 @@ TestPointSmmReadyToBootSmmPageProtectionHandler ( DEBUG((DEBUG_ERROR, "TestPointSmmReadyToBootSmmPageProtectionHandler: GcdMemoryMapOffset invalid!\n")); goto Done; } - if (CommData->GcdMemoryMapSize > TempCommBufferSize - CommData->GcdMemoryMapOffset) { + if (CommData->GcdMemoryMapSize >= TempCommBufferSize - CommData->GcdMemoryMapOffset) { DEBUG((DEBUG_ERROR, "TestPointSmmReadyToBootSmmPageProtectionHandler: GcdMemoryMapSize invalid!\n")); goto Done; } @@ -300,10 +315,18 @@ TestPointSmmReadyToBootSmmPageProtectionHandler ( DEBUG((DEBUG_ERROR, "TestPointSmmReadyToBootSmmPageProtectionHandler: GcdIoMapOffset invalid!\n")); goto Done; } - if (CommData->GcdIoMapSize != TempCommBufferSize - CommData->GcdIoMapOffset) { + if (CommData->GcdIoMapSize >= TempCommBufferSize - CommData->GcdIoMapOffset) { DEBUG((DEBUG_ERROR, "TestPointSmmReadyToBootSmmPageProtectionHandler: GcdIoMapSize invalid!\n")); goto Done; } + if (CommData->UefiMemoryAttributeTableOffset != CommData->GcdIoMapOffset + CommData->GcdIoMapSize) { + DEBUG((DEBUG_ERROR, "TestPointSmmReadyToBootSmmPageProtectionHandler: UefiMemoryAttributeTableOffset invalid!\n")); + goto Done; + } + if (CommData->UefiMemoryAttributeTableSize != TempCommBufferSize - CommData->UefiMemoryAttributeTableOffset) { + DEBUG((DEBUG_ERROR, "TestPointSmmReadyToBootSmmPageProtectionHandler: UefiMemoryAttributeTableSize invalid!\n")); + goto Done; + } if (CommData->UefiMemoryMapSize != 0) { Result = TRUE; @@ -311,7 +334,8 @@ TestPointSmmReadyToBootSmmPageProtectionHandler ( Status = TestPointCheckSmmCommunicationBuffer ( (EFI_MEMORY_DESCRIPTOR *)((UINTN)CommData + CommData->UefiMemoryMapOffset), (UINTN)CommData->UefiMemoryMapSize, - mUefiDescriptorSize + mUefiDescriptorSize, + (CommData->UefiMemoryAttributeTableSize != 0) ? (EFI_MEMORY_ATTRIBUTES_TABLE *)((UINTN)CommData + CommData->UefiMemoryAttributeTableOffset) : NULL ); if (EFI_ERROR(Status)) { Result = FALSE; diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.inf b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.inf index cb1a21bff0..2dd81ba261 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.inf +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.inf @@ -29,6 +29,7 @@ MemoryAllocationLib DevicePathLib PeCoffGetEntryPointLib + UefiLib SmmMemLib TestPointLib diff --git a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/TestPointInternal.h b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/TestPointInternal.h index 2144e94239..82dfaed6b5 100644 --- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/TestPointInternal.h +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/TestPointInternal.h @@ -36,6 +36,8 @@ typedef struct { UINT64 GcdMemoryMapSize; UINT64 GcdIoMapOffset; UINT64 GcdIoMapSize; + UINT64 UefiMemoryAttributeTableOffset; + UINT64 UefiMemoryAttributeTableSize; } TEST_POINT_SMM_COMMUNICATION_UEFI_GCD_MAP_INFO; #define TEST_POINT_SMM_COMMUNICATION_GUID { \ -- cgit v1.2.3