* [PATCH v2 1/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-03 12:22 ` [edk2-devel] " Laszlo Ersek
2020-07-03 13:52 ` Laszlo Ersek
2020-07-02 5:15 ` [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098) Guomin Jiang
` (8 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel
Cc: Michael Kubacki, Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao,
Debkumar De, Harry Han, Catharine West
From: Michael Kubacki <michael.a.kubacki@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
Introduces new changes to PeiCore to move the contents of temporary
RAM visible to the PeiCore to permanent memory. This expands on
pre-existing shadowing support in the PeiCore to perform the following
additional actions:
1. Migrate pointers in PPIs installed in PeiCore to the permanent
memory copy of PeiCore.
2. Copy all installed firmware volumes to permanent memory.
3. Relocate and fix up the PEIMs within the firmware volumes.
4. Convert all PPIs into the migrated firmware volume to the corresponding
PPI address in the permanent memory location.
This applies to PPIs and PEI notifications.
5. Convert all status code callbacks in the migrated firmware volume to
the corresponding address in the permanent memory location.
6. Update the FV HOB to the corresponding firmware volume in permanent
memory.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 402 ++++++++++++++++++
MdeModulePkg/Core/Pei/Image/Image.c | 115 +++++
MdeModulePkg/Core/Pei/Memory/MemoryServices.c | 82 ++++
MdeModulePkg/Core/Pei/PeiMain.h | 168 ++++++++
MdeModulePkg/Core/Pei/PeiMain.inf | 1 +
MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 16 +
MdeModulePkg/Core/Pei/Ppi/Ppi.c | 287 +++++++++++++
7 files changed, 1071 insertions(+)
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 4c2eac1384e8..ef88b3423376 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -952,6 +952,408 @@ PeiCheckAndSwitchStack (
}
}
+/**
+ Migrate a PEIM from Temporary RAM to permanent memory.
+
+ @param PeimFileHandle Pointer to the FFS file header of the image.
+ @param MigratedFileHandle Pointer to the FFS file header of the migrated image.
+
+ @retval EFI_SUCCESS Sucessfully migrated the PEIM to permanent memory.
+
+**/
+EFI_STATUS
+EFIAPI
+MigratePeim (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN EFI_PEI_FILE_HANDLE MigratedFileHandle
+ )
+{
+ EFI_STATUS Status;
+ EFI_FFS_FILE_HEADER *FileHeader;
+ VOID *Pe32Data;
+ VOID *ImageAddress;
+ CHAR8 *AsciiString;
+ UINTN Index;
+
+ Status = EFI_SUCCESS;
+
+ FileHeader = (EFI_FFS_FILE_HEADER *) FileHandle;
+ ASSERT (!IS_FFS_FILE2 (FileHeader));
+
+ ImageAddress = NULL;
+ PeiGetPe32Data (MigratedFileHandle, &ImageAddress);
+ if (ImageAddress != NULL) {
+ AsciiString = PeCoffLoaderGetPdbPointer (ImageAddress);
+ for (Index = 0; AsciiString[Index] != 0; Index++) {
+ if (AsciiString[Index] == '\\' || AsciiString[Index] == '/') {
+ AsciiString = AsciiString + Index + 1;
+ Index = 0;
+ } else if (AsciiString[Index] == '.') {
+ AsciiString[Index] = 0;
+ }
+ }
+ DEBUG ((DEBUG_INFO, "%a", AsciiString));
+
+ Pe32Data = (VOID *) ((UINTN) ImageAddress - (UINTN) MigratedFileHandle + (UINTN) FileHandle);
+ Status = LoadAndRelocatePeCoffImageInPlace (Pe32Data, ImageAddress);
+ ASSERT_EFI_ERROR (Status);
+ }
+
+ return Status;
+}
+
+/**
+ Migrate Status Code Callback function pointers inside an FV from temporary memory to permanent memory.
+
+ @param OrgFvHandle Address of FV Handle in temporary memory.
+ @param FvHandle Address of FV Handle in permanent memory.
+ @param FvSize Size of the FV.
+
+**/
+VOID
+ConvertStatusCodeCallbacks (
+ IN UINTN OrgFvHandle,
+ IN UINTN FvHandle,
+ IN UINTN FvSize
+ )
+{
+ EFI_PEI_HOB_POINTERS Hob;
+ UINTN *NumberOfEntries;
+ UINTN *CallbackEntry;
+ UINTN Index;
+
+ Hob.Raw = GetFirstGuidHob (&gStatusCodeCallbackGuid);
+ while (Hob.Raw != NULL) {
+ NumberOfEntries = GET_GUID_HOB_DATA (Hob);
+ CallbackEntry = NumberOfEntries + 1;
+ for (Index = 0; Index < *NumberOfEntries; Index++) {
+ if (((VOID *) CallbackEntry[Index]) != NULL) {
+ if ((CallbackEntry[Index] >= OrgFvHandle) && (CallbackEntry[Index] < (OrgFvHandle + FvSize))) {
+ DEBUG ((DEBUG_INFO, "Migrating CallbackEntry[%d] from 0x%08X to ", Index, CallbackEntry[Index]));
+ if (OrgFvHandle > FvHandle) {
+ CallbackEntry[Index] = CallbackEntry[Index] - (OrgFvHandle - FvHandle);
+ } else {
+ CallbackEntry[Index] = CallbackEntry[Index] + (FvHandle - OrgFvHandle);
+ }
+ DEBUG ((DEBUG_INFO, "0x%08X\n", CallbackEntry[Index]));
+ }
+ }
+ }
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ Hob.Raw = GetNextGuidHob (&gStatusCodeCallbackGuid, Hob.Raw);
+ }
+}
+
+/**
+ Migrates SEC modules in the given firmware volume.
+
+ Migrating SECURITY_CORE files requires special treatment since they are not tracked for PEI dispatch.
+
+ This functioun should be called after the FV has been copied to its post-memory location and the PEI Core FV list has
+ been updated.
+
+ @param Private Pointer to the PeiCore's private data structure.
+ @param FvIndex The firmware volume index to migrate.
+ @param OrgFvHandle The handle to the firmware volume in temporary memory.
+
+ @retval EFI_SUCCESS SEC modules were migrated successfully
+ @retval EFI_INVALID_PARAMETER The Private pointer is NULL or FvCount is invalid.
+
+**/
+EFI_STATUS
+EFIAPI
+MigrateSecModulesInFv (
+ IN PEI_CORE_INSTANCE *Private,
+ IN UINTN FvIndex,
+ IN UINTN OrgFvHandle
+ )
+{
+ EFI_STATUS Status;
+ EFI_STATUS FindFileStatus;
+ EFI_PEI_FILE_HANDLE MigratedFileHandle;
+ EFI_PEI_FILE_HANDLE FileHandle;
+ UINT32 SectionAuthenticationStatus;
+ UINT32 FileSize;
+ VOID *OrgPe32SectionData;
+ VOID *Pe32SectionData;
+ EFI_FFS_FILE_HEADER *FfsFileHeader;
+ EFI_COMMON_SECTION_HEADER *Section;
+ BOOLEAN IsFfs3Fv;
+ UINTN SectionInstance;
+
+ if (Private == NULL || FvIndex >= Private->FvCount) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ do {
+ FindFileStatus = PeiFfsFindNextFile (
+ GetPeiServicesTablePointer (),
+ EFI_FV_FILETYPE_SECURITY_CORE,
+ Private->Fv[FvIndex].FvHandle,
+ &MigratedFileHandle
+ );
+ if (!EFI_ERROR (FindFileStatus ) && MigratedFileHandle != NULL) {
+ FileHandle = (EFI_PEI_FILE_HANDLE) ((UINTN) MigratedFileHandle - (UINTN) Private->Fv[FvIndex].FvHandle + OrgFvHandle);
+ FfsFileHeader = (EFI_FFS_FILE_HEADER *) MigratedFileHandle;
+
+ DEBUG ((DEBUG_VERBOSE, " Migrating SEC_CORE MigratedFileHandle at 0x%x.\n", (UINTN) MigratedFileHandle));
+ DEBUG ((DEBUG_VERBOSE, " FileHandle at 0x%x.\n", (UINTN) FileHandle));
+
+ IsFfs3Fv = CompareGuid (&Private->Fv[FvIndex].FvHeader->FileSystemGuid, &gEfiFirmwareFileSystem3Guid);
+ if (IS_FFS_FILE2 (FfsFileHeader)) {
+ ASSERT (FFS_FILE2_SIZE (FfsFileHeader) > 0x00FFFFFF);
+ if (!IsFfs3Fv) {
+ DEBUG ((DEBUG_ERROR, "It is a FFS3 formatted file: %g in a non-FFS3 formatted FV.\n", &FfsFileHeader->Name));
+ return EFI_NOT_FOUND;
+ }
+ Section = (EFI_COMMON_SECTION_HEADER *) ((UINT8 *) FfsFileHeader + sizeof (EFI_FFS_FILE_HEADER2));
+ FileSize = FFS_FILE2_SIZE (FfsFileHeader) - sizeof (EFI_FFS_FILE_HEADER2);
+ } else {
+ Section = (EFI_COMMON_SECTION_HEADER *) ((UINT8 *) FfsFileHeader + sizeof (EFI_FFS_FILE_HEADER));
+ FileSize = FFS_FILE_SIZE (FfsFileHeader) - sizeof (EFI_FFS_FILE_HEADER);
+ }
+
+ SectionInstance = 1;
+ SectionAuthenticationStatus = 0;
+ Status = ProcessSection (
+ GetPeiServicesTablePointer (),
+ EFI_SECTION_PE32,
+ &SectionInstance,
+ Section,
+ FileSize,
+ &Pe32SectionData,
+ &SectionAuthenticationStatus,
+ IsFfs3Fv
+ );
+
+ if (!EFI_ERROR (Status)) {
+ OrgPe32SectionData = (VOID *) ((UINTN) Pe32SectionData - (UINTN) MigratedFileHandle + (UINTN) FileHandle);
+ DEBUG ((DEBUG_VERBOSE, " PE32 section in migrated file at 0x%x.\n", (UINTN) Pe32SectionData));
+ DEBUG ((DEBUG_VERBOSE, " PE32 section in original file at 0x%x.\n", (UINTN) OrgPe32SectionData));
+ Status = LoadAndRelocatePeCoffImageInPlace (OrgPe32SectionData, Pe32SectionData);
+ ASSERT_EFI_ERROR (Status);
+ }
+ }
+ } while (!EFI_ERROR (FindFileStatus));
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Migrates PEIMs in the given firmware volume.
+
+ @param Private Pointer to the PeiCore's private data structure.
+ @param FvIndex The firmware volume index to migrate.
+ @param OrgFvHandle The handle to the firmware volume in temporary memory.
+ @param FvHandle The handle to the firmware volume in permanent memory.
+
+ @retval EFI_SUCCESS The PEIMs in the FV were migrated successfully
+ @retval EFI_INVALID_PARAMETER The Private pointer is NULL or FvCount is invalid.
+
+**/
+EFI_STATUS
+EFIAPI
+MigratePeimsInFv (
+ IN PEI_CORE_INSTANCE *Private,
+ IN UINTN FvIndex,
+ IN UINTN OrgFvHandle,
+ IN UINTN FvHandle
+ )
+{
+ EFI_STATUS Status;
+ volatile UINTN FileIndex;
+ EFI_PEI_FILE_HANDLE MigratedFileHandle;
+ EFI_PEI_FILE_HANDLE FileHandle;
+
+ if (Private == NULL || FvIndex >= Private->FvCount) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Private->Fv[FvIndex].ScanFv) {
+ for (FileIndex = 0; FileIndex < Private->Fv[FvIndex].PeimCount; FileIndex++) {
+ if (Private->Fv[FvIndex].FvFileHandles[FileIndex] != NULL) {
+ FileHandle = Private->Fv[FvIndex].FvFileHandles[FileIndex];
+
+ MigratedFileHandle = (EFI_PEI_FILE_HANDLE) ((UINTN) FileHandle - OrgFvHandle + FvHandle);
+
+ DEBUG ((DEBUG_VERBOSE, " Migrating FileHandle %2d ", FileIndex));
+ Status = MigratePeim (FileHandle, MigratedFileHandle);
+ DEBUG ((DEBUG_INFO, "\n"));
+ ASSERT_EFI_ERROR (Status);
+
+ if (!EFI_ERROR (Status)) {
+ // if (Private->Fv[FvIndex].PeimState[FileIndex] == PEIM_STATE_REGISTER_FOR_SHADOW) {
+ // Private->Fv[FvIndex].PeimState[FileIndex]++;
+ // }
+ Private->Fv[FvIndex].FvFileHandles[FileIndex] = MigratedFileHandle;
+ if (FvIndex == Private->CurrentPeimFvCount) {
+ Private->CurrentFvFileHandles[FileIndex] = MigratedFileHandle;
+ }
+ }
+ }
+ }
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Migrate FVs out of temporary RAM before the cache is flushed.
+
+ @param Private PeiCore's private data structure
+ @param SecCoreData Points to a data structure containing information about the PEI core's operating
+ environment, such as the size and location of temporary RAM, the stack location and
+ the BFV location.
+
+ @retval EFI_SUCCESS Succesfully migrated installed FVs from temporary RAM to permanent memory.
+ @retval EFI_OUT_OF_RESOURCES Insufficient memory exists to allocate needed pages.
+
+**/
+EFI_STATUS
+EFIAPI
+EvacuateTempRam (
+ IN PEI_CORE_INSTANCE *Private,
+ IN CONST EFI_SEC_PEI_HAND_OFF *SecCoreData
+ )
+{
+ EFI_STATUS Status;
+ volatile UINTN FvIndex;
+ volatile UINTN FvChildIndex;
+ UINTN ChildFvOffset;
+ EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
+ EFI_FIRMWARE_VOLUME_HEADER *ChildFvHeader;
+ EFI_FIRMWARE_VOLUME_HEADER *MigratedFvHeader;
+ EFI_FIRMWARE_VOLUME_HEADER *MigratedChildFvHeader;
+
+ PEI_CORE_FV_HANDLE PeiCoreFvHandle;
+ EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
+
+ ASSERT (Private->PeiMemoryInstalled);
+
+ DEBUG ((DEBUG_VERBOSE, "Beginning evacuation of content in temporary RAM.\n"));
+
+ //
+ // Migrate PPI Pointers of PEI_CORE from temporary memory to newly loaded PEI_CORE in permanent memory.
+ //
+ Status = PeiLocatePpi ((CONST EFI_PEI_SERVICES **) &Private->Ps, &gEfiPeiCoreFvLocationPpiGuid, 0, NULL, (VOID **) &PeiCoreFvLocationPpi);
+ if (!EFI_ERROR (Status) && (PeiCoreFvLocationPpi->PeiCoreFvLocation != NULL)) {
+ PeiCoreFvHandle.FvHandle = (EFI_PEI_FV_HANDLE) PeiCoreFvLocationPpi->PeiCoreFvLocation;
+ } else {
+ PeiCoreFvHandle.FvHandle = (EFI_PEI_FV_HANDLE) SecCoreData->BootFirmwareVolumeBase;
+ }
+ for (FvIndex = 0; FvIndex < Private->FvCount; FvIndex++) {
+ if (Private->Fv[FvIndex].FvHandle == PeiCoreFvHandle.FvHandle) {
+ PeiCoreFvHandle = Private->Fv[FvIndex];
+ break;
+ }
+ }
+ Status = EFI_SUCCESS;
+
+ ConvertPeiCorePpiPointers (Private, PeiCoreFvHandle);
+
+ for (FvIndex = 0; FvIndex < Private->FvCount; FvIndex++) {
+ FvHeader = Private->Fv[FvIndex].FvHeader;
+ ASSERT (FvHeader != NULL);
+ ASSERT (FvIndex < Private->FvCount);
+
+ DEBUG ((DEBUG_VERBOSE, "FV[%02d] at 0x%x.\n", FvIndex, (UINTN) FvHeader));
+ if (
+ !(
+ ((EFI_PHYSICAL_ADDRESS)(UINTN) FvHeader >= Private->PhysicalMemoryBegin) &&
+ (((EFI_PHYSICAL_ADDRESS)(UINTN) FvHeader + (FvHeader->FvLength - 1)) < Private->FreePhysicalMemoryTop)
+ )
+ ) {
+ Status = PeiServicesAllocatePages (
+ EfiBootServicesCode,
+ EFI_SIZE_TO_PAGES ((UINTN) FvHeader->FvLength),
+ (EFI_PHYSICAL_ADDRESS *) &MigratedFvHeader
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ DEBUG ((
+ DEBUG_VERBOSE,
+ " Migrating FV[%d] from 0x%08X to 0x%08X\n",
+ FvIndex,
+ (UINTN) FvHeader,
+ (UINTN) MigratedFvHeader
+ ));
+
+ CopyMem (MigratedFvHeader, FvHeader, (UINTN) FvHeader->FvLength);
+
+ //
+ // Migrate any children for this FV now
+ //
+ for (FvChildIndex = FvIndex; FvChildIndex < Private->FvCount; FvChildIndex++) {
+ ChildFvHeader = Private->Fv[FvChildIndex].FvHeader;
+ if (
+ ((UINTN) ChildFvHeader > (UINTN) FvHeader) &&
+ (((UINTN) ChildFvHeader + ChildFvHeader->FvLength) < ((UINTN) FvHeader) + FvHeader->FvLength)
+ ) {
+ DEBUG ((DEBUG_VERBOSE, " Child FV[%02d] is being migrated.\n", FvChildIndex));
+ ChildFvOffset = (UINTN) ChildFvHeader - (UINTN) FvHeader;
+ DEBUG ((DEBUG_VERBOSE, " Child FV offset = 0x%x.\n", ChildFvOffset));
+ MigratedChildFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) ((UINTN) MigratedFvHeader + ChildFvOffset);
+ Private->Fv[FvChildIndex].FvHeader = MigratedChildFvHeader;
+ Private->Fv[FvChildIndex].FvHandle = (EFI_PEI_FV_HANDLE) MigratedChildFvHeader;
+ DEBUG ((DEBUG_VERBOSE, " Child migrated FV header at 0x%x.\n", (UINTN) MigratedChildFvHeader));
+
+ // @todo: find issue with file and section alignment in SEC PE32 images for migration
+ // (alignment in P32 is given as 32-bit when actual alignment is 16-bit)
+ // SEC PPIs are currently re-installed with a dedicated PEIM
+ // Status = MigrateSecModulesInFv (Private, FvChildIndex, (UINTN) ChildFvHeader);
+ // ASSERT_EFI_ERROR (Status);
+ Status = MigratePeimsInFv (Private, FvChildIndex, (UINTN) ChildFvHeader, (UINTN) MigratedChildFvHeader);
+ ASSERT_EFI_ERROR (Status);
+
+ ConvertPpiPointersFv (
+ Private,
+ (UINTN) ChildFvHeader,
+ (UINTN) MigratedChildFvHeader,
+ (UINTN) ChildFvHeader->FvLength - 1
+ );
+
+ ConvertStatusCodeCallbacks (
+ (UINTN) ChildFvHeader,
+ (UINTN) MigratedChildFvHeader,
+ (UINTN) ChildFvHeader->FvLength - 1
+ );
+
+ ConvertFvHob (Private, (UINTN) ChildFvHeader, (UINTN) MigratedChildFvHeader);
+ }
+ }
+ Private->Fv[FvIndex].FvHeader = MigratedFvHeader;
+ Private->Fv[FvIndex].FvHandle = (EFI_PEI_FV_HANDLE) MigratedFvHeader;
+
+ // @todo: find issue with file and section alignment in SEC PE32 images for migration
+ // (alignment in P32 is given as 32-bit when actual alignment is 16-bit)
+ // SEC PPIs are currently re-installed with a dedicated PEIM
+ // Status = MigrateSecModulesInFv (Private, FvIndex, (UINTN) FvHeader);
+ // ASSERT_EFI_ERROR (Status);
+ Status = MigratePeimsInFv (Private, FvIndex, (UINTN) FvHeader, (UINTN) MigratedFvHeader);
+ ASSERT_EFI_ERROR (Status);
+
+ ConvertPpiPointersFv (
+ Private,
+ (UINTN) FvHeader,
+ (UINTN) MigratedFvHeader,
+ (UINTN) FvHeader->FvLength - 1
+ );
+
+ ConvertStatusCodeCallbacks (
+ (UINTN) FvHeader,
+ (UINTN) MigratedFvHeader,
+ (UINTN) FvHeader->FvLength - 1
+ );
+
+ ConvertFvHob (Private, (UINTN) FvHeader, (UINTN) MigratedFvHeader);
+ }
+ }
+
+ RemoveFvHobsInTemporaryMemory (Private);
+
+ return Status;
+}
+
/**
Conduct PEIM dispatch.
diff --git a/MdeModulePkg/Core/Pei/Image/Image.c b/MdeModulePkg/Core/Pei/Image/Image.c
index e3ee3699337f..612797722a3e 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -444,6 +444,121 @@ LoadAndRelocatePeCoffImage (
return ReturnStatus;
}
+/**
+ Loads and relocates a PE/COFF image in place.
+
+ @param Pe32Data The base address of the PE/COFF file that is to be loaded and relocated
+ @param ImageAddress The base address of the relocated PE/COFF image
+
+ @retval EFI_SUCCESS The file was loaded and relocated
+
+**/
+EFI_STATUS
+LoadAndRelocatePeCoffImageInPlace (
+ IN VOID *Pe32Data,
+ IN VOID *ImageAddress
+ )
+{
+ EFI_STATUS Status;
+ PE_COFF_LOADER_IMAGE_CONTEXT ImageContext;
+
+ ZeroMem (&ImageContext, sizeof (ImageContext));
+ ImageContext.Handle = Pe32Data;
+ ImageContext.ImageRead = PeiImageRead;
+
+ Status = PeCoffLoaderGetImageInfo (&ImageContext);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ ImageContext.ImageAddress = (PHYSICAL_ADDRESS)(UINTN) ImageAddress;
+
+ //
+ // Load the image in place
+ //
+ Status = PeCoffLoaderLoadImage (&ImageContext);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ //
+ // Relocate the image in place
+ //
+ Status = PeCoffLoaderRelocateImage (&ImageContext);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ //
+ // Flush the instruction cache so the image data is written before we execute it
+ //
+ if (ImageContext.ImageAddress != (EFI_PHYSICAL_ADDRESS)(UINTN) Pe32Data) {
+ InvalidateInstructionCacheRange ((VOID *)(UINTN)ImageContext.ImageAddress, (UINTN)ImageContext.ImageSize);
+ }
+
+ return Status;
+}
+
+/**
+ Find the PE32 Data for an FFS file.
+
+ @param FileHandle Pointer to the FFS file header of the image.
+ @param Pe32Data Pointer to a (VOID *) PE32 Data pointer.
+
+ @retval EFI_SUCCESS Image is successfully loaded.
+ @retval EFI_NOT_FOUND Fail to locate PE32 Data.
+
+**/
+EFI_STATUS
+PeiGetPe32Data (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ OUT VOID **Pe32Data
+ )
+{
+ EFI_STATUS Status;
+ EFI_SECTION_TYPE SearchType1;
+ EFI_SECTION_TYPE SearchType2;
+ UINT32 AuthenticationState;
+
+ *Pe32Data = NULL;
+
+ if (FeaturePcdGet (PcdPeiCoreImageLoaderSearchTeSectionFirst)) {
+ SearchType1 = EFI_SECTION_TE;
+ SearchType2 = EFI_SECTION_PE32;
+ } else {
+ SearchType1 = EFI_SECTION_PE32;
+ SearchType2 = EFI_SECTION_TE;
+ }
+
+ //
+ // Try to find a first exe section (if PcdPeiCoreImageLoaderSearchTeSectionFirst
+ // is true, TE will be searched first).
+ //
+ Status = PeiServicesFfsFindSectionData3 (
+ SearchType1,
+ 0,
+ FileHandle,
+ Pe32Data,
+ &AuthenticationState
+ );
+ //
+ // If we didn't find a first exe section, try to find the second exe section.
+ //
+ if (EFI_ERROR (Status)) {
+ Status = PeiServicesFfsFindSectionData3 (
+ SearchType2,
+ 0,
+ FileHandle,
+ Pe32Data,
+ &AuthenticationState
+ );
+ }
+ return Status;
+}
+
/**
Loads a PEIM into memory for subsequent execution. If there are compressed
images or images that need to be relocated into memory for performance reasons,
diff --git a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
index 6b3a64a811cd..9d933f0393a8 100644
--- a/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
+++ b/MdeModulePkg/Core/Pei/Memory/MemoryServices.c
@@ -166,6 +166,88 @@ MigrateMemoryPages (
Private->FreePhysicalMemoryTop = NewMemPagesBase;
}
+/**
+ Removes any FV HOBs whose base address is not in PEI installed memory.
+
+ @param[in] Private Pointer to PeiCore's private data structure.
+
+**/
+VOID
+RemoveFvHobsInTemporaryMemory (
+ IN PEI_CORE_INSTANCE *Private
+ )
+{
+ EFI_PEI_HOB_POINTERS Hob;
+ EFI_HOB_FIRMWARE_VOLUME *FirmwareVolumeHob;
+
+ DEBUG ((DEBUG_INFO, "Removing FVs in FV HOB not already migrated to permanent memory.\n"));
+
+ for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
+ if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV || GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV2 || GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV3) {
+ FirmwareVolumeHob = Hob.FirmwareVolume;
+ DEBUG ((DEBUG_INFO, " Found FV HOB.\n"));
+ DEBUG ((
+ DEBUG_INFO,
+ " BA=%016lx L=%016lx\n",
+ FirmwareVolumeHob->BaseAddress,
+ FirmwareVolumeHob->Length
+ ));
+ if (
+ !(
+ ((EFI_PHYSICAL_ADDRESS) (UINTN) FirmwareVolumeHob->BaseAddress >= Private->PhysicalMemoryBegin) &&
+ (((EFI_PHYSICAL_ADDRESS) (UINTN) FirmwareVolumeHob->BaseAddress + (FirmwareVolumeHob->Length - 1)) < Private->FreePhysicalMemoryTop)
+ )
+ ) {
+ DEBUG ((DEBUG_INFO, " Removing FV HOB to an FV in T-RAM (was not migrated).\n"));
+ Hob.Header->HobType = EFI_HOB_TYPE_UNUSED;
+ }
+ }
+ }
+}
+
+/**
+ Migrate the base address in firmware volume allocation HOBs
+ from temporary memory to PEI installed memory.
+
+ @param[in] PrivateData Pointer to PeiCore's private data structure.
+ @param[in] OrgFvHandle Address of FV Handle in temporary memory.
+ @param[in] FvHandle Address of FV Handle in permanent memory.
+
+**/
+VOID
+ConvertFvHob (
+ IN PEI_CORE_INSTANCE *PrivateData,
+ IN UINTN OrgFvHandle,
+ IN UINTN FvHandle
+ )
+{
+ EFI_PEI_HOB_POINTERS Hob;
+ EFI_HOB_FIRMWARE_VOLUME *FirmwareVolumeHob;
+ EFI_HOB_FIRMWARE_VOLUME2 *FirmwareVolume2Hob;
+ EFI_HOB_FIRMWARE_VOLUME3 *FirmwareVolume3Hob;
+
+ DEBUG ((DEBUG_INFO, "Converting FVs in FV HOB.\n"));
+
+ for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
+ if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV) {
+ FirmwareVolumeHob = Hob.FirmwareVolume;
+ if (FirmwareVolumeHob->BaseAddress == OrgFvHandle) {
+ FirmwareVolumeHob->BaseAddress = FvHandle;
+ }
+ } else if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV2) {
+ FirmwareVolume2Hob = Hob.FirmwareVolume2;
+ if (FirmwareVolume2Hob->BaseAddress == OrgFvHandle) {
+ FirmwareVolume2Hob->BaseAddress = FvHandle;
+ }
+ } else if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV3) {
+ FirmwareVolume3Hob = Hob.FirmwareVolume3;
+ if (FirmwareVolume3Hob->BaseAddress == OrgFvHandle) {
+ FirmwareVolume3Hob->BaseAddress = FvHandle;
+ }
+ }
+ }
+}
+
/**
Migrate MemoryBaseAddress in memory allocation HOBs
from the temporary memory to PEI installed memory.
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
index 56b3bd85793d..b0101dba5e30 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -394,6 +394,41 @@ PeimDispatchReadiness (
IN VOID *DependencyExpression
);
+/**
+ Migrate a PEIM from Temporary RAM to permanent memory.
+
+ @param PeimFileHandle Pointer to the FFS file header of the image.
+ @param MigratedFileHandle Pointer to the FFS file header of the migrated image.
+
+ @retval EFI_SUCCESS Sucessfully migrated the PEIM to permanent memory.
+
+**/
+EFI_STATUS
+EFIAPI
+MigratePeim (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN EFI_PEI_FILE_HANDLE MigratedFileHandle
+ );
+
+/**
+ Migrate FVs out of Temporary RAM before the cache is flushed.
+
+ @param Private PeiCore's private data structure
+ @param SecCoreData Points to a data structure containing information about the PEI core's operating
+ environment, such as the size and location of temporary RAM, the stack location and
+ the BFV location.
+
+ @retval EFI_SUCCESS Succesfully migrated installed FVs from Temporary RAM to permanent memory.
+ @retval EFI_OUT_OF_RESOURCES Insufficient memory exists to allocate needed pages.
+
+**/
+EFI_STATUS
+EFIAPI
+EvacuateTempRam (
+ IN PEI_CORE_INSTANCE *Private,
+ IN CONST EFI_SEC_PEI_HAND_OFF *SecCoreData
+ );
+
/**
Conduct PEIM dispatch.
@@ -477,6 +512,50 @@ ConvertPpiPointers (
IN PEI_CORE_INSTANCE *PrivateData
);
+/**
+
+ Migrate Notify Pointers inside an FV from temporary memory to permanent memory.
+
+ @param PrivateData Pointer to PeiCore's private data structure.
+ @param OrgFvHandle Address of FV Handle in temporary memory.
+ @param FvHandle Address of FV Handle in permanent memory.
+ @param FvSize Size of the FV.
+
+**/
+VOID
+ConvertPpiPointersFv (
+ IN PEI_CORE_INSTANCE *PrivateData,
+ IN UINTN OrgFvHandle,
+ IN UINTN FvHandle,
+ IN UINTN FvSize
+ );
+
+/**
+
+ Migrate PPI Pointers of PEI_CORE from temporary memory to permanent memory.
+
+ @param PrivateData Pointer to PeiCore's private data structure.
+ @param CoreFvHandle Address of PEI_CORE FV Handle in temporary memory.
+
+**/
+VOID
+ConvertPeiCorePpiPointers (
+ IN PEI_CORE_INSTANCE *PrivateData,
+ PEI_CORE_FV_HANDLE CoreFvHandle
+ );
+
+/**
+
+ Dumps the PPI lists to debug output.
+
+ @param PrivateData Points to PeiCore's private instance data.
+
+**/
+VOID
+DumpPpiList (
+ IN PEI_CORE_INSTANCE *PrivateData
+ );
+
/**
Install PPI services. It is implementation of EFI_PEI_SERVICE.InstallPpi.
@@ -808,6 +887,37 @@ PeiFfsFindNextFile (
IN OUT EFI_PEI_FILE_HANDLE *FileHandle
);
+/**
+ Go through the file to search SectionType section.
+ Search within encapsulation sections (compression and GUIDed) recursively,
+ until the match section is found.
+
+ @param PeiServices An indirect pointer to the EFI_PEI_SERVICES table published by the PEI Foundation.
+ @param SectionType Filter to find only section of this type.
+ @param SectionInstance Pointer to the filter to find the specific instance of section.
+ @param Section From where to search.
+ @param SectionSize The file size to search.
+ @param OutputBuffer A pointer to the discovered section, if successful.
+ NULL if section not found
+ @param AuthenticationStatus Updated upon return to point to the authentication status for this section.
+ @param IsFfs3Fv Indicates the FV format.
+
+ @return EFI_NOT_FOUND The match section is not found.
+ @return EFI_SUCCESS The match section is found.
+
+**/
+EFI_STATUS
+ProcessSection (
+ IN CONST EFI_PEI_SERVICES **PeiServices,
+ IN EFI_SECTION_TYPE SectionType,
+ IN OUT UINTN *SectionInstance,
+ IN EFI_COMMON_SECTION_HEADER *Section,
+ IN UINTN SectionSize,
+ OUT VOID **OutputBuffer,
+ OUT UINT32 *AuthenticationStatus,
+ IN BOOLEAN IsFfs3Fv
+ );
+
/**
Searches for the next matching section within the specified file.
@@ -931,6 +1041,33 @@ MigrateMemoryPages (
IN BOOLEAN TemporaryRamMigrated
);
+/**
+ Removes any FV HOBs whose base address is not in PEI installed memory.
+
+ @param[in] Private Pointer to PeiCore's private data structure.
+
+**/
+VOID
+RemoveFvHobsInTemporaryMemory (
+ IN PEI_CORE_INSTANCE *Private
+ );
+
+/**
+ Migrate the base address in firmware volume allocation HOBs
+ from temporary memory to PEI installed memory.
+
+ @param[in] PrivateData Pointer to PeiCore's private data structure.
+ @param[in] OrgFvHandle Address of FV Handle in temporary memory.
+ @param[in] FvHandle Address of FV Handle in permanent memory.
+
+**/
+VOID
+ConvertFvHob (
+ IN PEI_CORE_INSTANCE *PrivateData,
+ IN UINTN OrgFvHandle,
+ IN UINTN FvHandle
+ );
+
/**
Migrate MemoryBaseAddress in memory allocation HOBs
from the temporary memory to PEI installed memory.
@@ -1249,6 +1386,37 @@ InitializeImageServices (
IN PEI_CORE_INSTANCE *OldCoreData
);
+/**
+ Loads and relocates a PE/COFF image in place.
+
+ @param Pe32Data The base address of the PE/COFF file that is to be loaded and relocated
+ @param ImageAddress The base address of the relocated PE/COFF image
+
+ @retval EFI_SUCCESS The file was loaded and relocated
+
+**/
+EFI_STATUS
+LoadAndRelocatePeCoffImageInPlace (
+ IN VOID *Pe32Data,
+ IN VOID *ImageAddress
+ );
+
+/**
+ Find the PE32 Data for an FFS file.
+
+ @param FileHandle Pointer to the FFS file header of the image.
+ @param Pe32Data Pointer to a (VOID *) PE32 Data pointer.
+
+ @retval EFI_SUCCESS Image is successfully loaded.
+ @retval EFI_NOT_FOUND Fail to locate PE32 Data.
+
+**/
+EFI_STATUS
+PeiGetPe32Data (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ OUT VOID **Pe32Data
+ );
+
/**
The wrapper function of PeiLoadImageLoadImage().
diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf
index 6e25cc40232a..5ff14100a65f 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.inf
+++ b/MdeModulePkg/Core/Pei/PeiMain.inf
@@ -76,6 +76,7 @@ [Guids]
## CONSUMES ## UNDEFINED # Locate PPI
## CONSUMES ## GUID # Used to compare with FV's file system GUID and get the FV's file system format
gEfiFirmwareFileSystem3Guid
+ gStatusCodeCallbackGuid
[Ppis]
gEfiPeiStatusCodePpiGuid ## SOMETIMES_CONSUMES # PeiReportStatusService is not ready if this PPI doesn't exist
diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index cca57c4c0686..802cd239e2eb 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -418,6 +418,22 @@ PeiCore (
ProcessPpiListFromSec ((CONST EFI_PEI_SERVICES **) &PrivateData.Ps, PpiList);
}
} else {
+ if (
+ (!(PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnBoot)) ||
+ ((PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot))
+ ) {
+ DEBUG ((DEBUG_VERBOSE, "PPI lists before temporary RAM evacuation:\n"));
+ DumpPpiList (&PrivateData);
+
+ //
+ // Migrate installed content from Temporary RAM to Permanent RAM
+ //
+ EvacuateTempRam (&PrivateData, SecCoreData);
+
+ DEBUG ((DEBUG_VERBOSE, "PPI lists after temporary RAM evacuation:\n"));
+ DumpPpiList (&PrivateData);
+ }
+
//
// Try to locate Temporary RAM Done Ppi.
//
diff --git a/MdeModulePkg/Core/Pei/Ppi/Ppi.c b/MdeModulePkg/Core/Pei/Ppi/Ppi.c
index 1ffe718c4702..018b25f86470 100644
--- a/MdeModulePkg/Core/Pei/Ppi/Ppi.c
+++ b/MdeModulePkg/Core/Pei/Ppi/Ppi.c
@@ -198,6 +198,227 @@ ConvertPpiPointers (
}
}
+/**
+
+ Migrate Notify Pointers inside an FV from temporary memory to permanent memory.
+
+ @param PrivateData Pointer to PeiCore's private data structure.
+ @param OrgFvHandle Address of FV Handle in temporary memory.
+ @param FvHandle Address of FV Handle in permanent memory.
+ @param FvSize Size of the FV.
+
+**/
+VOID
+ConvertPpiPointersFv (
+ IN PEI_CORE_INSTANCE *PrivateData,
+ IN UINTN OrgFvHandle,
+ IN UINTN FvHandle,
+ IN UINTN FvSize
+ )
+{
+ UINT8 Index;
+ UINTN Offset;
+ BOOLEAN OffsetPositive;
+ EFI_PEI_FIRMWARE_VOLUME_INFO_PPI *FvInfoPpi;
+ UINT8 GuidIndex;
+ EFI_GUID *Guid;
+ EFI_GUID *GuidCheckList[2];
+
+ GuidCheckList[0] = &gEfiPeiFirmwareVolumeInfoPpiGuid;
+ GuidCheckList[1] = &gEfiPeiFirmwareVolumeInfo2PpiGuid;
+
+ if (FvHandle > OrgFvHandle) {
+ OffsetPositive = TRUE;
+ Offset = FvHandle - OrgFvHandle;
+ } else {
+ OffsetPositive = FALSE;
+ Offset = OrgFvHandle - FvHandle;
+ }
+
+ DEBUG ((DEBUG_VERBOSE, "Converting PPI pointers in FV.\n"));
+ DEBUG ((
+ DEBUG_VERBOSE,
+ " OrgFvHandle at 0x%08x. FvHandle at 0x%08x. FvSize = 0x%x\n",
+ (UINTN) OrgFvHandle,
+ (UINTN) FvHandle,
+ FvSize
+ ));
+ DEBUG ((
+ DEBUG_VERBOSE,
+ " OrgFvHandle range: 0x%08x - 0x%08x\n",
+ OrgFvHandle,
+ OrgFvHandle + FvSize
+ ));
+
+ for (Index = 0; Index < PrivateData->PpiData.CallbackNotifyList.CurrentCount; Index++) {
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.CallbackNotifyList.NotifyPtrs[Index].Raw,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.CallbackNotifyList.NotifyPtrs[Index].Notify->Guid,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.CallbackNotifyList.NotifyPtrs[Index].Notify->Notify,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ }
+
+ for (Index = 0; Index < PrivateData->PpiData.DispatchNotifyList.CurrentCount; Index++) {
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.DispatchNotifyList.NotifyPtrs[Index].Raw,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.DispatchNotifyList.NotifyPtrs[Index].Notify->Guid,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.DispatchNotifyList.NotifyPtrs[Index].Notify->Notify,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ }
+
+ for (Index = 0; Index < PrivateData->PpiData.PpiList.CurrentCount; Index++) {
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.PpiList.PpiPtrs[Index].Raw,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.PpiList.PpiPtrs[Index].Ppi->Guid,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ ConvertPointer (
+ (VOID **) &PrivateData->PpiData.PpiList.PpiPtrs[Index].Ppi->Ppi,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+
+ Guid = PrivateData->PpiData.PpiList.PpiPtrs[Index].Ppi->Guid;
+ for (GuidIndex = 0; GuidIndex < ARRAY_SIZE (GuidCheckList); ++GuidIndex) {
+ //
+ // Don't use CompareGuid function here for performance reasons.
+ // Instead we compare the GUID as INT32 at a time and branch
+ // on the first failed comparison.
+ //
+ if ((((INT32 *)Guid)[0] == ((INT32 *)GuidCheckList[GuidIndex])[0]) &&
+ (((INT32 *)Guid)[1] == ((INT32 *)GuidCheckList[GuidIndex])[1]) &&
+ (((INT32 *)Guid)[2] == ((INT32 *)GuidCheckList[GuidIndex])[2]) &&
+ (((INT32 *)Guid)[3] == ((INT32 *)GuidCheckList[GuidIndex])[3])) {
+ FvInfoPpi = PrivateData->PpiData.PpiList.PpiPtrs[Index].Ppi->Ppi;
+ DEBUG ((DEBUG_VERBOSE, " FvInfo: %p -> ", FvInfoPpi->FvInfo));
+ if ((UINTN)FvInfoPpi->FvInfo == OrgFvHandle) {
+ ConvertPointer (
+ (VOID **)&FvInfoPpi->FvInfo,
+ OrgFvHandle,
+ OrgFvHandle + FvSize,
+ Offset,
+ OffsetPositive
+ );
+ DEBUG ((DEBUG_VERBOSE, "%p", FvInfoPpi->FvInfo));
+ }
+ DEBUG ((DEBUG_VERBOSE, "\n"));
+ break;
+ }
+ }
+ }
+}
+
+/**
+
+ Dumps the PPI lists to debug output.
+
+ @param PrivateData Points to PeiCore's private instance data.
+
+**/
+VOID
+DumpPpiList (
+ IN PEI_CORE_INSTANCE *PrivateData
+ )
+{
+ DEBUG_CODE_BEGIN ();
+ UINTN Index;
+
+ if (PrivateData == NULL) {
+ return;
+ }
+
+ for (Index = 0; Index < PrivateData->PpiData.CallbackNotifyList.CurrentCount; Index++) {
+ DEBUG ((
+ DEBUG_VERBOSE,
+ "CallbackNotify[%2d] {%g} at 0x%x (%a)\n",
+ Index,
+ PrivateData->PpiData.CallbackNotifyList.NotifyPtrs[Index].Notify->Guid,
+ (UINTN) PrivateData->PpiData.CallbackNotifyList.NotifyPtrs[Index].Raw,
+ (
+ !(
+ ((EFI_PHYSICAL_ADDRESS) (UINTN) PrivateData->PpiData.CallbackNotifyList.NotifyPtrs[Index].Raw >= PrivateData->PhysicalMemoryBegin) &&
+ (((EFI_PHYSICAL_ADDRESS) ((UINTN) PrivateData->PpiData.CallbackNotifyList.NotifyPtrs[Index].Raw) + sizeof (EFI_PEI_NOTIFY_DESCRIPTOR)) < PrivateData->FreePhysicalMemoryTop)
+ )
+ ? "CAR" : "Post-Memory"
+ )
+ ));
+ }
+ for (Index = 0; Index < PrivateData->PpiData.DispatchNotifyList.CurrentCount; Index++) {
+ DEBUG ((DEBUG_VERBOSE,
+ "DispatchNotify[%2d] {%g} at 0x%x (%a)\n",
+ Index,
+ PrivateData->PpiData.DispatchNotifyList.NotifyPtrs[Index].Notify->Guid,
+ (UINTN) PrivateData->PpiData.DispatchNotifyList.NotifyPtrs[Index].Raw,
+ (
+ !(
+ ((EFI_PHYSICAL_ADDRESS) (UINTN) PrivateData->PpiData.DispatchNotifyList.NotifyPtrs[Index].Raw >=PrivateData->PhysicalMemoryBegin) &&
+ (((EFI_PHYSICAL_ADDRESS) ((UINTN) PrivateData->PpiData.DispatchNotifyList.NotifyPtrs[Index].Raw) + sizeof (EFI_PEI_NOTIFY_DESCRIPTOR)) < PrivateData->FreePhysicalMemoryTop)
+ )
+ ? "CAR" : "Post-Memory"
+ )
+ ));
+ }
+ for (Index = 0; Index < PrivateData->PpiData.PpiList.CurrentCount; Index++) {
+ DEBUG ((DEBUG_VERBOSE,
+ "PPI[%2d] {%g} at 0x%x (%a)\n",
+ Index,
+ PrivateData->PpiData.PpiList.PpiPtrs[Index].Ppi->Guid,
+ (UINTN) PrivateData->PpiData.PpiList.PpiPtrs[Index].Raw,
+ (
+ !(
+ ((EFI_PHYSICAL_ADDRESS) (UINTN) PrivateData->PpiData.PpiList.PpiPtrs[Index].Raw >= PrivateData->PhysicalMemoryBegin) &&
+ (((EFI_PHYSICAL_ADDRESS) ((UINTN) PrivateData->PpiData.PpiList.PpiPtrs[Index].Raw) + sizeof (EFI_PEI_PPI_DESCRIPTOR)) < PrivateData->FreePhysicalMemoryTop)
+ )
+ ? "CAR" : "Post-Memory"
+ )
+ ));
+ }
+ DEBUG_CODE_END ();
+}
+
/**
This function installs an interface in the PEI PPI database by GUID.
@@ -830,3 +1051,69 @@ ProcessPpiListFromSec (
}
}
+/**
+
+ Migrate PPI Pointers of PEI_CORE from temporary memory to permanent memory.
+
+ @param PrivateData Pointer to PeiCore's private data structure.
+ @param CoreFvHandle Address of PEI_CORE FV Handle in temporary memory.
+
+**/
+
+VOID
+ConvertPeiCorePpiPointers (
+ IN PEI_CORE_INSTANCE *PrivateData,
+ PEI_CORE_FV_HANDLE CoreFvHandle
+ )
+{
+ EFI_FV_FILE_INFO FileInfo;
+ EFI_PHYSICAL_ADDRESS OrgImageBase;
+ EFI_PHYSICAL_ADDRESS MigratedImageBase;
+ UINTN PeiCoreModuleSize;
+ EFI_PEI_FILE_HANDLE PeiCoreFileHandle;
+ VOID *PeiCoreImageBase;
+ VOID *PeiCoreEntryPoint;
+ EFI_STATUS Status;
+
+ PeiCoreFileHandle = NULL;
+
+ //
+ // Find the PEI Core in the BFV in temporary memory.
+ //
+ Status = CoreFvHandle.FvPpi->FindFileByType (
+ CoreFvHandle.FvPpi,
+ EFI_FV_FILETYPE_PEI_CORE,
+ CoreFvHandle.FvHandle,
+ &PeiCoreFileHandle
+ );
+ ASSERT_EFI_ERROR (Status);
+
+ if (!EFI_ERROR (Status)) {
+ Status = CoreFvHandle.FvPpi->GetFileInfo (CoreFvHandle.FvPpi, PeiCoreFileHandle, &FileInfo);
+ ASSERT_EFI_ERROR (Status);
+
+ Status = PeiGetPe32Data (PeiCoreFileHandle, &PeiCoreImageBase);
+ ASSERT_EFI_ERROR (Status);
+
+ //
+ // Find PEI Core EntryPoint in the BFV in temporary memory.
+ //
+ Status = PeCoffLoaderGetEntryPoint ((VOID *) (UINTN) PeiCoreImageBase, &PeiCoreEntryPoint);
+ ASSERT_EFI_ERROR (Status);
+
+ OrgImageBase = (UINTN) PeiCoreImageBase;
+ MigratedImageBase = (UINTN) _ModuleEntryPoint - ((UINTN) PeiCoreEntryPoint - (UINTN) PeiCoreImageBase);
+
+ //
+ // Size of loaded PEI_CORE in permanent memory.
+ //
+ PeiCoreModuleSize = (UINTN)FileInfo.BufferSize - ((UINTN) OrgImageBase - (UINTN) FileInfo.Buffer);
+
+ //
+ // Migrate PEI_CORE PPI pointers from temporary memory to newly
+ // installed PEI_CORE in permanent memory.
+ //
+ ConvertPpiPointersFv (PrivateData, (UINTN) OrgImageBase, (UINTN) MigratedImageBase, PeiCoreModuleSize);
+ }
+}
+
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 1/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 1/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098) Guomin Jiang
@ 2020-07-03 12:22 ` Laszlo Ersek
2020-07-03 13:52 ` Laszlo Ersek
1 sibling, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 12:22 UTC (permalink / raw)
To: devel, guomin.jiang
Cc: Michael Kubacki, Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao,
Debkumar De, Harry Han, Catharine West
On 07/02/20 07:15, Guomin Jiang wrote:
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index cca57c4c0686..802cd239e2eb 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -418,6 +418,22 @@ PeiCore (
> ProcessPpiListFromSec ((CONST EFI_PEI_SERVICES **) &PrivateData.Ps, PpiList);
> }
> } else {
> + if (
> + (!(PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnBoot)) ||
> + ((PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot))
> + ) {
(1) This condition is expressed very confusingly.
First, it is unhelpful to express the condition
BootMode != BOOT_ON_S3_RESUME
as
!(BootMode == BOOT_ON_S3_RESUME)
Second, we can simplify this a lot, by selecting the PCD explicitly,
dependent on boot mode, that controls shadowing.
Put differently, if the boot mode is *not* S3, and PcdShadowPeimOnBoot
is FALSE (and so we will not shadow the PEI core), then it makes no
sense to check the boot mode *again*, on the next line.
So I suggest:
BOOLEAN Shadow;
if (PrivateData.HobList.HandoffInformationTable->BootMode ==
BOOT_ON_S3_RESUME) {
Shadow = PcdGetBool (PcdShadowPeimOnS3Boot);
} else {
Shadow = PcdGetBool (PcdShadowPeimOnBoot);
}
if (Shadow) {
//
// ...
//
}
> + DEBUG ((DEBUG_VERBOSE, "PPI lists before temporary RAM evacuation:\n"));
> + DumpPpiList (&PrivateData);
> +
> + //
> + // Migrate installed content from Temporary RAM to Permanent RAM
> + //
> + EvacuateTempRam (&PrivateData, SecCoreData);
> +
> + DEBUG ((DEBUG_VERBOSE, "PPI lists after temporary RAM evacuation:\n"));
> + DumpPpiList (&PrivateData);
> + }
> +
> //
> // Try to locate Temporary RAM Done Ppi.
> //
Thanks
Laszlo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 1/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 1/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098) Guomin Jiang
2020-07-03 12:22 ` [edk2-devel] " Laszlo Ersek
@ 2020-07-03 13:52 ` Laszlo Ersek
1 sibling, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 13:52 UTC (permalink / raw)
To: devel, guomin.jiang
Cc: Michael Kubacki, Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao,
Debkumar De, Harry Han, Catharine West
On 07/02/20 07:15, Guomin Jiang wrote:
> From: Michael Kubacki <michael.a.kubacki@intel.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> Introduces new changes to PeiCore to move the contents of temporary
> RAM visible to the PeiCore to permanent memory. This expands on
> pre-existing shadowing support in the PeiCore to perform the following
> additional actions:
>
> 1. Migrate pointers in PPIs installed in PeiCore to the permanent
> memory copy of PeiCore.
>
> 2. Copy all installed firmware volumes to permanent memory.
>
> 3. Relocate and fix up the PEIMs within the firmware volumes.
>
> 4. Convert all PPIs into the migrated firmware volume to the corresponding
> PPI address in the permanent memory location.
>
> This applies to PPIs and PEI notifications.
>
> 5. Convert all status code callbacks in the migrated firmware volume to
> the corresponding address in the permanent memory location.
>
> 6. Update the FV HOB to the corresponding firmware volume in permanent
> memory.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Harry Han <harry.han@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
> MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 402 ++++++++++++++++++
> MdeModulePkg/Core/Pei/Image/Image.c | 115 +++++
> MdeModulePkg/Core/Pei/Memory/MemoryServices.c | 82 ++++
> MdeModulePkg/Core/Pei/PeiMain.h | 168 ++++++++
> MdeModulePkg/Core/Pei/PeiMain.inf | 1 +
> MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 16 +
> MdeModulePkg/Core/Pei/Ppi/Ppi.c | 287 +++++++++++++
> 7 files changed, 1071 insertions(+)
(2) The commit message of this patch should be extended with a
simplified call tree of *all* the functions introduced. As follows:
PeiCore()
DumpPpiList()
EvacuateTempRam()
ConvertPeiCorePpiPointers()
PeiGetPe32Data()
ConvertPpiPointersFv()
MigrateSecModulesInFv()
LoadAndRelocatePeCoffImageInPlace()
MigratePeimsInFv()
MigratePeim()
PeiGetPe32Data()
LoadAndRelocatePeCoffImageInPlace()
ConvertPpiPointersFv()
ConvertStatusCodeCallbacks()
ConvertFvHob()
RemoveFvHobsInTemporaryMemory()
DumpPpiList()
Thanks
Laszlo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
2020-07-02 5:15 ` [PATCH v2 1/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-02 7:36 ` [edk2-devel] " Ni, Ray
` (2 more replies)
2020-07-02 5:15 ` [PATCH v2 3/9] UefiCpuPkg/SecMigrationPei: Add initial PEIM (CVE-2019-11098) Guomin Jiang
` (7 subsequent siblings)
9 siblings, 3 replies; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel; +Cc: Michael Kubacki, Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar
From: Michael Kubacki <michael.a.kubacki@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
Moves the GDT and IDT to permanent memory in a memory discovered
callback. This is done to ensure the GDT and IDT authenticated in
pre-memory is not fetched from outside a verified location after
the permanent memory transition.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
UefiCpuPkg/CpuMpPei/CpuMpPei.c | 40 ++++++++++++++++++-
UefiCpuPkg/CpuMpPei/CpuMpPei.h | 13 ++++++
UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++--
.../Ia32/ArchExceptionHandler.c | 4 +-
.../SecPeiCpuException.c | 2 +-
5 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
index 07ccbe7c6a91..2d6f1bc98851 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
@@ -429,6 +429,44 @@ GetGdtr (
AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
}
+/**
+ Migrates the Global Descriptor Table (GDT) to permanent memory.
+
+ @retval EFI_SUCCESS The GDT was migrated successfully.
+ @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
+
+**/
+EFI_STATUS
+EFIAPI
+MigrateGdt (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ UINTN GdtBufferSize;
+ IA32_DESCRIPTOR Gdtr;
+ UINT8 *GdtBuffer;
+
+ AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr);
+ GdtBufferSize = sizeof (IA32_TSS_DESCRIPTOR) + Gdtr.Limit + 1;
+
+ Status = PeiServicesAllocatePool (
+ GdtBufferSize,
+ (VOID **) &GdtBuffer
+ );
+ ASSERT (GdtBuffer != NULL);
+ if (EFI_ERROR (Status)) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR));
+ CopyMem ((VOID *) (UINTN) GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1);
+ Gdtr.Base = (UINT32)(UINTN) GdtBuffer;
+ AsmWriteGdtr (&Gdtr);
+
+ return EFI_SUCCESS;
+}
+
/**
Initializes CPU exceptions handlers for the sake of stack switch requirement.
@@ -644,7 +682,7 @@ InitializeCpuMpWorker (
&gEfiVectorHandoffInfoPpiGuid,
0,
NULL,
- (VOID **)&VectorHandoffInfoPpi
+ (VOID **) &VectorHandoffInfoPpi
);
if (Status == EFI_SUCCESS) {
VectorInfo = VectorHandoffInfoPpi->Info;
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
index 7d5c527d6006..5dc956409594 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
@@ -397,6 +397,19 @@ SecPlatformInformation2 (
OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2
);
+/**
+ Migrates the Global Descriptor Table (GDT) to permanent memory.
+
+ @retval EFI_SUCCESS The GDT was migrated successfully.
+ @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
+
+**/
+EFI_STATUS
+EFIAPI
+MigrateGdt (
+ VOID
+ );
+
/**
Initializes MP and exceptions handlers.
diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index a462e7ee1e38..d0cbebf70bbf 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -152,7 +152,7 @@ GetPhysicalAddressWidth (
Get the type of top level page table.
@retval Page512G PML4 paging.
- @retval Page1G PAE paing.
+ @retval Page1G PAE paging.
**/
PAGE_ATTRIBUTE
@@ -582,7 +582,7 @@ SetupStackGuardPage (
}
/**
- Enabl/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
+ Enable/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
Doing this in the memory-discovered callback is to make sure the Stack Guard
feature to cover as most PEI code as possible.
@@ -602,8 +602,14 @@ MemoryDiscoveredPpiNotifyCallback (
IN VOID *Ppi
)
{
- EFI_STATUS Status;
- BOOLEAN InitStackGuard;
+ EFI_STATUS Status;
+ BOOLEAN InitStackGuard;
+ BOOLEAN InterruptState;
+
+ InterruptState = SaveAndDisableInterrupts ();
+ Status = MigrateGdt ();
+ ASSERT_EFI_ERROR (Status);
+ SetInterruptState (InterruptState);
//
// Paging must be setup first. Otherwise the exception TSS setup during MP
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
index 1aafb7dac139..903449e0daa9 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
@@ -18,8 +18,8 @@
**/
VOID
ArchUpdateIdtEntry (
- IN IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
- IN UINTN InterruptHandler
+ OUT IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
+ IN UINTN InterruptHandler
)
{
IdtEntry->Bits.OffsetLow = (UINT16)(UINTN)InterruptHandler;
diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
index 20148db74cf8..d4ae153c5742 100644
--- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
+++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
@@ -87,7 +87,7 @@ InitializeCpuExceptionHandlers (
IdtEntryCount = (IdtDescriptor.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR);
if (IdtEntryCount > CPU_EXCEPTION_NUM) {
//
- // CPU exeption library only setup CPU_EXCEPTION_NUM exception handler at most
+ // CPU exception library only setup CPU_EXCEPTION_NUM exception handler at most
//
IdtEntryCount = CPU_EXCEPTION_NUM;
}
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 7:36 ` Ni, Ray
2020-07-03 11:36 ` Laszlo Ersek
2020-07-03 13:57 ` Laszlo Ersek
2 siblings, 0 replies; 25+ messages in thread
From: Ni, Ray @ 2020-07-02 7:36 UTC (permalink / raw)
To: devel@edk2.groups.io, Jiang, Guomin
Cc: Michael Kubacki, Dong, Eric, Laszlo Ersek, Kumar, Rahul1
Guomin,
Can you please separate the coding style change and the functionality change in different patches?
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Guomin Jiang
> Sent: Thursday, July 2, 2020 1:15 PM
> To: devel@edk2.groups.io
> Cc: Michael Kubacki <michael.a.kubacki@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> Subject: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098)
>
> From: Michael Kubacki <michael.a.kubacki@intel.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> Moves the GDT and IDT to permanent memory in a memory discovered
> callback. This is done to ensure the GDT and IDT authenticated in
> pre-memory is not fetched from outside a verified location after
> the permanent memory transition.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
> UefiCpuPkg/CpuMpPei/CpuMpPei.c | 40 ++++++++++++++++++-
> UefiCpuPkg/CpuMpPei/CpuMpPei.h | 13 ++++++
> UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++--
> .../Ia32/ArchExceptionHandler.c | 4 +-
> .../SecPeiCpuException.c | 2 +-
> 5 files changed, 65 insertions(+), 8 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> index 07ccbe7c6a91..2d6f1bc98851 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> @@ -429,6 +429,44 @@ GetGdtr (
> AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
> }
>
> +/**
> + Migrates the Global Descriptor Table (GDT) to permanent memory.
> +
> + @retval EFI_SUCCESS The GDT was migrated successfully.
> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MigrateGdt (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + UINTN GdtBufferSize;
> + IA32_DESCRIPTOR Gdtr;
> + UINT8 *GdtBuffer;
> +
> + AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr);
> + GdtBufferSize = sizeof (IA32_TSS_DESCRIPTOR) + Gdtr.Limit + 1;
> +
> + Status = PeiServicesAllocatePool (
> + GdtBufferSize,
> + (VOID **) &GdtBuffer
> + );
> + ASSERT (GdtBuffer != NULL);
> + if (EFI_ERROR (Status)) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR));
> + CopyMem ((VOID *) (UINTN) GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1);
> + Gdtr.Base = (UINT32)(UINTN) GdtBuffer;
> + AsmWriteGdtr (&Gdtr);
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> Initializes CPU exceptions handlers for the sake of stack switch requirement.
>
> @@ -644,7 +682,7 @@ InitializeCpuMpWorker (
> &gEfiVectorHandoffInfoPpiGuid,
> 0,
> NULL,
> - (VOID **)&VectorHandoffInfoPpi
> + (VOID **) &VectorHandoffInfoPpi
> );
> if (Status == EFI_SUCCESS) {
> VectorInfo = VectorHandoffInfoPpi->Info;
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> index 7d5c527d6006..5dc956409594 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> @@ -397,6 +397,19 @@ SecPlatformInformation2 (
> OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2
> );
>
> +/**
> + Migrates the Global Descriptor Table (GDT) to permanent memory.
> +
> + @retval EFI_SUCCESS The GDT was migrated successfully.
> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MigrateGdt (
> + VOID
> + );
> +
> /**
> Initializes MP and exceptions handlers.
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index a462e7ee1e38..d0cbebf70bbf 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -152,7 +152,7 @@ GetPhysicalAddressWidth (
> Get the type of top level page table.
>
> @retval Page512G PML4 paging.
> - @retval Page1G PAE paing.
> + @retval Page1G PAE paging.
>
> **/
> PAGE_ATTRIBUTE
> @@ -582,7 +582,7 @@ SetupStackGuardPage (
> }
>
> /**
> - Enabl/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
> + Enable/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
>
> Doing this in the memory-discovered callback is to make sure the Stack Guard
> feature to cover as most PEI code as possible.
> @@ -602,8 +602,14 @@ MemoryDiscoveredPpiNotifyCallback (
> IN VOID *Ppi
> )
> {
> - EFI_STATUS Status;
> - BOOLEAN InitStackGuard;
> + EFI_STATUS Status;
> + BOOLEAN InitStackGuard;
> + BOOLEAN InterruptState;
> +
> + InterruptState = SaveAndDisableInterrupts ();
> + Status = MigrateGdt ();
> + ASSERT_EFI_ERROR (Status);
> + SetInterruptState (InterruptState);
>
> //
> // Paging must be setup first. Otherwise the exception TSS setup during MP
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> index 1aafb7dac139..903449e0daa9 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> @@ -18,8 +18,8 @@
> **/
> VOID
> ArchUpdateIdtEntry (
> - IN IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
> - IN UINTN InterruptHandler
> + OUT IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
> + IN UINTN InterruptHandler
> )
> {
> IdtEntry->Bits.OffsetLow = (UINT16)(UINTN)InterruptHandler;
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> index 20148db74cf8..d4ae153c5742 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> @@ -87,7 +87,7 @@ InitializeCpuExceptionHandlers (
> IdtEntryCount = (IdtDescriptor.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR);
> if (IdtEntryCount > CPU_EXCEPTION_NUM) {
> //
> - // CPU exeption library only setup CPU_EXCEPTION_NUM exception handler at most
> + // CPU exception library only setup CPU_EXCEPTION_NUM exception handler at most
> //
> IdtEntryCount = CPU_EXCEPTION_NUM;
> }
> --
> 2.25.1.windows.1
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098) Guomin Jiang
2020-07-02 7:36 ` [edk2-devel] " Ni, Ray
@ 2020-07-03 11:36 ` Laszlo Ersek
2020-07-03 11:52 ` Laszlo Ersek
2020-07-03 13:57 ` Laszlo Ersek
2 siblings, 1 reply; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 11:36 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Michael Kubacki, Eric Dong, Ray Ni, Rahul Kumar
Hi,
this patch contains a bunch of changes that are not related to the main
purpose of the patch. See below.
On 07/02/20 07:15, Guomin Jiang wrote:
> From: Michael Kubacki <michael.a.kubacki@intel.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> Moves the GDT and IDT to permanent memory in a memory discovered
> callback. This is done to ensure the GDT and IDT authenticated in
> pre-memory is not fetched from outside a verified location after
> the permanent memory transition.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
> UefiCpuPkg/CpuMpPei/CpuMpPei.c | 40 ++++++++++++++++++-
> UefiCpuPkg/CpuMpPei/CpuMpPei.h | 13 ++++++
> UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++--
> .../Ia32/ArchExceptionHandler.c | 4 +-
> .../SecPeiCpuException.c | 2 +-
> 5 files changed, 65 insertions(+), 8 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> index 07ccbe7c6a91..2d6f1bc98851 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> @@ -429,6 +429,44 @@ GetGdtr (
> AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
> }
>
> +/**
> + Migrates the Global Descriptor Table (GDT) to permanent memory.
> +
> + @retval EFI_SUCCESS The GDT was migrated successfully.
> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MigrateGdt (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + UINTN GdtBufferSize;
> + IA32_DESCRIPTOR Gdtr;
> + UINT8 *GdtBuffer;
> +
> + AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr);
> + GdtBufferSize = sizeof (IA32_TSS_DESCRIPTOR) + Gdtr.Limit + 1;
> +
> + Status = PeiServicesAllocatePool (
> + GdtBufferSize,
> + (VOID **) &GdtBuffer
> + );
> + ASSERT (GdtBuffer != NULL);
> + if (EFI_ERROR (Status)) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR));
> + CopyMem ((VOID *) (UINTN) GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1);
> + Gdtr.Base = (UINT32)(UINTN) GdtBuffer;
> + AsmWriteGdtr (&Gdtr);
> +
> + return EFI_SUCCESS;
> +}
> +
So this hunk relates to the main purpose of the patch; OK.
> /**
> Initializes CPU exceptions handlers for the sake of stack switch requirement.
>
> @@ -644,7 +682,7 @@ InitializeCpuMpWorker (
> &gEfiVectorHandoffInfoPpiGuid,
> 0,
> NULL,
> - (VOID **)&VectorHandoffInfoPpi
> + (VOID **) &VectorHandoffInfoPpi
> );
> if (Status == EFI_SUCCESS) {
> VectorInfo = VectorHandoffInfoPpi->Info;
This change is both useless and totally unrelated to the patch.
(1) Please drop this hunk.
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> index 7d5c527d6006..5dc956409594 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> @@ -397,6 +397,19 @@ SecPlatformInformation2 (
> OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2
> );
>
> +/**
> + Migrates the Global Descriptor Table (GDT) to permanent memory.
> +
> + @retval EFI_SUCCESS The GDT was migrated successfully.
> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MigrateGdt (
> + VOID
> + );
> +
> /**
> Initializes MP and exceptions handlers.
>
(2) There's no need to declare this function as EFIAPI. Using EFIAPI is
confusing, because it suggests that the interface is called between
modules. But that's not the case, as far as I can tell. Please drop EFIAPI.
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index a462e7ee1e38..d0cbebf70bbf 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -152,7 +152,7 @@ GetPhysicalAddressWidth (
> Get the type of top level page table.
>
> @retval Page512G PML4 paging.
> - @retval Page1G PAE paing.
> + @retval Page1G PAE paging.
>
> **/
> PAGE_ATTRIBUTE
> @@ -582,7 +582,7 @@ SetupStackGuardPage (
> }
>
> /**
> - Enabl/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
> + Enable/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
>
> Doing this in the memory-discovered callback is to make sure the Stack Guard
> feature to cover as most PEI code as possible.
(3) These changes are valid (they are typo fixes), but they definitely
belong to a separate patch. Please split them off.
> @@ -602,8 +602,14 @@ MemoryDiscoveredPpiNotifyCallback (
> IN VOID *Ppi
> )
> {
> - EFI_STATUS Status;
> - BOOLEAN InitStackGuard;
> + EFI_STATUS Status;
> + BOOLEAN InitStackGuard;
> + BOOLEAN InterruptState;
> +
> + InterruptState = SaveAndDisableInterrupts ();
> + Status = MigrateGdt ();
> + ASSERT_EFI_ERROR (Status);
> + SetInterruptState (InterruptState);
>
> //
> // Paging must be setup first. Otherwise the exception TSS setup during MP
This does belong here, OK.
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> index 1aafb7dac139..903449e0daa9 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> @@ -18,8 +18,8 @@
> **/
> VOID
> ArchUpdateIdtEntry (
> - IN IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
> - IN UINTN InterruptHandler
> + OUT IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
> + IN UINTN InterruptHandler
> )
> {
> IdtEntry->Bits.OffsetLow = (UINT16)(UINTN)InterruptHandler;
(4) Please split this off to a separate patch.
(It's OK to create just one other patch named "UefiCpuPkg/CpuMpPei:
trivial cleanups", and to move the IN/OUT update and the typo fixes to
that patch. I'm not requesting that every trivial update be placed in
its own patch, just that the trivial updates be kept separate from a
patch that is supposed to fix a CVE.)
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> index 20148db74cf8..d4ae153c5742 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> @@ -87,7 +87,7 @@ InitializeCpuExceptionHandlers (
> IdtEntryCount = (IdtDescriptor.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR);
> if (IdtEntryCount > CPU_EXCEPTION_NUM) {
> //
> - // CPU exeption library only setup CPU_EXCEPTION_NUM exception handler at most
> + // CPU exception library only setup CPU_EXCEPTION_NUM exception handler at most
> //
> IdtEntryCount = CPU_EXCEPTION_NUM;
> }
>
(5) Same as (3).
Thanks
Laszlo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098)
2020-07-03 11:36 ` Laszlo Ersek
@ 2020-07-03 11:52 ` Laszlo Ersek
0 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 11:52 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Michael Kubacki, Eric Dong, Ray Ni, Rahul Kumar
Hi,
more comments on the MigrateGdt() function:
On 07/03/20 13:36, Laszlo Ersek wrote:
> Hi,
>
> this patch contains a bunch of changes that are not related to the main
> purpose of the patch. See below.
>
> On 07/02/20 07:15, Guomin Jiang wrote:
>> From: Michael Kubacki <michael.a.kubacki@intel.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>>
>> Moves the GDT and IDT to permanent memory in a memory discovered
>> callback. This is done to ensure the GDT and IDT authenticated in
>> pre-memory is not fetched from outside a verified location after
>> the permanent memory transition.
>>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
>> ---
>> UefiCpuPkg/CpuMpPei/CpuMpPei.c | 40 ++++++++++++++++++-
>> UefiCpuPkg/CpuMpPei/CpuMpPei.h | 13 ++++++
>> UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++--
>> .../Ia32/ArchExceptionHandler.c | 4 +-
>> .../SecPeiCpuException.c | 2 +-
>> 5 files changed, 65 insertions(+), 8 deletions(-)
>>
>> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>> index 07ccbe7c6a91..2d6f1bc98851 100644
>> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>> @@ -429,6 +429,44 @@ GetGdtr (
>> AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
>> }
>>
>> +/**
>> + Migrates the Global Descriptor Table (GDT) to permanent memory.
>> +
>> + @retval EFI_SUCCESS The GDT was migrated successfully.
>> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +MigrateGdt (
>> + VOID
>> + )
>> +{
>> + EFI_STATUS Status;
>> + UINTN GdtBufferSize;
>> + IA32_DESCRIPTOR Gdtr;
>> + UINT8 *GdtBuffer;
>> +
>> + AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr);
>> + GdtBufferSize = sizeof (IA32_TSS_DESCRIPTOR) + Gdtr.Limit + 1;
(6) I don't understand why IA32_TSS_DESCRIPTOR is used in this function.
While technically it should not cause problems, "TSS" seems completely
irrelevant here. Please use IA32_SEGMENT_DESCRIPTOR instead.
(7) The buffer size is too large. "Gdtr.Limit" is an inclusive limit
indeed, so the "+1" at the end is justified, for determining the size.
However, for the alignment, we only need *at most*
sizeof (IA32_SEGMENT_DESCRIPTOR) - 1
bytes. (If the allocation is immediately well-aligned, then we need 0
bytes for ensuring the proper alignment, and not "sizeof
(IA32_SEGMENT_DESCRIPTOR)" bytes.)
So I suggest
GdtBufferSize = sizeof (IA32_SEGMENT_DESCRIPTOR) - 1 + Gdtr.Limit + 1;
>> +
>> + Status = PeiServicesAllocatePool (
>> + GdtBufferSize,
>> + (VOID **) &GdtBuffer
>> + );
(8) We have way too much casting back and forth here. Just make
"GdtBuffer" a (VOID*), and then you can drop this cast.
>> + ASSERT (GdtBuffer != NULL);
>> + if (EFI_ERROR (Status)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> +
>> + GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR));
(9) Same as (6).
>> + CopyMem ((VOID *) (UINTN) GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1);
(10) Same as (8); drop the (VOID *)(UINTN) casts, please.
>> + Gdtr.Base = (UINT32)(UINTN) GdtBuffer;
(11) The (UINT32) cast is wrong, please drop it. "Gdtr.Base" has type UINTN.
Thanks
Laszlo
>> + AsmWriteGdtr (&Gdtr);
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>
> So this hunk relates to the main purpose of the patch; OK.
>
>> /**
>> Initializes CPU exceptions handlers for the sake of stack switch requirement.
>>
>> @@ -644,7 +682,7 @@ InitializeCpuMpWorker (
>> &gEfiVectorHandoffInfoPpiGuid,
>> 0,
>> NULL,
>> - (VOID **)&VectorHandoffInfoPpi
>> + (VOID **) &VectorHandoffInfoPpi
>> );
>> if (Status == EFI_SUCCESS) {
>> VectorInfo = VectorHandoffInfoPpi->Info;
>
> This change is both useless and totally unrelated to the patch.
>
> (1) Please drop this hunk.
>
>> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
>> index 7d5c527d6006..5dc956409594 100644
>> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
>> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
>> @@ -397,6 +397,19 @@ SecPlatformInformation2 (
>> OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2
>> );
>>
>> +/**
>> + Migrates the Global Descriptor Table (GDT) to permanent memory.
>> +
>> + @retval EFI_SUCCESS The GDT was migrated successfully.
>> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +MigrateGdt (
>> + VOID
>> + );
>> +
>> /**
>> Initializes MP and exceptions handlers.
>>
>
> (2) There's no need to declare this function as EFIAPI. Using EFIAPI is
> confusing, because it suggests that the interface is called between
> modules. But that's not the case, as far as I can tell. Please drop EFIAPI.
>
>> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
>> index a462e7ee1e38..d0cbebf70bbf 100644
>> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
>> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
>> @@ -152,7 +152,7 @@ GetPhysicalAddressWidth (
>> Get the type of top level page table.
>>
>> @retval Page512G PML4 paging.
>> - @retval Page1G PAE paing.
>> + @retval Page1G PAE paging.
>>
>> **/
>> PAGE_ATTRIBUTE
>> @@ -582,7 +582,7 @@ SetupStackGuardPage (
>> }
>>
>> /**
>> - Enabl/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
>> + Enable/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
>>
>> Doing this in the memory-discovered callback is to make sure the Stack Guard
>> feature to cover as most PEI code as possible.
>
> (3) These changes are valid (they are typo fixes), but they definitely
> belong to a separate patch. Please split them off.
>
>
>> @@ -602,8 +602,14 @@ MemoryDiscoveredPpiNotifyCallback (
>> IN VOID *Ppi
>> )
>> {
>> - EFI_STATUS Status;
>> - BOOLEAN InitStackGuard;
>> + EFI_STATUS Status;
>> + BOOLEAN InitStackGuard;
>> + BOOLEAN InterruptState;
>> +
>> + InterruptState = SaveAndDisableInterrupts ();
>> + Status = MigrateGdt ();
>> + ASSERT_EFI_ERROR (Status);
>> + SetInterruptState (InterruptState);
>>
>> //
>> // Paging must be setup first. Otherwise the exception TSS setup during MP
>
> This does belong here, OK.
>
>> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
>> index 1aafb7dac139..903449e0daa9 100644
>> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
>> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
>> @@ -18,8 +18,8 @@
>> **/
>> VOID
>> ArchUpdateIdtEntry (
>> - IN IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
>> - IN UINTN InterruptHandler
>> + OUT IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
>> + IN UINTN InterruptHandler
>> )
>> {
>> IdtEntry->Bits.OffsetLow = (UINT16)(UINTN)InterruptHandler;
>
> (4) Please split this off to a separate patch.
>
> (It's OK to create just one other patch named "UefiCpuPkg/CpuMpPei:
> trivial cleanups", and to move the IN/OUT update and the typo fixes to
> that patch. I'm not requesting that every trivial update be placed in
> its own patch, just that the trivial updates be kept separate from a
> patch that is supposed to fix a CVE.)
>
>> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
>> index 20148db74cf8..d4ae153c5742 100644
>> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
>> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
>> @@ -87,7 +87,7 @@ InitializeCpuExceptionHandlers (
>> IdtEntryCount = (IdtDescriptor.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR);
>> if (IdtEntryCount > CPU_EXCEPTION_NUM) {
>> //
>> - // CPU exeption library only setup CPU_EXCEPTION_NUM exception handler at most
>> + // CPU exception library only setup CPU_EXCEPTION_NUM exception handler at most
>> //
>> IdtEntryCount = CPU_EXCEPTION_NUM;
>> }
>>
>
> (5) Same as (3).
>
> Thanks
> Laszlo
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098) Guomin Jiang
2020-07-02 7:36 ` [edk2-devel] " Ni, Ray
2020-07-03 11:36 ` Laszlo Ersek
@ 2020-07-03 13:57 ` Laszlo Ersek
2020-07-03 14:33 ` Laszlo Ersek
2 siblings, 1 reply; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 13:57 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Michael Kubacki, Eric Dong, Ray Ni, Rahul Kumar
On 07/02/20 07:15, Guomin Jiang wrote:
> From: Michael Kubacki <michael.a.kubacki@intel.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> Moves the GDT and IDT to permanent memory in a memory discovered
> callback. This is done to ensure the GDT and IDT authenticated in
> pre-memory is not fetched from outside a verified location after
> the permanent memory transition.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
> UefiCpuPkg/CpuMpPei/CpuMpPei.c | 40 ++++++++++++++++++-
> UefiCpuPkg/CpuMpPei/CpuMpPei.h | 13 ++++++
> UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++--
> .../Ia32/ArchExceptionHandler.c | 4 +-
> .../SecPeiCpuException.c | 2 +-
> 5 files changed, 65 insertions(+), 8 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> index 07ccbe7c6a91..2d6f1bc98851 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
> @@ -429,6 +429,44 @@ GetGdtr (
> AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
> }
>
> +/**
> + Migrates the Global Descriptor Table (GDT) to permanent memory.
> +
> + @retval EFI_SUCCESS The GDT was migrated successfully.
> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MigrateGdt (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + UINTN GdtBufferSize;
> + IA32_DESCRIPTOR Gdtr;
> + UINT8 *GdtBuffer;
> +
> + AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr);
> + GdtBufferSize = sizeof (IA32_TSS_DESCRIPTOR) + Gdtr.Limit + 1;
> +
> + Status = PeiServicesAllocatePool (
> + GdtBufferSize,
> + (VOID **) &GdtBuffer
> + );
> + ASSERT (GdtBuffer != NULL);
> + if (EFI_ERROR (Status)) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR));
> + CopyMem ((VOID *) (UINTN) GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1);
> + Gdtr.Base = (UINT32)(UINTN) GdtBuffer;
> + AsmWriteGdtr (&Gdtr);
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> Initializes CPU exceptions handlers for the sake of stack switch requirement.
>
> @@ -644,7 +682,7 @@ InitializeCpuMpWorker (
> &gEfiVectorHandoffInfoPpiGuid,
> 0,
> NULL,
> - (VOID **)&VectorHandoffInfoPpi
> + (VOID **) &VectorHandoffInfoPpi
> );
> if (Status == EFI_SUCCESS) {
> VectorInfo = VectorHandoffInfoPpi->Info;
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> index 7d5c527d6006..5dc956409594 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
> @@ -397,6 +397,19 @@ SecPlatformInformation2 (
> OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2
> );
>
> +/**
> + Migrates the Global Descriptor Table (GDT) to permanent memory.
> +
> + @retval EFI_SUCCESS The GDT was migrated successfully.
> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MigrateGdt (
> + VOID
> + );
> +
> /**
> Initializes MP and exceptions handlers.
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index a462e7ee1e38..d0cbebf70bbf 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -152,7 +152,7 @@ GetPhysicalAddressWidth (
> Get the type of top level page table.
>
> @retval Page512G PML4 paging.
> - @retval Page1G PAE paing.
> + @retval Page1G PAE paging.
>
> **/
> PAGE_ATTRIBUTE
> @@ -582,7 +582,7 @@ SetupStackGuardPage (
> }
>
> /**
> - Enabl/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
> + Enable/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
>
> Doing this in the memory-discovered callback is to make sure the Stack Guard
> feature to cover as most PEI code as possible.
> @@ -602,8 +602,14 @@ MemoryDiscoveredPpiNotifyCallback (
> IN VOID *Ppi
> )
> {
> - EFI_STATUS Status;
> - BOOLEAN InitStackGuard;
> + EFI_STATUS Status;
> + BOOLEAN InitStackGuard;
> + BOOLEAN InterruptState;
> +
> + InterruptState = SaveAndDisableInterrupts ();
> + Status = MigrateGdt ();
> + ASSERT_EFI_ERROR (Status);
> + SetInterruptState (InterruptState);
>
> //
> // Paging must be setup first. Otherwise the exception TSS setup during MP
(12) The GDT migration should be made dependent on
"PcdMigrateTemporaryRamFirmwareVolumes", shouldn't it?
Thanks
Laszlo
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> index 1aafb7dac139..903449e0daa9 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
> @@ -18,8 +18,8 @@
> **/
> VOID
> ArchUpdateIdtEntry (
> - IN IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
> - IN UINTN InterruptHandler
> + OUT IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
> + IN UINTN InterruptHandler
> )
> {
> IdtEntry->Bits.OffsetLow = (UINT16)(UINTN)InterruptHandler;
> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> index 20148db74cf8..d4ae153c5742 100644
> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
> @@ -87,7 +87,7 @@ InitializeCpuExceptionHandlers (
> IdtEntryCount = (IdtDescriptor.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR);
> if (IdtEntryCount > CPU_EXCEPTION_NUM) {
> //
> - // CPU exeption library only setup CPU_EXCEPTION_NUM exception handler at most
> + // CPU exception library only setup CPU_EXCEPTION_NUM exception handler at most
> //
> IdtEntryCount = CPU_EXCEPTION_NUM;
> }
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098)
2020-07-03 13:57 ` Laszlo Ersek
@ 2020-07-03 14:33 ` Laszlo Ersek
0 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 14:33 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Michael Kubacki, Eric Dong, Ray Ni, Rahul Kumar
On 07/03/20 15:57, Laszlo Ersek wrote:
> On 07/02/20 07:15, Guomin Jiang wrote:
>> From: Michael Kubacki <michael.a.kubacki@intel.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>>
>> Moves the GDT and IDT to permanent memory in a memory discovered
>> callback. This is done to ensure the GDT and IDT authenticated in
>> pre-memory is not fetched from outside a verified location after
>> the permanent memory transition.
>>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
>> ---
>> UefiCpuPkg/CpuMpPei/CpuMpPei.c | 40 ++++++++++++++++++-
>> UefiCpuPkg/CpuMpPei/CpuMpPei.h | 13 ++++++
>> UefiCpuPkg/CpuMpPei/CpuPaging.c | 14 +++++--
>> .../Ia32/ArchExceptionHandler.c | 4 +-
>> .../SecPeiCpuException.c | 2 +-
>> 5 files changed, 65 insertions(+), 8 deletions(-)
>>
>> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>> index 07ccbe7c6a91..2d6f1bc98851 100644
>> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c
>> @@ -429,6 +429,44 @@ GetGdtr (
>> AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer);
>> }
>>
>> +/**
>> + Migrates the Global Descriptor Table (GDT) to permanent memory.
>> +
>> + @retval EFI_SUCCESS The GDT was migrated successfully.
>> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +MigrateGdt (
>> + VOID
>> + )
>> +{
>> + EFI_STATUS Status;
>> + UINTN GdtBufferSize;
>> + IA32_DESCRIPTOR Gdtr;
>> + UINT8 *GdtBuffer;
>> +
>> + AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr);
>> + GdtBufferSize = sizeof (IA32_TSS_DESCRIPTOR) + Gdtr.Limit + 1;
>> +
>> + Status = PeiServicesAllocatePool (
>> + GdtBufferSize,
>> + (VOID **) &GdtBuffer
>> + );
>> + ASSERT (GdtBuffer != NULL);
>> + if (EFI_ERROR (Status)) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> +
>> + GdtBuffer = ALIGN_POINTER (GdtBuffer, sizeof (IA32_TSS_DESCRIPTOR));
>> + CopyMem ((VOID *) (UINTN) GdtBuffer, (VOID *) Gdtr.Base, Gdtr.Limit + 1);
>> + Gdtr.Base = (UINT32)(UINTN) GdtBuffer;
>> + AsmWriteGdtr (&Gdtr);
>> +
>> + return EFI_SUCCESS;
>> +}
>> +
>> /**
>> Initializes CPU exceptions handlers for the sake of stack switch requirement.
>>
>> @@ -644,7 +682,7 @@ InitializeCpuMpWorker (
>> &gEfiVectorHandoffInfoPpiGuid,
>> 0,
>> NULL,
>> - (VOID **)&VectorHandoffInfoPpi
>> + (VOID **) &VectorHandoffInfoPpi
>> );
>> if (Status == EFI_SUCCESS) {
>> VectorInfo = VectorHandoffInfoPpi->Info;
>> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
>> index 7d5c527d6006..5dc956409594 100644
>> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h
>> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h
>> @@ -397,6 +397,19 @@ SecPlatformInformation2 (
>> OUT EFI_SEC_PLATFORM_INFORMATION_RECORD2 *PlatformInformationRecord2
>> );
>>
>> +/**
>> + Migrates the Global Descriptor Table (GDT) to permanent memory.
>> +
>> + @retval EFI_SUCCESS The GDT was migrated successfully.
>> + @retval EFI_OUT_OF_RESOURCES The GDT could not be migrated due to lack of available memory.
>> +
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +MigrateGdt (
>> + VOID
>> + );
>> +
>> /**
>> Initializes MP and exceptions handlers.
>>
>> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
>> index a462e7ee1e38..d0cbebf70bbf 100644
>> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
>> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
>> @@ -152,7 +152,7 @@ GetPhysicalAddressWidth (
>> Get the type of top level page table.
>>
>> @retval Page512G PML4 paging.
>> - @retval Page1G PAE paing.
>> + @retval Page1G PAE paging.
>>
>> **/
>> PAGE_ATTRIBUTE
>> @@ -582,7 +582,7 @@ SetupStackGuardPage (
>> }
>>
>> /**
>> - Enabl/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
>> + Enable/setup stack guard for each processor if PcdCpuStackGuard is set to TRUE.
>>
>> Doing this in the memory-discovered callback is to make sure the Stack Guard
>> feature to cover as most PEI code as possible.
>> @@ -602,8 +602,14 @@ MemoryDiscoveredPpiNotifyCallback (
>> IN VOID *Ppi
>> )
>> {
>> - EFI_STATUS Status;
>> - BOOLEAN InitStackGuard;
>> + EFI_STATUS Status;
>> + BOOLEAN InitStackGuard;
>> + BOOLEAN InterruptState;
>> +
>> + InterruptState = SaveAndDisableInterrupts ();
>> + Status = MigrateGdt ();
>> + ASSERT_EFI_ERROR (Status);
>> + SetInterruptState (InterruptState);
>>
>> //
>> // Paging must be setup first. Otherwise the exception TSS setup during MP
>
> (12) The GDT migration should be made dependent on
> "PcdMigrateTemporaryRamFirmwareVolumes", shouldn't it?
... Or is this *another* preexistent bug that we should fix regardless
of the "temporary RAM evacuation" feature?
I mean, considering current master, once we switch to permanent PEI RAM,
do we still rely on a GDT that lives in temp RAM?
If that's the case, then we should even split this series into two
series. The first series should fix the other issues first -- typos,
IN/OUT mistakes, this GDT problem, and the S3 shadowing bug in the DXE
IPL PEIM.
Once all that is done, we can introduce
"PcdMigrateTemporaryRamFirmwareVolumes", and the dependent new feature.
Thanks
Laszlo
>> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
>> index 1aafb7dac139..903449e0daa9 100644
>> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
>> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c
>> @@ -18,8 +18,8 @@
>> **/
>> VOID
>> ArchUpdateIdtEntry (
>> - IN IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
>> - IN UINTN InterruptHandler
>> + OUT IA32_IDT_GATE_DESCRIPTOR *IdtEntry,
>> + IN UINTN InterruptHandler
>> )
>> {
>> IdtEntry->Bits.OffsetLow = (UINT16)(UINTN)InterruptHandler;
>> diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
>> index 20148db74cf8..d4ae153c5742 100644
>> --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
>> +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuException.c
>> @@ -87,7 +87,7 @@ InitializeCpuExceptionHandlers (
>> IdtEntryCount = (IdtDescriptor.Limit + 1) / sizeof (IA32_IDT_GATE_DESCRIPTOR);
>> if (IdtEntryCount > CPU_EXCEPTION_NUM) {
>> //
>> - // CPU exeption library only setup CPU_EXCEPTION_NUM exception handler at most
>> + // CPU exception library only setup CPU_EXCEPTION_NUM exception handler at most
>> //
>> IdtEntryCount = CPU_EXCEPTION_NUM;
>> }
>>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 3/9] UefiCpuPkg/SecMigrationPei: Add initial PEIM (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
2020-07-02 5:15 ` [PATCH v2 1/9] MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore (CVE-2019-11098) Guomin Jiang
2020-07-02 5:15 ` [PATCH v2 2/9] UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-03 11:38 ` [edk2-devel] " Laszlo Ersek
2020-07-02 5:15 ` [PATCH v2 4/9] MdeModulePkg/DxeIplPeim: Register for shadow on S3 shadowed boot (CVE-2019-11098) Guomin Jiang
` (6 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel
Cc: Michael Kubacki, Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar,
Debkumar De, Harry Han, Catharine West
From: Michael Kubacki <michael.a.kubacki@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
Adds a PEIM that republishes structures produced in SEC. This
is done because SEC modules may not be shadowed in some platforms
due to space constraints or special alignment requirements. The
SecMigrationPei module locates interfaces that may be published in
SEC and reinstalls the interface with permanent memory addresses.
This is important if pre-memory address access is forbidden after
memory initialization and data such as a PPI descriptor, PPI GUID,
or PPI inteface reside in pre-memory.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
---
UefiCpuPkg/Include/Ppi/RepublishSecPpi.h | 54 +++
UefiCpuPkg/SecCore/SecCore.inf | 2 +
UefiCpuPkg/SecCore/SecMain.c | 26 +-
UefiCpuPkg/SecCore/SecMain.h | 1 +
UefiCpuPkg/SecMigrationPei/SecMigrationPei.c | 372 ++++++++++++++++++
UefiCpuPkg/SecMigrationPei/SecMigrationPei.h | 170 ++++++++
.../SecMigrationPei/SecMigrationPei.inf | 64 +++
.../SecMigrationPei/SecMigrationPei.uni | 13 +
UefiCpuPkg/UefiCpuPkg.dec | 4 +
UefiCpuPkg/UefiCpuPkg.dsc | 1 +
10 files changed, 705 insertions(+), 2 deletions(-)
create mode 100644 UefiCpuPkg/Include/Ppi/RepublishSecPpi.h
create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.h
create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni
diff --git a/UefiCpuPkg/Include/Ppi/RepublishSecPpi.h b/UefiCpuPkg/Include/Ppi/RepublishSecPpi.h
new file mode 100644
index 000000000000..6fb9f1b005b4
--- /dev/null
+++ b/UefiCpuPkg/Include/Ppi/RepublishSecPpi.h
@@ -0,0 +1,54 @@
+/** @file
+ This file declares Sec Platform Information PPI.
+
+ This service is the primary handoff state into the PEI Foundation.
+ The Security (SEC) component creates the early, transitory memory
+ environment and also encapsulates knowledge of at least the
+ location of the Boot Firmware Volume (BFV).
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Revision Reference:
+ This PPI is introduced in PI Version 1.0.
+
+**/
+
+#ifndef __REPUBLISH_SEC_PPI_H__
+#define __REPUBLISH_SEC_PPI_H__
+
+#include <Pi/PiPeiCis.h>
+
+#define REPUBLISH_SEC_PPI_PPI_GUID \
+ { \
+ 0x27a71b1e, 0x73ee, 0x43d6, { 0xac, 0xe3, 0x52, 0x1a, 0x2d, 0xc5, 0xd0, 0x92 } \
+ }
+
+typedef struct _REPUBLISH_SEC_PPI_PPI REPUBLISH_SEC_PPI_PPI;
+
+/**
+ This interface re-installs PPIs installed in SecCore from a post-memory PEIM.
+
+ This is to allow a platform that may not support relocation of SecCore to update the PPI instance to a post-memory
+ copy from a PEIM that has been shadowed to permanent memory.
+
+ @retval EFI_SUCCESS The SecCore PPIs were re-installed successfully.
+ @retval Others An error occurred re-installing the SecCore PPIs.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *REPUBLISH_SEC_PPI_REPUBLISH_SEC_PPIS)(
+ VOID
+ );
+
+///
+///
+///
+struct _REPUBLISH_SEC_PPI_PPI {
+ REPUBLISH_SEC_PPI_REPUBLISH_SEC_PPIS RepublishSecPpis;
+};
+
+extern EFI_GUID gRepublishSecPpiPpiGuid;
+
+#endif
diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
index 0562820c95e0..545781d6b4b3 100644
--- a/UefiCpuPkg/SecCore/SecCore.inf
+++ b/UefiCpuPkg/SecCore/SecCore.inf
@@ -68,6 +68,8 @@ [Ppis]
## SOMETIMES_CONSUMES
gPeiSecPerformancePpiGuid
gEfiPeiCoreFvLocationPpiGuid
+ ## CONSUMES
+ gRepublishSecPpiPpiGuid
[Guids]
## SOMETIMES_PRODUCES ## HOB
diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
index 5d5e7f17dced..155be49a6011 100644
--- a/UefiCpuPkg/SecCore/SecMain.c
+++ b/UefiCpuPkg/SecCore/SecMain.c
@@ -370,13 +370,35 @@ SecTemporaryRamDone (
VOID
)
{
- BOOLEAN State;
+ EFI_STATUS Status;
+ EFI_STATUS Status2;
+ UINTN Index;
+ BOOLEAN State;
+ EFI_PEI_PPI_DESCRIPTOR *PeiPpiDescriptor;
+ REPUBLISH_SEC_PPI_PPI *RepublishSecPpiPpi;
//
// Republish Sec Platform Information(2) PPI
//
RepublishSecPlatformInformationPpi ();
+ //
+ // Re-install SEC PPIs using a PEIM produced service if published
+ //
+ for (Index = 0, Status = EFI_SUCCESS; Status == EFI_SUCCESS; Index++) {
+ Status = PeiServicesLocatePpi (
+ &gRepublishSecPpiPpiGuid,
+ Index,
+ &PeiPpiDescriptor,
+ (VOID **) &RepublishSecPpiPpi
+ );
+ if (!EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Calling RepublishSecPpi instance %d.\n", Index));
+ Status2 = RepublishSecPpiPpi->RepublishSecPpis ();
+ ASSERT_EFI_ERROR (Status2);
+ }
+ }
+
//
// Migrate DebugAgentContext.
//
@@ -385,7 +407,7 @@ SecTemporaryRamDone (
//
// Disable interrupts and save current interrupt state
//
- State = SaveAndDisableInterrupts();
+ State = SaveAndDisableInterrupts ();
//
// Disable Temporary RAM after Stack and Heap have been migrated at this point.
diff --git a/UefiCpuPkg/SecCore/SecMain.h b/UefiCpuPkg/SecCore/SecMain.h
index e8c05d713668..e20bcf86532c 100644
--- a/UefiCpuPkg/SecCore/SecMain.h
+++ b/UefiCpuPkg/SecCore/SecMain.h
@@ -15,6 +15,7 @@
#include <Ppi/TemporaryRamDone.h>
#include <Ppi/SecPerformance.h>
#include <Ppi/PeiCoreFvLocation.h>
+#include <Ppi/RepublishSecPpi.h>
#include <Guid/FirmwarePerformance.h>
diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
new file mode 100644
index 000000000000..f96013b09b21
--- /dev/null
+++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
@@ -0,0 +1,372 @@
+/** @file
+ Migrates SEC structures after permanent memory is installed.
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PeiServicesLib.h>
+#include <Library/PeiServicesTablePointerLib.h>
+
+#include "SecMigrationPei.h"
+
+STATIC REPUBLISH_SEC_PPI_PPI mEdkiiRepublishSecPpiPpi = {
+ RepublishSecPpis
+ };
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_SEC_PLATFORM_INFORMATION_PPI mSecPlatformInformationPostMemoryPpi = {
+ SecPlatformInformationPostMemory
+ };
+
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_TEMPORARY_RAM_DONE_PPI mSecTemporaryRamDonePostMemoryPpi = {
+ SecTemporaryRamDonePostMemory
+ };
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI mSecTemporaryRamSupportPostMemoryPpi = {
+ SecTemporaryRamSupportPostMemory
+ };
+
+GLOBAL_REMOVE_IF_UNREFERENCED PEI_SEC_PERFORMANCE_PPI mSecPerformancePpi = {
+ GetPerformancePostMemory
+ };
+
+STATIC EFI_PEI_PPI_DESCRIPTOR mEdkiiRepublishSecPpiDescriptor = {
+ (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+ &gRepublishSecPpiPpiGuid,
+ &mEdkiiRepublishSecPpiPpi
+ };
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mSecPlatformInformationPostMemoryDescriptor = {
+ (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+ &gEfiSecPlatformInformationPpiGuid,
+ &mSecPlatformInformationPostMemoryPpi
+ };
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mSecTemporaryRamDonePostMemoryDescriptor = {
+ (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+ &gEfiTemporaryRamDonePpiGuid,
+ &mSecTemporaryRamDonePostMemoryPpi
+ };
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mSecTemporaryRamSupportPostMemoryDescriptor = {
+ (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+ &gEfiTemporaryRamSupportPpiGuid,
+ &mSecTemporaryRamSupportPostMemoryPpi
+ };
+
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mSecPerformancePpiDescriptor = {
+ (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+ &gPeiSecPerformancePpiGuid,
+ &mSecPerformancePpi
+ };
+
+/**
+ Disables the use of Temporary RAM.
+
+ If present, this service is invoked by the PEI Foundation after
+ the EFI_PEI_PERMANANT_MEMORY_INSTALLED_PPI is installed.
+
+ @retval EFI_SUCCESS Use of Temporary RAM was disabled.
+ @retval EFI_INVALID_PARAMETER Temporary RAM could not be disabled.
+
+**/
+EFI_STATUS
+EFIAPI
+SecTemporaryRamDonePostMemory (
+ VOID
+ )
+{
+ //
+ // Temporary RAM Done is already done in post-memory
+ // install a stub function that is located in permanent memory
+ //
+ return EFI_SUCCESS;
+}
+
+/**
+ This service of the EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI that migrates temporary RAM into
+ permanent memory.
+
+ @param PeiServices Pointer to the PEI Services Table.
+ @param TemporaryMemoryBase Source Address in temporary memory from which the SEC or PEIM will copy the
+ Temporary RAM contents.
+ @param PermanentMemoryBase Destination Address in permanent memory into which the SEC or PEIM will copy the
+ Temporary RAM contents.
+ @param CopySize Amount of memory to migrate from temporary to permanent memory.
+
+ @retval EFI_SUCCESS The data was successfully returned.
+ @retval EFI_INVALID_PARAMETER PermanentMemoryBase + CopySize > TemporaryMemoryBase when
+ TemporaryMemoryBase > PermanentMemoryBase.
+
+**/
+EFI_STATUS
+EFIAPI
+SecTemporaryRamSupportPostMemory (
+ IN CONST EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PHYSICAL_ADDRESS TemporaryMemoryBase,
+ IN EFI_PHYSICAL_ADDRESS PermanentMemoryBase,
+ IN UINTN CopySize
+ )
+{
+ //
+ // Temporary RAM Support is already done in post-memory
+ // install a stub function that is located in permanent memory
+ //
+ return EFI_SUCCESS;
+}
+
+/**
+ This interface conveys performance information out of the Security (SEC) phase into PEI.
+
+ This service is published by the SEC phase. The SEC phase handoff has an optional
+ EFI_PEI_PPI_DESCRIPTOR list as its final argument when control is passed from SEC into the
+ PEI Foundation. As such, if the platform supports collecting performance data in SEC,
+ this information is encapsulated into the data structure abstracted by this service.
+ This information is collected for the boot-strap processor (BSP) on IA-32.
+
+ @param[in] PeiServices The pointer to the PEI Services Table.
+ @param[in] This The pointer to this instance of the PEI_SEC_PERFORMANCE_PPI.
+ @param[out] Performance The pointer to performance data collected in SEC phase.
+
+ @retval EFI_SUCCESS The performance data was successfully returned.
+
+**/
+EFI_STATUS
+EFIAPI
+GetPerformancePostMemory (
+ IN CONST EFI_PEI_SERVICES **PeiServices,
+ IN PEI_SEC_PERFORMANCE_PPI *This,
+ OUT FIRMWARE_SEC_PERFORMANCE *Performance
+ )
+{
+ SEC_PLATFORM_INFORMATION_CONTEXT_HOB *SecPlatformInformationContexHob;
+
+ if (This == NULL || Performance == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ SecPlatformInformationContexHob = GetFirstGuidHob (&gEfiCallerIdGuid);
+ if (SecPlatformInformationContexHob == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ Performance->ResetEnd = SecPlatformInformationContexHob->FirmwareSecPerformance.ResetEnd;
+
+ return EFI_SUCCESS;
+}
+
+/**
+ This interface conveys state information out of the Security (SEC) phase into PEI.
+
+ @param[in] PeiServices Pointer to the PEI Services Table.
+ @param[in,out] StructureSize Pointer to the variable describing size of the input buffer.
+ @param[out] PlatformInformationRecord Pointer to the EFI_SEC_PLATFORM_INFORMATION_RECORD.
+
+ @retval EFI_SUCCESS The data was successfully returned.
+ @retval EFI_BUFFER_TOO_SMALL The buffer was too small.
+
+**/
+EFI_STATUS
+EFIAPI
+SecPlatformInformationPostMemory (
+ IN CONST EFI_PEI_SERVICES **PeiServices,
+ IN OUT UINT64 *StructureSize,
+ OUT EFI_SEC_PLATFORM_INFORMATION_RECORD *PlatformInformationRecord
+ )
+{
+ SEC_PLATFORM_INFORMATION_CONTEXT_HOB *SecPlatformInformationContexHob;
+
+ if (StructureSize == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ SecPlatformInformationContexHob = GetFirstGuidHob (&gEfiCallerIdGuid);
+ if (SecPlatformInformationContexHob == NULL) {
+ return EFI_NOT_FOUND;
+ }
+
+ if (*StructureSize < SecPlatformInformationContexHob->Context.StructureSize) {
+ *StructureSize = SecPlatformInformationContexHob->Context.StructureSize;
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ if (PlatformInformationRecord == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ *StructureSize = SecPlatformInformationContexHob->Context.StructureSize;
+ CopyMem (
+ (VOID *) PlatformInformationRecord,
+ (VOID *) SecPlatformInformationContexHob->Context.PlatformInformationRecord,
+ (UINTN) SecPlatformInformationContexHob->Context.StructureSize
+ );
+
+ return EFI_SUCCESS;
+}
+
+/**
+ This interface re-installs PPIs installed in SecCore from a post-memory PEIM.
+
+ This is to allow a platform that may not support relocation of SecCore to update the PPI instance to a post-memory
+ copy from a PEIM that has been shadowed to permanent memory.
+
+ @retval EFI_SUCCESS The SecCore PPIs were re-installed successfully.
+ @retval Others An error occurred re-installing the SecCore PPIs.
+
+**/
+EFI_STATUS
+EFIAPI
+RepublishSecPpis (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ EFI_PEI_PPI_DESCRIPTOR *PeiPpiDescriptor;
+ VOID *PeiPpi;
+ SEC_PLATFORM_INFORMATION_CONTEXT_HOB *SecPlatformInformationContextHob;
+ EFI_SEC_PLATFORM_INFORMATION_RECORD *SecPlatformInformationPtr;
+ UINT64 SecStructureSize;
+
+ SecPlatformInformationPtr = NULL;
+ SecStructureSize = 0;
+
+ Status = PeiServicesLocatePpi (
+ &gEfiTemporaryRamDonePpiGuid,
+ 0,
+ &PeiPpiDescriptor,
+ (VOID **) &PeiPpi
+ );
+ if (!EFI_ERROR (Status)) {
+ Status = PeiServicesReInstallPpi (
+ PeiPpiDescriptor,
+ &mSecTemporaryRamDonePostMemoryDescriptor
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
+
+ Status = PeiServicesLocatePpi (
+ &gEfiTemporaryRamSupportPpiGuid,
+ 0,
+ &PeiPpiDescriptor,
+ (VOID **) &PeiPpi
+ );
+ if (!EFI_ERROR (Status)) {
+ Status = PeiServicesReInstallPpi (
+ PeiPpiDescriptor,
+ &mSecTemporaryRamSupportPostMemoryDescriptor
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
+
+ Status = PeiServicesCreateHob (
+ EFI_HOB_TYPE_GUID_EXTENSION,
+ sizeof (SEC_PLATFORM_INFORMATION_CONTEXT_HOB),
+ (VOID **) &SecPlatformInformationContextHob
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "SecPlatformInformation Context HOB could not be created.\n"));
+ return Status;
+ }
+
+ SecPlatformInformationContextHob->Header.Name = gEfiCallerIdGuid;
+ SecPlatformInformationContextHob->Revision = 1;
+
+ Status = PeiServicesLocatePpi (
+ &gPeiSecPerformancePpiGuid,
+ 0,
+ &PeiPpiDescriptor,
+ (VOID **) &PeiPpi
+ );
+ if (!EFI_ERROR (Status)) {
+ Status = ((PEI_SEC_PERFORMANCE_PPI *) PeiPpi)->GetPerformance (
+ GetPeiServicesTablePointer (),
+ (PEI_SEC_PERFORMANCE_PPI *) PeiPpi,
+ &SecPlatformInformationContextHob->FirmwareSecPerformance
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (!EFI_ERROR (Status)) {
+ Status = PeiServicesReInstallPpi (
+ PeiPpiDescriptor,
+ &mSecPerformancePpiDescriptor
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
+ }
+
+ Status = PeiServicesLocatePpi (
+ &gEfiSecPlatformInformationPpiGuid,
+ 0,
+ &PeiPpiDescriptor,
+ (VOID **) &PeiPpi
+ );
+ if (!EFI_ERROR (Status)) {
+ Status = ((EFI_SEC_PLATFORM_INFORMATION_PPI *) PeiPpi)->PlatformInformation (
+ GetPeiServicesTablePointer (),
+ &SecStructureSize,
+ SecPlatformInformationPtr
+ );
+ ASSERT (Status == EFI_BUFFER_TOO_SMALL);
+ if (Status != EFI_BUFFER_TOO_SMALL) {
+ return EFI_NOT_FOUND;
+ }
+
+ ZeroMem ((VOID *) &(SecPlatformInformationContextHob->Context), sizeof (SEC_PLATFORM_INFORMATION_CONTEXT));
+ SecPlatformInformationContextHob->Context.PlatformInformationRecord = AllocatePool ((UINTN) SecStructureSize);
+ ASSERT (SecPlatformInformationContextHob->Context.PlatformInformationRecord != NULL);
+ if (SecPlatformInformationContextHob->Context.PlatformInformationRecord == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+ SecPlatformInformationContextHob->Context.StructureSize = SecStructureSize;
+
+ Status = ((EFI_SEC_PLATFORM_INFORMATION_PPI *) PeiPpi)->PlatformInformation (
+ GetPeiServicesTablePointer (),
+ &(SecPlatformInformationContextHob->Context.StructureSize),
+ SecPlatformInformationContextHob->Context.PlatformInformationRecord
+ );
+ ASSERT_EFI_ERROR (Status);
+ if (!EFI_ERROR (Status)) {
+ Status = PeiServicesReInstallPpi (
+ PeiPpiDescriptor,
+ &mSecPlatformInformationPostMemoryDescriptor
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
+ This function is the entry point which installs an instance of REPUBLISH_SEC_PPI_PPI.
+
+ @param[in] FileHandle Pointer to image file handle.
+ @param[in] PeiServices Pointer to PEI Services Table
+
+ @retval EFI_SUCCESS An instance of REPUBLISH_SEC_PPI_PPI was installed successfully.
+ @retval Others An error occurred installing and instance of REPUBLISH_SEC_PPI_PPI.
+
+**/
+EFI_STATUS
+EFIAPI
+SecMigrationPeiInitialize (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN CONST EFI_PEI_SERVICES **PeiServices
+ )
+{
+ EFI_STATUS Status;
+
+ Status = PeiServicesInstallPpi (&mEdkiiRepublishSecPpiDescriptor);
+ ASSERT_EFI_ERROR (Status);
+
+ return Status;
+}
diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.h b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.h
new file mode 100644
index 000000000000..372f8044bdb2
--- /dev/null
+++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.h
@@ -0,0 +1,170 @@
+/** @file
+ Migrates SEC structures after permanent memory is installed.
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __SEC_MIGRATION_H__
+#define __SEC_MIGRATION_H__
+
+#include <Base.h>
+
+#include <Pi/PiPeiCis.h>
+#include <Ppi/RepublishSecPpi.h>
+#include <Ppi/SecPerformance.h>
+#include <Ppi/SecPlatformInformation.h>
+#include <Ppi/SecPlatformInformation2.h>
+#include <Ppi/TemporaryRamDone.h>
+#include <Ppi/TemporaryRamSupport.h>
+
+/**
+ This interface conveys state information out of the Security (SEC) phase into PEI.
+
+ @param[in] PeiServices Pointer to the PEI Services Table.
+ @param[in,out] StructureSize Pointer to the variable describing size of the input buffer.
+ @param[out] PlatformInformationRecord Pointer to the EFI_SEC_PLATFORM_INFORMATION_RECORD.
+
+ @retval EFI_SUCCESS The data was successfully returned.
+ @retval EFI_BUFFER_TOO_SMALL The buffer was too small.
+
+**/
+EFI_STATUS
+EFIAPI
+SecPlatformInformationPostMemory (
+ IN CONST EFI_PEI_SERVICES **PeiServices,
+ IN OUT UINT64 *StructureSize,
+ OUT EFI_SEC_PLATFORM_INFORMATION_RECORD *PlatformInformationRecord
+ );
+
+/**
+ Re-installs the SEC Platform Information PPIs to implementation in this module to support post-memory.
+
+ @param[in] PeiServices An indirect pointer to the EFI_PEI_SERVICES table published by the PEI Foundation.
+ @param[in] NotifyDescriptor Address of the notification descriptor data structure.
+ @param[in] Ppi Address of the PPI that was installed.
+
+ @retval EFI_SUCCESS The SEC Platform Information PPI could not be re-installed.
+ @return Others An error occurred during PPI re-install.
+
+**/
+EFI_STATUS
+EFIAPI
+SecPlatformInformationPpiNotifyCallback (
+ IN EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor,
+ IN VOID *Ppi
+ );
+
+/**
+ This interface re-installs PPIs installed in SecCore from a post-memory PEIM.
+
+ This is to allow a platform that may not support relocation of SecCore to update the PPI instance to a post-memory
+ copy from a PEIM that has been shadowed to permanent memory.
+
+ @retval EFI_SUCCESS The SecCore PPIs were re-installed successfully.
+ @retval Others An error occurred re-installing the SecCore PPIs.
+
+**/
+EFI_STATUS
+EFIAPI
+RepublishSecPpis (
+ VOID
+ );
+
+/**
+ Disables the use of Temporary RAM.
+
+ If present, this service is invoked by the PEI Foundation after
+ the EFI_PEI_PERMANANT_MEMORY_INSTALLED_PPI is installed.
+
+ @retval EFI_SUCCESS Use of Temporary RAM was disabled.
+ @retval EFI_INVALID_PARAMETER Temporary RAM could not be disabled.
+
+**/
+EFI_STATUS
+EFIAPI
+SecTemporaryRamDonePostMemory (
+ VOID
+ );
+
+/**
+ This service of the EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI that migrates temporary RAM into
+ permanent memory.
+
+ @param PeiServices Pointer to the PEI Services Table.
+ @param TemporaryMemoryBase Source Address in temporary memory from which the SEC or PEIM will copy the
+ Temporary RAM contents.
+ @param PermanentMemoryBase Destination Address in permanent memory into which the SEC or PEIM will copy the
+ Temporary RAM contents.
+ @param CopySize Amount of memory to migrate from temporary to permanent memory.
+
+ @retval EFI_SUCCESS The data was successfully returned.
+ @retval EFI_INVALID_PARAMETER PermanentMemoryBase + CopySize > TemporaryMemoryBase when
+ TemporaryMemoryBase > PermanentMemoryBase.
+
+**/
+EFI_STATUS
+EFIAPI
+SecTemporaryRamSupportPostMemory (
+ IN CONST EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PHYSICAL_ADDRESS TemporaryMemoryBase,
+ IN EFI_PHYSICAL_ADDRESS PermanentMemoryBase,
+ IN UINTN CopySize
+ );
+
+/**
+ This interface conveys performance information out of the Security (SEC) phase into PEI.
+
+ This service is published by the SEC phase. The SEC phase handoff has an optional
+ EFI_PEI_PPI_DESCRIPTOR list as its final argument when control is passed from SEC into the
+ PEI Foundation. As such, if the platform supports collecting performance data in SEC,
+ this information is encapsulated into the data structure abstracted by this service.
+ This information is collected for the boot-strap processor (BSP) on IA-32.
+
+ @param[in] PeiServices The pointer to the PEI Services Table.
+ @param[in] This The pointer to this instance of the PEI_SEC_PERFORMANCE_PPI.
+ @param[out] Performance The pointer to performance data collected in SEC phase.
+
+ @retval EFI_SUCCESS The performance data was successfully returned.
+
+**/
+EFI_STATUS
+EFIAPI
+GetPerformancePostMemory (
+ IN CONST EFI_PEI_SERVICES **PeiServices,
+ IN PEI_SEC_PERFORMANCE_PPI *This,
+ OUT FIRMWARE_SEC_PERFORMANCE *Performance
+ );
+
+// /**
+// Disables the use of Temporary RAM.
+
+// If present, this service is invoked by the PEI Foundation after
+// the EFI_PEI_PERMANANT_MEMORY_INSTALLED_PPI is installed.
+
+// @retval EFI_SUCCESS Use of Temporary RAM was disabled.
+// @retval EFI_INVALID_PARAMETER Temporary RAM could not be disabled.
+
+// **/
+// EFI_STATUS
+// EFIAPI
+// SecTemporaryRamDonePostMemory (
+// VOID
+// );
+
+typedef struct {
+ UINT64 StructureSize;
+ EFI_SEC_PLATFORM_INFORMATION_RECORD *PlatformInformationRecord;
+} SEC_PLATFORM_INFORMATION_CONTEXT;
+
+typedef struct {
+ EFI_HOB_GUID_TYPE Header;
+ UINT8 Revision;
+ UINT8 Reserved[3];
+ FIRMWARE_SEC_PERFORMANCE FirmwareSecPerformance;
+ SEC_PLATFORM_INFORMATION_CONTEXT Context;
+} SEC_PLATFORM_INFORMATION_CONTEXT_HOB;
+
+#endif
diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
new file mode 100644
index 000000000000..e29c04710941
--- /dev/null
+++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
@@ -0,0 +1,64 @@
+## @file
+# Migrates SEC structures after permanent memory is installed.
+#
+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = SecMigrationPei
+ MODULE_UNI_FILE = SecMigrationPei.uni
+ FILE_GUID = 58B35361-8922-41BC-B313-EF7ED9ADFDF7
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.0
+ ENTRY_POINT = SecMigrationPeiInitialize
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64 EBC
+#
+
+[Sources]
+ SecMigrationPei.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ HobLib
+ MemoryAllocationLib
+ PeimEntryPoint
+ PeiServicesLib
+ PeiServicesTablePointerLib
+
+[Ppis]
+ ## PRODUCES
+ gRepublishSecPpiPpiGuid
+
+ ## SOMETIMES_PRODUCES
+ gEfiTemporaryRamDonePpiGuid
+
+ ## SOMETIME_PRODUCES
+ gEfiTemporaryRamSupportPpiGuid
+
+ ## SOMETIMES_PRODUCES
+ gPeiSecPerformancePpiGuid
+
+ ## SOMETIMES_CONSUMES
+ ## PRODUCES
+ gEfiSecPlatformInformationPpiGuid
+
+ ## SOMETIMES_CONSUMES
+ ## SOMETIMES_PRODUCES
+ gEfiSecPlatformInformation2PpiGuid
+
+[Depex]
+ TRUE
diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni
new file mode 100644
index 000000000000..62c2064ba217
--- /dev/null
+++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni
@@ -0,0 +1,13 @@
+// /** @file
+// Migrates SEC structures after permanent memory is installed.
+//
+// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT #language en-US "Migrates SEC structures after permanent memory is installed"
+
+#string STR_MODULE_DESCRIPTION #language en-US "Migrates SEC structures after permanent memory is installed."
+
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 762badf5d239..0a005bd20311 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -66,6 +66,10 @@ [Guids]
## Include/Guid/MicrocodePatchHob.h
gEdkiiMicrocodePatchHobGuid = { 0xd178f11d, 0x8716, 0x418e, { 0xa1, 0x31, 0x96, 0x7d, 0x2a, 0xc4, 0x28, 0x43 }}
+[Ppis]
+ ## Include/Ppi/RepublishSecPpi.h
+ gRepublishSecPpiPpiGuid = { 0x27a71b1e, 0x73ee, 0x43d6, { 0xac, 0xe3, 0x52, 0x1a, 0x2d, 0xc5, 0xd0, 0x92 }}
+
[Protocols]
## Include/Protocol/SmmCpuService.h
gEfiSmmCpuServiceProtocolGuid = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index afa304128221..964720048dd7 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -146,6 +146,7 @@ [Components.IA32, Components.X64]
UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
UefiCpuPkg/SecCore/SecCore.inf
+ UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf {
<Defines>
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 3/9] UefiCpuPkg/SecMigrationPei: Add initial PEIM (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 3/9] UefiCpuPkg/SecMigrationPei: Add initial PEIM (CVE-2019-11098) Guomin Jiang
@ 2020-07-03 11:38 ` Laszlo Ersek
0 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 11:38 UTC (permalink / raw)
To: devel, guomin.jiang
Cc: Michael Kubacki, Eric Dong, Ray Ni, Rahul Kumar, Debkumar De,
Harry Han, Catharine West
On 07/02/20 07:15, Guomin Jiang wrote:
> From: Michael Kubacki <michael.a.kubacki@intel.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> Adds a PEIM that republishes structures produced in SEC. This
> is done because SEC modules may not be shadowed in some platforms
> due to space constraints or special alignment requirements. The
> SecMigrationPei module locates interfaces that may be published in
> SEC and reinstalls the interface with permanent memory addresses.
>
> This is important if pre-memory address access is forbidden after
> memory initialization and data such as a PPI descriptor, PPI GUID,
> or PPI inteface reside in pre-memory.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Harry Han <harry.han@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Michael Kubacki <michael.a.kubacki@intel.com>
> ---
> UefiCpuPkg/Include/Ppi/RepublishSecPpi.h | 54 +++
> UefiCpuPkg/SecCore/SecCore.inf | 2 +
> UefiCpuPkg/SecCore/SecMain.c | 26 +-
> UefiCpuPkg/SecCore/SecMain.h | 1 +
> UefiCpuPkg/SecMigrationPei/SecMigrationPei.c | 372 ++++++++++++++++++
> UefiCpuPkg/SecMigrationPei/SecMigrationPei.h | 170 ++++++++
> .../SecMigrationPei/SecMigrationPei.inf | 64 +++
> .../SecMigrationPei/SecMigrationPei.uni | 13 +
> UefiCpuPkg/UefiCpuPkg.dec | 4 +
> UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> 10 files changed, 705 insertions(+), 2 deletions(-)
> create mode 100644 UefiCpuPkg/Include/Ppi/RepublishSecPpi.h
> create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
> create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.h
> create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
> create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni
So this patch introduces SecMigrationPei, and a new PPI. OVMF doesn't
use those.
The patch also changes the existent UefiCpuPkg/SecCore module. OVMF
doesn't use that either.
Because of the above, I defer to Eric and Ray on this patch.
Thanks
Laszlo
>
> diff --git a/UefiCpuPkg/Include/Ppi/RepublishSecPpi.h b/UefiCpuPkg/Include/Ppi/RepublishSecPpi.h
> new file mode 100644
> index 000000000000..6fb9f1b005b4
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Ppi/RepublishSecPpi.h
> @@ -0,0 +1,54 @@
> +/** @file
> + This file declares Sec Platform Information PPI.
> +
> + This service is the primary handoff state into the PEI Foundation.
> + The Security (SEC) component creates the early, transitory memory
> + environment and also encapsulates knowledge of at least the
> + location of the Boot Firmware Volume (BFV).
> +
> + Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + @par Revision Reference:
> + This PPI is introduced in PI Version 1.0.
> +
> +**/
> +
> +#ifndef __REPUBLISH_SEC_PPI_H__
> +#define __REPUBLISH_SEC_PPI_H__
> +
> +#include <Pi/PiPeiCis.h>
> +
> +#define REPUBLISH_SEC_PPI_PPI_GUID \
> + { \
> + 0x27a71b1e, 0x73ee, 0x43d6, { 0xac, 0xe3, 0x52, 0x1a, 0x2d, 0xc5, 0xd0, 0x92 } \
> + }
> +
> +typedef struct _REPUBLISH_SEC_PPI_PPI REPUBLISH_SEC_PPI_PPI;
> +
> +/**
> + This interface re-installs PPIs installed in SecCore from a post-memory PEIM.
> +
> + This is to allow a platform that may not support relocation of SecCore to update the PPI instance to a post-memory
> + copy from a PEIM that has been shadowed to permanent memory.
> +
> + @retval EFI_SUCCESS The SecCore PPIs were re-installed successfully.
> + @retval Others An error occurred re-installing the SecCore PPIs.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *REPUBLISH_SEC_PPI_REPUBLISH_SEC_PPIS)(
> + VOID
> + );
> +
> +///
> +///
> +///
> +struct _REPUBLISH_SEC_PPI_PPI {
> + REPUBLISH_SEC_PPI_REPUBLISH_SEC_PPIS RepublishSecPpis;
> +};
> +
> +extern EFI_GUID gRepublishSecPpiPpiGuid;
> +
> +#endif
> diff --git a/UefiCpuPkg/SecCore/SecCore.inf b/UefiCpuPkg/SecCore/SecCore.inf
> index 0562820c95e0..545781d6b4b3 100644
> --- a/UefiCpuPkg/SecCore/SecCore.inf
> +++ b/UefiCpuPkg/SecCore/SecCore.inf
> @@ -68,6 +68,8 @@ [Ppis]
> ## SOMETIMES_CONSUMES
> gPeiSecPerformancePpiGuid
> gEfiPeiCoreFvLocationPpiGuid
> + ## CONSUMES
> + gRepublishSecPpiPpiGuid
>
> [Guids]
> ## SOMETIMES_PRODUCES ## HOB
> diff --git a/UefiCpuPkg/SecCore/SecMain.c b/UefiCpuPkg/SecCore/SecMain.c
> index 5d5e7f17dced..155be49a6011 100644
> --- a/UefiCpuPkg/SecCore/SecMain.c
> +++ b/UefiCpuPkg/SecCore/SecMain.c
> @@ -370,13 +370,35 @@ SecTemporaryRamDone (
> VOID
> )
> {
> - BOOLEAN State;
> + EFI_STATUS Status;
> + EFI_STATUS Status2;
> + UINTN Index;
> + BOOLEAN State;
> + EFI_PEI_PPI_DESCRIPTOR *PeiPpiDescriptor;
> + REPUBLISH_SEC_PPI_PPI *RepublishSecPpiPpi;
>
> //
> // Republish Sec Platform Information(2) PPI
> //
> RepublishSecPlatformInformationPpi ();
>
> + //
> + // Re-install SEC PPIs using a PEIM produced service if published
> + //
> + for (Index = 0, Status = EFI_SUCCESS; Status == EFI_SUCCESS; Index++) {
> + Status = PeiServicesLocatePpi (
> + &gRepublishSecPpiPpiGuid,
> + Index,
> + &PeiPpiDescriptor,
> + (VOID **) &RepublishSecPpiPpi
> + );
> + if (!EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_INFO, "Calling RepublishSecPpi instance %d.\n", Index));
> + Status2 = RepublishSecPpiPpi->RepublishSecPpis ();
> + ASSERT_EFI_ERROR (Status2);
> + }
> + }
> +
> //
> // Migrate DebugAgentContext.
> //
> @@ -385,7 +407,7 @@ SecTemporaryRamDone (
> //
> // Disable interrupts and save current interrupt state
> //
> - State = SaveAndDisableInterrupts();
> + State = SaveAndDisableInterrupts ();
>
> //
> // Disable Temporary RAM after Stack and Heap have been migrated at this point.
> diff --git a/UefiCpuPkg/SecCore/SecMain.h b/UefiCpuPkg/SecCore/SecMain.h
> index e8c05d713668..e20bcf86532c 100644
> --- a/UefiCpuPkg/SecCore/SecMain.h
> +++ b/UefiCpuPkg/SecCore/SecMain.h
> @@ -15,6 +15,7 @@
> #include <Ppi/TemporaryRamDone.h>
> #include <Ppi/SecPerformance.h>
> #include <Ppi/PeiCoreFvLocation.h>
> +#include <Ppi/RepublishSecPpi.h>
>
> #include <Guid/FirmwarePerformance.h>
>
> diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
> new file mode 100644
> index 000000000000..f96013b09b21
> --- /dev/null
> +++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
> @@ -0,0 +1,372 @@
> +/** @file
> + Migrates SEC structures after permanent memory is installed.
> +
> + Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PeiServicesLib.h>
> +#include <Library/PeiServicesTablePointerLib.h>
> +
> +#include "SecMigrationPei.h"
> +
> +STATIC REPUBLISH_SEC_PPI_PPI mEdkiiRepublishSecPpiPpi = {
> + RepublishSecPpis
> + };
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_SEC_PLATFORM_INFORMATION_PPI mSecPlatformInformationPostMemoryPpi = {
> + SecPlatformInformationPostMemory
> + };
> +
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_TEMPORARY_RAM_DONE_PPI mSecTemporaryRamDonePostMemoryPpi = {
> + SecTemporaryRamDonePostMemory
> + };
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI mSecTemporaryRamSupportPostMemoryPpi = {
> + SecTemporaryRamSupportPostMemory
> + };
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED PEI_SEC_PERFORMANCE_PPI mSecPerformancePpi = {
> + GetPerformancePostMemory
> + };
> +
> +STATIC EFI_PEI_PPI_DESCRIPTOR mEdkiiRepublishSecPpiDescriptor = {
> + (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> + &gRepublishSecPpiPpiGuid,
> + &mEdkiiRepublishSecPpiPpi
> + };
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mSecPlatformInformationPostMemoryDescriptor = {
> + (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> + &gEfiSecPlatformInformationPpiGuid,
> + &mSecPlatformInformationPostMemoryPpi
> + };
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mSecTemporaryRamDonePostMemoryDescriptor = {
> + (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> + &gEfiTemporaryRamDonePpiGuid,
> + &mSecTemporaryRamDonePostMemoryPpi
> + };
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mSecTemporaryRamSupportPostMemoryDescriptor = {
> + (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> + &gEfiTemporaryRamSupportPpiGuid,
> + &mSecTemporaryRamSupportPostMemoryPpi
> + };
> +
> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PEI_PPI_DESCRIPTOR mSecPerformancePpiDescriptor = {
> + (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> + &gPeiSecPerformancePpiGuid,
> + &mSecPerformancePpi
> + };
> +
> +/**
> + Disables the use of Temporary RAM.
> +
> + If present, this service is invoked by the PEI Foundation after
> + the EFI_PEI_PERMANANT_MEMORY_INSTALLED_PPI is installed.
> +
> + @retval EFI_SUCCESS Use of Temporary RAM was disabled.
> + @retval EFI_INVALID_PARAMETER Temporary RAM could not be disabled.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecTemporaryRamDonePostMemory (
> + VOID
> + )
> +{
> + //
> + // Temporary RAM Done is already done in post-memory
> + // install a stub function that is located in permanent memory
> + //
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + This service of the EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI that migrates temporary RAM into
> + permanent memory.
> +
> + @param PeiServices Pointer to the PEI Services Table.
> + @param TemporaryMemoryBase Source Address in temporary memory from which the SEC or PEIM will copy the
> + Temporary RAM contents.
> + @param PermanentMemoryBase Destination Address in permanent memory into which the SEC or PEIM will copy the
> + Temporary RAM contents.
> + @param CopySize Amount of memory to migrate from temporary to permanent memory.
> +
> + @retval EFI_SUCCESS The data was successfully returned.
> + @retval EFI_INVALID_PARAMETER PermanentMemoryBase + CopySize > TemporaryMemoryBase when
> + TemporaryMemoryBase > PermanentMemoryBase.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecTemporaryRamSupportPostMemory (
> + IN CONST EFI_PEI_SERVICES **PeiServices,
> + IN EFI_PHYSICAL_ADDRESS TemporaryMemoryBase,
> + IN EFI_PHYSICAL_ADDRESS PermanentMemoryBase,
> + IN UINTN CopySize
> + )
> +{
> + //
> + // Temporary RAM Support is already done in post-memory
> + // install a stub function that is located in permanent memory
> + //
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + This interface conveys performance information out of the Security (SEC) phase into PEI.
> +
> + This service is published by the SEC phase. The SEC phase handoff has an optional
> + EFI_PEI_PPI_DESCRIPTOR list as its final argument when control is passed from SEC into the
> + PEI Foundation. As such, if the platform supports collecting performance data in SEC,
> + this information is encapsulated into the data structure abstracted by this service.
> + This information is collected for the boot-strap processor (BSP) on IA-32.
> +
> + @param[in] PeiServices The pointer to the PEI Services Table.
> + @param[in] This The pointer to this instance of the PEI_SEC_PERFORMANCE_PPI.
> + @param[out] Performance The pointer to performance data collected in SEC phase.
> +
> + @retval EFI_SUCCESS The performance data was successfully returned.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetPerformancePostMemory (
> + IN CONST EFI_PEI_SERVICES **PeiServices,
> + IN PEI_SEC_PERFORMANCE_PPI *This,
> + OUT FIRMWARE_SEC_PERFORMANCE *Performance
> + )
> +{
> + SEC_PLATFORM_INFORMATION_CONTEXT_HOB *SecPlatformInformationContexHob;
> +
> + if (This == NULL || Performance == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + SecPlatformInformationContexHob = GetFirstGuidHob (&gEfiCallerIdGuid);
> + if (SecPlatformInformationContexHob == NULL) {
> + return EFI_NOT_FOUND;
> + }
> +
> + Performance->ResetEnd = SecPlatformInformationContexHob->FirmwareSecPerformance.ResetEnd;
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + This interface conveys state information out of the Security (SEC) phase into PEI.
> +
> + @param[in] PeiServices Pointer to the PEI Services Table.
> + @param[in,out] StructureSize Pointer to the variable describing size of the input buffer.
> + @param[out] PlatformInformationRecord Pointer to the EFI_SEC_PLATFORM_INFORMATION_RECORD.
> +
> + @retval EFI_SUCCESS The data was successfully returned.
> + @retval EFI_BUFFER_TOO_SMALL The buffer was too small.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecPlatformInformationPostMemory (
> + IN CONST EFI_PEI_SERVICES **PeiServices,
> + IN OUT UINT64 *StructureSize,
> + OUT EFI_SEC_PLATFORM_INFORMATION_RECORD *PlatformInformationRecord
> + )
> +{
> + SEC_PLATFORM_INFORMATION_CONTEXT_HOB *SecPlatformInformationContexHob;
> +
> + if (StructureSize == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + SecPlatformInformationContexHob = GetFirstGuidHob (&gEfiCallerIdGuid);
> + if (SecPlatformInformationContexHob == NULL) {
> + return EFI_NOT_FOUND;
> + }
> +
> + if (*StructureSize < SecPlatformInformationContexHob->Context.StructureSize) {
> + *StructureSize = SecPlatformInformationContexHob->Context.StructureSize;
> + return EFI_BUFFER_TOO_SMALL;
> + }
> +
> + if (PlatformInformationRecord == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + *StructureSize = SecPlatformInformationContexHob->Context.StructureSize;
> + CopyMem (
> + (VOID *) PlatformInformationRecord,
> + (VOID *) SecPlatformInformationContexHob->Context.PlatformInformationRecord,
> + (UINTN) SecPlatformInformationContexHob->Context.StructureSize
> + );
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + This interface re-installs PPIs installed in SecCore from a post-memory PEIM.
> +
> + This is to allow a platform that may not support relocation of SecCore to update the PPI instance to a post-memory
> + copy from a PEIM that has been shadowed to permanent memory.
> +
> + @retval EFI_SUCCESS The SecCore PPIs were re-installed successfully.
> + @retval Others An error occurred re-installing the SecCore PPIs.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +RepublishSecPpis (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + EFI_PEI_PPI_DESCRIPTOR *PeiPpiDescriptor;
> + VOID *PeiPpi;
> + SEC_PLATFORM_INFORMATION_CONTEXT_HOB *SecPlatformInformationContextHob;
> + EFI_SEC_PLATFORM_INFORMATION_RECORD *SecPlatformInformationPtr;
> + UINT64 SecStructureSize;
> +
> + SecPlatformInformationPtr = NULL;
> + SecStructureSize = 0;
> +
> + Status = PeiServicesLocatePpi (
> + &gEfiTemporaryRamDonePpiGuid,
> + 0,
> + &PeiPpiDescriptor,
> + (VOID **) &PeiPpi
> + );
> + if (!EFI_ERROR (Status)) {
> + Status = PeiServicesReInstallPpi (
> + PeiPpiDescriptor,
> + &mSecTemporaryRamDonePostMemoryDescriptor
> + );
> + ASSERT_EFI_ERROR (Status);
> + }
> +
> + Status = PeiServicesLocatePpi (
> + &gEfiTemporaryRamSupportPpiGuid,
> + 0,
> + &PeiPpiDescriptor,
> + (VOID **) &PeiPpi
> + );
> + if (!EFI_ERROR (Status)) {
> + Status = PeiServicesReInstallPpi (
> + PeiPpiDescriptor,
> + &mSecTemporaryRamSupportPostMemoryDescriptor
> + );
> + ASSERT_EFI_ERROR (Status);
> + }
> +
> + Status = PeiServicesCreateHob (
> + EFI_HOB_TYPE_GUID_EXTENSION,
> + sizeof (SEC_PLATFORM_INFORMATION_CONTEXT_HOB),
> + (VOID **) &SecPlatformInformationContextHob
> + );
> + ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "SecPlatformInformation Context HOB could not be created.\n"));
> + return Status;
> + }
> +
> + SecPlatformInformationContextHob->Header.Name = gEfiCallerIdGuid;
> + SecPlatformInformationContextHob->Revision = 1;
> +
> + Status = PeiServicesLocatePpi (
> + &gPeiSecPerformancePpiGuid,
> + 0,
> + &PeiPpiDescriptor,
> + (VOID **) &PeiPpi
> + );
> + if (!EFI_ERROR (Status)) {
> + Status = ((PEI_SEC_PERFORMANCE_PPI *) PeiPpi)->GetPerformance (
> + GetPeiServicesTablePointer (),
> + (PEI_SEC_PERFORMANCE_PPI *) PeiPpi,
> + &SecPlatformInformationContextHob->FirmwareSecPerformance
> + );
> + ASSERT_EFI_ERROR (Status);
> + if (!EFI_ERROR (Status)) {
> + Status = PeiServicesReInstallPpi (
> + PeiPpiDescriptor,
> + &mSecPerformancePpiDescriptor
> + );
> + ASSERT_EFI_ERROR (Status);
> + }
> + }
> +
> + Status = PeiServicesLocatePpi (
> + &gEfiSecPlatformInformationPpiGuid,
> + 0,
> + &PeiPpiDescriptor,
> + (VOID **) &PeiPpi
> + );
> + if (!EFI_ERROR (Status)) {
> + Status = ((EFI_SEC_PLATFORM_INFORMATION_PPI *) PeiPpi)->PlatformInformation (
> + GetPeiServicesTablePointer (),
> + &SecStructureSize,
> + SecPlatformInformationPtr
> + );
> + ASSERT (Status == EFI_BUFFER_TOO_SMALL);
> + if (Status != EFI_BUFFER_TOO_SMALL) {
> + return EFI_NOT_FOUND;
> + }
> +
> + ZeroMem ((VOID *) &(SecPlatformInformationContextHob->Context), sizeof (SEC_PLATFORM_INFORMATION_CONTEXT));
> + SecPlatformInformationContextHob->Context.PlatformInformationRecord = AllocatePool ((UINTN) SecStructureSize);
> + ASSERT (SecPlatformInformationContextHob->Context.PlatformInformationRecord != NULL);
> + if (SecPlatformInformationContextHob->Context.PlatformInformationRecord == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> + SecPlatformInformationContextHob->Context.StructureSize = SecStructureSize;
> +
> + Status = ((EFI_SEC_PLATFORM_INFORMATION_PPI *) PeiPpi)->PlatformInformation (
> + GetPeiServicesTablePointer (),
> + &(SecPlatformInformationContextHob->Context.StructureSize),
> + SecPlatformInformationContextHob->Context.PlatformInformationRecord
> + );
> + ASSERT_EFI_ERROR (Status);
> + if (!EFI_ERROR (Status)) {
> + Status = PeiServicesReInstallPpi (
> + PeiPpiDescriptor,
> + &mSecPlatformInformationPostMemoryDescriptor
> + );
> + ASSERT_EFI_ERROR (Status);
> + }
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + This function is the entry point which installs an instance of REPUBLISH_SEC_PPI_PPI.
> +
> + @param[in] FileHandle Pointer to image file handle.
> + @param[in] PeiServices Pointer to PEI Services Table
> +
> + @retval EFI_SUCCESS An instance of REPUBLISH_SEC_PPI_PPI was installed successfully.
> + @retval Others An error occurred installing and instance of REPUBLISH_SEC_PPI_PPI.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecMigrationPeiInitialize (
> + IN EFI_PEI_FILE_HANDLE FileHandle,
> + IN CONST EFI_PEI_SERVICES **PeiServices
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = PeiServicesInstallPpi (&mEdkiiRepublishSecPpiDescriptor);
> + ASSERT_EFI_ERROR (Status);
> +
> + return Status;
> +}
> diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.h b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.h
> new file mode 100644
> index 000000000000..372f8044bdb2
> --- /dev/null
> +++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.h
> @@ -0,0 +1,170 @@
> +/** @file
> + Migrates SEC structures after permanent memory is installed.
> +
> + Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __SEC_MIGRATION_H__
> +#define __SEC_MIGRATION_H__
> +
> +#include <Base.h>
> +
> +#include <Pi/PiPeiCis.h>
> +#include <Ppi/RepublishSecPpi.h>
> +#include <Ppi/SecPerformance.h>
> +#include <Ppi/SecPlatformInformation.h>
> +#include <Ppi/SecPlatformInformation2.h>
> +#include <Ppi/TemporaryRamDone.h>
> +#include <Ppi/TemporaryRamSupport.h>
> +
> +/**
> + This interface conveys state information out of the Security (SEC) phase into PEI.
> +
> + @param[in] PeiServices Pointer to the PEI Services Table.
> + @param[in,out] StructureSize Pointer to the variable describing size of the input buffer.
> + @param[out] PlatformInformationRecord Pointer to the EFI_SEC_PLATFORM_INFORMATION_RECORD.
> +
> + @retval EFI_SUCCESS The data was successfully returned.
> + @retval EFI_BUFFER_TOO_SMALL The buffer was too small.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecPlatformInformationPostMemory (
> + IN CONST EFI_PEI_SERVICES **PeiServices,
> + IN OUT UINT64 *StructureSize,
> + OUT EFI_SEC_PLATFORM_INFORMATION_RECORD *PlatformInformationRecord
> + );
> +
> +/**
> + Re-installs the SEC Platform Information PPIs to implementation in this module to support post-memory.
> +
> + @param[in] PeiServices An indirect pointer to the EFI_PEI_SERVICES table published by the PEI Foundation.
> + @param[in] NotifyDescriptor Address of the notification descriptor data structure.
> + @param[in] Ppi Address of the PPI that was installed.
> +
> + @retval EFI_SUCCESS The SEC Platform Information PPI could not be re-installed.
> + @return Others An error occurred during PPI re-install.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecPlatformInformationPpiNotifyCallback (
> + IN EFI_PEI_SERVICES **PeiServices,
> + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor,
> + IN VOID *Ppi
> + );
> +
> +/**
> + This interface re-installs PPIs installed in SecCore from a post-memory PEIM.
> +
> + This is to allow a platform that may not support relocation of SecCore to update the PPI instance to a post-memory
> + copy from a PEIM that has been shadowed to permanent memory.
> +
> + @retval EFI_SUCCESS The SecCore PPIs were re-installed successfully.
> + @retval Others An error occurred re-installing the SecCore PPIs.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +RepublishSecPpis (
> + VOID
> + );
> +
> +/**
> + Disables the use of Temporary RAM.
> +
> + If present, this service is invoked by the PEI Foundation after
> + the EFI_PEI_PERMANANT_MEMORY_INSTALLED_PPI is installed.
> +
> + @retval EFI_SUCCESS Use of Temporary RAM was disabled.
> + @retval EFI_INVALID_PARAMETER Temporary RAM could not be disabled.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecTemporaryRamDonePostMemory (
> + VOID
> + );
> +
> +/**
> + This service of the EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI that migrates temporary RAM into
> + permanent memory.
> +
> + @param PeiServices Pointer to the PEI Services Table.
> + @param TemporaryMemoryBase Source Address in temporary memory from which the SEC or PEIM will copy the
> + Temporary RAM contents.
> + @param PermanentMemoryBase Destination Address in permanent memory into which the SEC or PEIM will copy the
> + Temporary RAM contents.
> + @param CopySize Amount of memory to migrate from temporary to permanent memory.
> +
> + @retval EFI_SUCCESS The data was successfully returned.
> + @retval EFI_INVALID_PARAMETER PermanentMemoryBase + CopySize > TemporaryMemoryBase when
> + TemporaryMemoryBase > PermanentMemoryBase.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SecTemporaryRamSupportPostMemory (
> + IN CONST EFI_PEI_SERVICES **PeiServices,
> + IN EFI_PHYSICAL_ADDRESS TemporaryMemoryBase,
> + IN EFI_PHYSICAL_ADDRESS PermanentMemoryBase,
> + IN UINTN CopySize
> + );
> +
> +/**
> + This interface conveys performance information out of the Security (SEC) phase into PEI.
> +
> + This service is published by the SEC phase. The SEC phase handoff has an optional
> + EFI_PEI_PPI_DESCRIPTOR list as its final argument when control is passed from SEC into the
> + PEI Foundation. As such, if the platform supports collecting performance data in SEC,
> + this information is encapsulated into the data structure abstracted by this service.
> + This information is collected for the boot-strap processor (BSP) on IA-32.
> +
> + @param[in] PeiServices The pointer to the PEI Services Table.
> + @param[in] This The pointer to this instance of the PEI_SEC_PERFORMANCE_PPI.
> + @param[out] Performance The pointer to performance data collected in SEC phase.
> +
> + @retval EFI_SUCCESS The performance data was successfully returned.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetPerformancePostMemory (
> + IN CONST EFI_PEI_SERVICES **PeiServices,
> + IN PEI_SEC_PERFORMANCE_PPI *This,
> + OUT FIRMWARE_SEC_PERFORMANCE *Performance
> + );
> +
> +// /**
> +// Disables the use of Temporary RAM.
> +
> +// If present, this service is invoked by the PEI Foundation after
> +// the EFI_PEI_PERMANANT_MEMORY_INSTALLED_PPI is installed.
> +
> +// @retval EFI_SUCCESS Use of Temporary RAM was disabled.
> +// @retval EFI_INVALID_PARAMETER Temporary RAM could not be disabled.
> +
> +// **/
> +// EFI_STATUS
> +// EFIAPI
> +// SecTemporaryRamDonePostMemory (
> +// VOID
> +// );
> +
> +typedef struct {
> + UINT64 StructureSize;
> + EFI_SEC_PLATFORM_INFORMATION_RECORD *PlatformInformationRecord;
> +} SEC_PLATFORM_INFORMATION_CONTEXT;
> +
> +typedef struct {
> + EFI_HOB_GUID_TYPE Header;
> + UINT8 Revision;
> + UINT8 Reserved[3];
> + FIRMWARE_SEC_PERFORMANCE FirmwareSecPerformance;
> + SEC_PLATFORM_INFORMATION_CONTEXT Context;
> +} SEC_PLATFORM_INFORMATION_CONTEXT_HOB;
> +
> +#endif
> diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
> new file mode 100644
> index 000000000000..e29c04710941
> --- /dev/null
> +++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
> @@ -0,0 +1,64 @@
> +## @file
> +# Migrates SEC structures after permanent memory is installed.
> +#
> +# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = SecMigrationPei
> + MODULE_UNI_FILE = SecMigrationPei.uni
> + FILE_GUID = 58B35361-8922-41BC-B313-EF7ED9ADFDF7
> + MODULE_TYPE = PEIM
> + VERSION_STRING = 1.0
> + ENTRY_POINT = SecMigrationPeiInitialize
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +# VALID_ARCHITECTURES = IA32 X64 EBC
> +#
> +
> +[Sources]
> + SecMigrationPei.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + BaseMemoryLib
> + DebugLib
> + HobLib
> + MemoryAllocationLib
> + PeimEntryPoint
> + PeiServicesLib
> + PeiServicesTablePointerLib
> +
> +[Ppis]
> + ## PRODUCES
> + gRepublishSecPpiPpiGuid
> +
> + ## SOMETIMES_PRODUCES
> + gEfiTemporaryRamDonePpiGuid
> +
> + ## SOMETIME_PRODUCES
> + gEfiTemporaryRamSupportPpiGuid
> +
> + ## SOMETIMES_PRODUCES
> + gPeiSecPerformancePpiGuid
> +
> + ## SOMETIMES_CONSUMES
> + ## PRODUCES
> + gEfiSecPlatformInformationPpiGuid
> +
> + ## SOMETIMES_CONSUMES
> + ## SOMETIMES_PRODUCES
> + gEfiSecPlatformInformation2PpiGuid
> +
> +[Depex]
> + TRUE
> diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni
> new file mode 100644
> index 000000000000..62c2064ba217
> --- /dev/null
> +++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni
> @@ -0,0 +1,13 @@
> +// /** @file
> +// Migrates SEC structures after permanent memory is installed.
> +//
> +// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +// **/
> +
> +
> +#string STR_MODULE_ABSTRACT #language en-US "Migrates SEC structures after permanent memory is installed"
> +
> +#string STR_MODULE_DESCRIPTION #language en-US "Migrates SEC structures after permanent memory is installed."
> +
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index 762badf5d239..0a005bd20311 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -66,6 +66,10 @@ [Guids]
> ## Include/Guid/MicrocodePatchHob.h
> gEdkiiMicrocodePatchHobGuid = { 0xd178f11d, 0x8716, 0x418e, { 0xa1, 0x31, 0x96, 0x7d, 0x2a, 0xc4, 0x28, 0x43 }}
>
> +[Ppis]
> + ## Include/Ppi/RepublishSecPpi.h
> + gRepublishSecPpiPpiGuid = { 0x27a71b1e, 0x73ee, 0x43d6, { 0xac, 0xe3, 0x52, 0x1a, 0x2d, 0xc5, 0xd0, 0x92 }}
> +
> [Protocols]
> ## Include/Protocol/SmmCpuService.h
> gEfiSmmCpuServiceProtocolGuid = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
> diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
> index afa304128221..964720048dd7 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dsc
> +++ b/UefiCpuPkg/UefiCpuPkg.dsc
> @@ -146,6 +146,7 @@ [Components.IA32, Components.X64]
> UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
> UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
> UefiCpuPkg/SecCore/SecCore.inf
> + UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf {
> <Defines>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 4/9] MdeModulePkg/DxeIplPeim: Register for shadow on S3 shadowed boot (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
` (2 preceding siblings ...)
2020-07-02 5:15 ` [PATCH v2 3/9] UefiCpuPkg/SecMigrationPei: Add initial PEIM (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-03 14:00 ` [edk2-devel] " Laszlo Ersek
2020-07-02 5:15 ` [PATCH v2 5/9] MdeModulePkg/Core: Create Migrated FV Info Hob for calculating hash (CVE-2019-11098) Guomin Jiang
` (5 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel; +Cc: Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao
From: Jian J Wang <jian.j.wang@intel.com>
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 3 +++
MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index 3f1702854660..4ab54594ed66 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -121,6 +121,9 @@ [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## SOMETIMES_CONSUMES
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnS3Boot ## CONSUMES
+
[Depex]
gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
index d48028cea0dd..9e1831c69819 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
@@ -77,7 +77,7 @@ PeimInitializeDxeIpl (
BootMode = GetBootModeHob ();
- if (BootMode != BOOT_ON_S3_RESUME) {
+ if (BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot)) {
Status = PeiServicesRegisterForShadow (FileHandle);
if (Status == EFI_SUCCESS) {
//
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 4/9] MdeModulePkg/DxeIplPeim: Register for shadow on S3 shadowed boot (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 4/9] MdeModulePkg/DxeIplPeim: Register for shadow on S3 shadowed boot (CVE-2019-11098) Guomin Jiang
@ 2020-07-03 14:00 ` Laszlo Ersek
2020-07-03 14:23 ` Laszlo Ersek
0 siblings, 1 reply; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 14:00 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao
On 07/02/20 07:15, Guomin Jiang wrote:
> From: Jian J Wang <jian.j.wang@intel.com>
>
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 3 +++
> MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 2 +-
> 2 files changed, 4 insertions(+), 1 deletion(-)
(1) The commit message is empty, and therefore useless. Please explain
why this change is being made.
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 3f1702854660..4ab54594ed66 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -121,6 +121,9 @@ [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## SOMETIMES_CONSUMES
>
> +[Pcd]
> + gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnS3Boot ## CONSUMES
> +
> [Depex]
> gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> index d48028cea0dd..9e1831c69819 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
> @@ -77,7 +77,7 @@ PeimInitializeDxeIpl (
>
> BootMode = GetBootModeHob ();
>
> - if (BootMode != BOOT_ON_S3_RESUME) {
> + if (BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot)) {
> Status = PeiServicesRegisterForShadow (FileHandle);
> if (Status == EFI_SUCCESS) {
> //
>
(2) The above check does not seem complete. I think it should consider
"PcdMigrateTemporaryRamFirmwareVolumes".
I don't exactly understand the impact of the change, but it seems to
potentially affect even such platforms that set
"PcdMigrateTemporaryRamFirmwareVolumes" to FALSE; and that seems wrong.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 4/9] MdeModulePkg/DxeIplPeim: Register for shadow on S3 shadowed boot (CVE-2019-11098)
2020-07-03 14:00 ` [edk2-devel] " Laszlo Ersek
@ 2020-07-03 14:23 ` Laszlo Ersek
0 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 14:23 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao
On 07/03/20 16:00, Laszlo Ersek wrote:
> On 07/02/20 07:15, Guomin Jiang wrote:
>> From: Jian J Wang <jian.j.wang@intel.com>
>>
>> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>> Cc: Dandan Bi <dandan.bi@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
>> ---
>> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 3 +++
>> MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 2 +-
>> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> (1) The commit message is empty, and therefore useless. Please explain
> why this change is being made.
>
>> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>> index 3f1702854660..4ab54594ed66 100644
>> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>> @@ -121,6 +121,9 @@ [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
>> gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES
>> gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## SOMETIMES_CONSUMES
>>
>> +[Pcd]
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnS3Boot ## CONSUMES
>> +
>> [Depex]
>> gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
>>
>> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>> index d48028cea0dd..9e1831c69819 100644
>> --- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>> @@ -77,7 +77,7 @@ PeimInitializeDxeIpl (
>>
>> BootMode = GetBootModeHob ();
>>
>> - if (BootMode != BOOT_ON_S3_RESUME) {
>> + if (BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot)) {
>> Status = PeiServicesRegisterForShadow (FileHandle);
>> if (Status == EFI_SUCCESS) {
>> //
>>
>
> (2) The above check does not seem complete. I think it should consider
> "PcdMigrateTemporaryRamFirmwareVolumes".
>
> I don't exactly understand the impact of the change, but it seems to
> potentially affect even such platforms that set
> "PcdMigrateTemporaryRamFirmwareVolumes" to FALSE; and that seems wrong.
... On further consideration, this patch seems to be fixing a
preexistent bug that is not related to the CVE at all. I think this
issue was simply exposed when testing the new feature. Is that right?
If that's correct, then please explain this very clearly in the commit
message.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 5/9] MdeModulePkg/Core: Create Migrated FV Info Hob for calculating hash (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
` (3 preceding siblings ...)
2020-07-02 5:15 ` [PATCH v2 4/9] MdeModulePkg/DxeIplPeim: Register for shadow on S3 shadowed boot (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-03 14:03 ` [edk2-devel] " Laszlo Ersek
2020-07-02 5:15 ` [PATCH v2 6/9] SecurityPkg/Tcg2Pei: Use " Guomin Jiang
` (4 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel
Cc: Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao, Debkumar De,
Harry Han, Catharine West
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
When we allocate pool to save rebased the PEIMs, the address will change
randomly, therefore the hash will change and result PCR0 change as well.
To avoid this, we save the raw PEIMs and use it to calculate hash.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 15 +++++++++++++
MdeModulePkg/Core/Pei/PeiMain.h | 1 +
MdeModulePkg/Core/Pei/PeiMain.inf | 1 +
MdeModulePkg/Include/Guid/MigratedFvInfo.h | 22 +++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +++
5 files changed, 42 insertions(+)
create mode 100644 MdeModulePkg/Include/Guid/MigratedFvInfo.h
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index ef88b3423376..7e1ac38f35c8 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -1223,10 +1223,12 @@ EvacuateTempRam (
EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
EFI_FIRMWARE_VOLUME_HEADER *ChildFvHeader;
EFI_FIRMWARE_VOLUME_HEADER *MigratedFvHeader;
+ EFI_FIRMWARE_VOLUME_HEADER *RawDataFvHeader;
EFI_FIRMWARE_VOLUME_HEADER *MigratedChildFvHeader;
PEI_CORE_FV_HANDLE PeiCoreFvHandle;
EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
+ EDKII_MIGRATED_FV_INFO MigratedFvInfo;
ASSERT (Private->PeiMemoryInstalled);
@@ -1270,6 +1272,13 @@ EvacuateTempRam (
);
ASSERT_EFI_ERROR (Status);
+ Status = PeiServicesAllocatePages (
+ EfiBootServicesCode,
+ EFI_SIZE_TO_PAGES ((UINTN) FvHeader->FvLength),
+ (EFI_PHYSICAL_ADDRESS *) &RawDataFvHeader
+ );
+ ASSERT_EFI_ERROR (Status);
+
DEBUG ((
DEBUG_VERBOSE,
" Migrating FV[%d] from 0x%08X to 0x%08X\n",
@@ -1279,6 +1288,12 @@ EvacuateTempRam (
));
CopyMem (MigratedFvHeader, FvHeader, (UINTN) FvHeader->FvLength);
+ CopyMem (RawDataFvHeader, MigratedFvHeader, (UINTN) FvHeader->FvLength);
+ MigratedFvInfo.FvOrgBase = (UINT32) (UINTN) FvHeader;
+ MigratedFvInfo.FvNewBase = (UINT32) (UINTN) MigratedFvHeader;
+ MigratedFvInfo.FvDataBase = (UINT32) (UINTN) RawDataFvHeader;
+ MigratedFvInfo.FvLength = (UINT32) (UINTN) FvHeader->FvLength;
+ BuildGuidDataHob (&gEdkiiMigratedFvInfoGuid, &MigratedFvInfo, sizeof (MigratedFvInfo));
//
// Migrate any children for this FV now
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
index b0101dba5e30..cbf74d5b9d9a 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -44,6 +44,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/FirmwareFileSystem2.h>
#include <Guid/FirmwareFileSystem3.h>
#include <Guid/AprioriFileName.h>
+#include <Guid/MigratedFvInfo.h>
///
/// It is an FFS type extension used for PeiFindFileEx. It indicates current
diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf
index 5ff14100a65f..c80d16b4efa6 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.inf
+++ b/MdeModulePkg/Core/Pei/PeiMain.inf
@@ -77,6 +77,7 @@ [Guids]
## CONSUMES ## GUID # Used to compare with FV's file system GUID and get the FV's file system format
gEfiFirmwareFileSystem3Guid
gStatusCodeCallbackGuid
+ gEdkiiMigratedFvInfoGuid ## SOMETIMES_PRODUCES ## HOB
[Ppis]
gEfiPeiStatusCodePpiGuid ## SOMETIMES_CONSUMES # PeiReportStatusService is not ready if this PPI doesn't exist
diff --git a/MdeModulePkg/Include/Guid/MigratedFvInfo.h b/MdeModulePkg/Include/Guid/MigratedFvInfo.h
new file mode 100644
index 000000000000..061c17ed0e48
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/MigratedFvInfo.h
@@ -0,0 +1,22 @@
+/** @file
+ Migrated FV information
+
+Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __EDKII_MIGRATED_FV_INFO_GUID_H__
+#define __EDKII_MIGRATED_FV_INFO_GUID_H__
+
+typedef struct {
+ UINT32 FvOrgBase; // original FV address
+ UINT32 FvNewBase; // new FV address
+ UINT32 FvDataBase; // original FV data
+ UINT32 FvLength; // Fv Length
+} EDKII_MIGRATED_FV_INFO;
+
+extern EFI_GUID gEdkiiMigratedFvInfoGuid;
+
+#endif // #ifndef __EDKII_MIGRATED_FV_INFO_GUID_H__
+
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 843e963ad34b..5e25cbe98ada 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -389,6 +389,9 @@ [Guids]
## GUID indicates the capsule is to store Capsule On Disk file names.
gEdkiiCapsuleOnDiskNameGuid = { 0x98c80a4f, 0xe16b, 0x4d11, { 0x93, 0x9a, 0xab, 0xe5, 0x61, 0x26, 0x3, 0x30 } }
+ ## Include/Guid/MigratedFvInfo.h
+ gEdkiiMigratedFvInfoGuid = { 0xc1ab12f7, 0x74aa, 0x408d, { 0xa2, 0xf4, 0xc6, 0xce, 0xfd, 0x17, 0x98, 0x71 } }
+
[Ppis]
## Include/Ppi/AtaController.h
gPeiAtaControllerPpiGuid = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 5/9] MdeModulePkg/Core: Create Migrated FV Info Hob for calculating hash (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 5/9] MdeModulePkg/Core: Create Migrated FV Info Hob for calculating hash (CVE-2019-11098) Guomin Jiang
@ 2020-07-03 14:03 ` Laszlo Ersek
0 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 14:03 UTC (permalink / raw)
To: devel, guomin.jiang
Cc: Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao, Debkumar De,
Harry Han, Catharine West
On 07/02/20 07:15, Guomin Jiang wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> When we allocate pool to save rebased the PEIMs, the address will change
> randomly, therefore the hash will change and result PCR0 change as well.
> To avoid this, we save the raw PEIMs and use it to calculate hash.
(1) Please extend the commit message. We should state that
"gEdkiiMigratedFvInfoGuid" HOBs are *never* produced if
"PcdMigrateTemporaryRamFirmwareVolumes" is FALSE.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Harry Han <harry.han@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
> ---
> MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 15 +++++++++++++
> MdeModulePkg/Core/Pei/PeiMain.h | 1 +
> MdeModulePkg/Core/Pei/PeiMain.inf | 1 +
> MdeModulePkg/Include/Guid/MigratedFvInfo.h | 22 +++++++++++++++++++
> MdeModulePkg/MdeModulePkg.dec | 3 +++
> 5 files changed, 42 insertions(+)
> create mode 100644 MdeModulePkg/Include/Guid/MigratedFvInfo.h
>
> diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> index ef88b3423376..7e1ac38f35c8 100644
> --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> @@ -1223,10 +1223,12 @@ EvacuateTempRam (
> EFI_FIRMWARE_VOLUME_HEADER *FvHeader;
> EFI_FIRMWARE_VOLUME_HEADER *ChildFvHeader;
> EFI_FIRMWARE_VOLUME_HEADER *MigratedFvHeader;
> + EFI_FIRMWARE_VOLUME_HEADER *RawDataFvHeader;
> EFI_FIRMWARE_VOLUME_HEADER *MigratedChildFvHeader;
>
> PEI_CORE_FV_HANDLE PeiCoreFvHandle;
> EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi;
> + EDKII_MIGRATED_FV_INFO MigratedFvInfo;
>
> ASSERT (Private->PeiMemoryInstalled);
>
> @@ -1270,6 +1272,13 @@ EvacuateTempRam (
> );
> ASSERT_EFI_ERROR (Status);
>
> + Status = PeiServicesAllocatePages (
> + EfiBootServicesCode,
> + EFI_SIZE_TO_PAGES ((UINTN) FvHeader->FvLength),
> + (EFI_PHYSICAL_ADDRESS *) &RawDataFvHeader
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> DEBUG ((
> DEBUG_VERBOSE,
> " Migrating FV[%d] from 0x%08X to 0x%08X\n",
> @@ -1279,6 +1288,12 @@ EvacuateTempRam (
> ));
>
> CopyMem (MigratedFvHeader, FvHeader, (UINTN) FvHeader->FvLength);
> + CopyMem (RawDataFvHeader, MigratedFvHeader, (UINTN) FvHeader->FvLength);
> + MigratedFvInfo.FvOrgBase = (UINT32) (UINTN) FvHeader;
> + MigratedFvInfo.FvNewBase = (UINT32) (UINTN) MigratedFvHeader;
> + MigratedFvInfo.FvDataBase = (UINT32) (UINTN) RawDataFvHeader;
> + MigratedFvInfo.FvLength = (UINT32) (UINTN) FvHeader->FvLength;
> + BuildGuidDataHob (&gEdkiiMigratedFvInfoGuid, &MigratedFvInfo, sizeof (MigratedFvInfo));
>
> //
> // Migrate any children for this FV now
(2) Please repeat the same statement here (from the commit message).
That statement is very important for understanding the control flow of
this feature. Some of the other modules do not consult
"PcdMigrateTemporaryRamFirmwareVolumes"; instead, they consume
"gEdkiiMigratedFvInfoGuid" HOBs.
We should explain (both in the commit message and in the code) that
"PcdMigrateTemporaryRamFirmwareVolumes" controls the *latter* kind of
modules too, via the production of "gEdkiiMigratedFvInfoGuid" HOBs. (In
other words, if the PCD is FALSE, such HOBs are never produced.)
Thanks
Laszlo
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
> index b0101dba5e30..cbf74d5b9d9a 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -44,6 +44,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Guid/FirmwareFileSystem2.h>
> #include <Guid/FirmwareFileSystem3.h>
> #include <Guid/AprioriFileName.h>
> +#include <Guid/MigratedFvInfo.h>
>
> ///
> /// It is an FFS type extension used for PeiFindFileEx. It indicates current
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf
> index 5ff14100a65f..c80d16b4efa6 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> @@ -77,6 +77,7 @@ [Guids]
> ## CONSUMES ## GUID # Used to compare with FV's file system GUID and get the FV's file system format
> gEfiFirmwareFileSystem3Guid
> gStatusCodeCallbackGuid
> + gEdkiiMigratedFvInfoGuid ## SOMETIMES_PRODUCES ## HOB
>
> [Ppis]
> gEfiPeiStatusCodePpiGuid ## SOMETIMES_CONSUMES # PeiReportStatusService is not ready if this PPI doesn't exist
> diff --git a/MdeModulePkg/Include/Guid/MigratedFvInfo.h b/MdeModulePkg/Include/Guid/MigratedFvInfo.h
> new file mode 100644
> index 000000000000..061c17ed0e48
> --- /dev/null
> +++ b/MdeModulePkg/Include/Guid/MigratedFvInfo.h
> @@ -0,0 +1,22 @@
> +/** @file
> + Migrated FV information
> +
> +Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __EDKII_MIGRATED_FV_INFO_GUID_H__
> +#define __EDKII_MIGRATED_FV_INFO_GUID_H__
> +
> +typedef struct {
> + UINT32 FvOrgBase; // original FV address
> + UINT32 FvNewBase; // new FV address
> + UINT32 FvDataBase; // original FV data
> + UINT32 FvLength; // Fv Length
> +} EDKII_MIGRATED_FV_INFO;
> +
> +extern EFI_GUID gEdkiiMigratedFvInfoGuid;
> +
> +#endif // #ifndef __EDKII_MIGRATED_FV_INFO_GUID_H__
> +
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 843e963ad34b..5e25cbe98ada 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -389,6 +389,9 @@ [Guids]
> ## GUID indicates the capsule is to store Capsule On Disk file names.
> gEdkiiCapsuleOnDiskNameGuid = { 0x98c80a4f, 0xe16b, 0x4d11, { 0x93, 0x9a, 0xab, 0xe5, 0x61, 0x26, 0x3, 0x30 } }
>
> + ## Include/Guid/MigratedFvInfo.h
> + gEdkiiMigratedFvInfoGuid = { 0xc1ab12f7, 0x74aa, 0x408d, { 0xa2, 0xf4, 0xc6, 0xce, 0xfd, 0x17, 0x98, 0x71 } }
> +
> [Ppis]
> ## Include/Ppi/AtaController.h
> gPeiAtaControllerPpiGuid = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 6/9] SecurityPkg/Tcg2Pei: Use Migrated FV Info Hob for calculating hash (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
` (4 preceding siblings ...)
2020-07-02 5:15 ` [PATCH v2 5/9] MdeModulePkg/Core: Create Migrated FV Info Hob for calculating hash (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-02 5:15 ` [PATCH v2 7/9] MdeModulePkg/Core: Add switch to enable or disable TOCTOU feature (CVE-2019-11098) Guomin Jiang
` (3 subsequent siblings)
9 siblings, 0 replies; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel; +Cc: Jiewen Yao, Jian J Wang, Chao Zhang, Qi Zhang, Rahul Kumar
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
When we allocate pool to save rebased the PEIMs, the address will change
randomly, therefore the hash will change and result PCR0 change as well.
To avoid this, we save the raw PEIMs and use it to calculate hash.
The Tcg2Pei calculate the hash and it use the Migrated FV Info.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 31 ++++++++++++++++++++++++++---
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 +
2 files changed, 29 insertions(+), 3 deletions(-)
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index 4852d8690617..651a60c1f0e2 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -21,6 +21,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/TcgEventHob.h>
#include <Guid/MeasuredFvHob.h>
#include <Guid/TpmInstance.h>
+#include <Guid/MigratedFvInfo.h>
#include <Library/DebugLib.h>
#include <Library/BaseMemoryLib.h>
@@ -536,6 +537,10 @@ MeasureFvImage (
EDKII_PEI_FIRMWARE_VOLUME_INFO_PREHASHED_FV_PPI *PrehashedFvPpi;
HASH_INFO *PreHashInfo;
UINT32 HashAlgoMask;
+ EFI_PHYSICAL_ADDRESS FvOrgBase;
+ EFI_PHYSICAL_ADDRESS FvDataBase;
+ EFI_PEI_HOB_POINTERS Hob;
+ EDKII_MIGRATED_FV_INFO *MigratedFvInfo;
//
// Check Excluded FV list
@@ -621,6 +626,26 @@ MeasureFvImage (
Instance++;
} while (!EFI_ERROR(Status));
+ //
+ // Search the matched migration FV info
+ //
+ FvOrgBase = FvBase;
+ FvDataBase = FvBase;
+ Hob.Raw = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
+ while (Hob.Raw != NULL) {
+ MigratedFvInfo = GET_GUID_HOB_DATA (Hob);
+ if ((MigratedFvInfo->FvNewBase == (UINT32) FvBase) && (MigratedFvInfo->FvLength == (UINT32) FvLength)) {
+ //
+ // Found the migrated FV info
+ //
+ FvOrgBase = (EFI_PHYSICAL_ADDRESS) (UINTN) MigratedFvInfo->FvOrgBase;
+ FvDataBase = (EFI_PHYSICAL_ADDRESS) (UINTN) MigratedFvInfo->FvDataBase;
+ break;
+ }
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ Hob.Raw = GetNextGuidHob (&gEdkiiMigratedFvInfoGuid, Hob.Raw);
+ }
+
//
// Init the log event for FV measurement
//
@@ -631,13 +656,13 @@ MeasureFvImage (
if (FvName != NULL) {
AsciiSPrint ((CHAR8 *)FvBlob2.BlobDescription, sizeof(FvBlob2.BlobDescription), "Fv(%g)", FvName);
}
- FvBlob2.BlobBase = FvBase;
+ FvBlob2.BlobBase = FvOrgBase;
FvBlob2.BlobLength = FvLength;
TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2;
TcgEventHdr.EventSize = sizeof (FvBlob2);
EventData = &FvBlob2;
} else {
- FvBlob.BlobBase = FvBase;
+ FvBlob.BlobBase = FvOrgBase;
FvBlob.BlobLength = FvLength;
TcgEventHdr.PCRIndex = 0;
TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB;
@@ -672,7 +697,7 @@ MeasureFvImage (
//
Status = HashLogExtendEvent (
0,
- (UINT8*) (UINTN) FvBase, // HashData
+ (UINT8*) (UINTN) FvDataBase, // HashData
(UINTN) FvLength, // HashDataLen
&TcgEventHdr, // EventHdr
EventData // EventData
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
index 3d361e8859e7..367df21eedaf 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
@@ -63,6 +63,7 @@ [Guids]
gTcgEvent2EntryHobGuid ## PRODUCES ## HOB
gEfiTpmDeviceInstanceNoneGuid ## SOMETIMES_PRODUCES ## GUID # TPM device identifier
gEfiTpmDeviceInstanceTpm12Guid ## SOMETIMES_PRODUCES ## GUID # TPM device identifier
+ gEdkiiMigratedFvInfoGuid ## SOMETIMES_CONSUMES ## HOB
[Ppis]
gEfiPeiFirmwareVolumeInfoPpiGuid ## SOMETIMES_CONSUMES ## NOTIFY
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH v2 7/9] MdeModulePkg/Core: Add switch to enable or disable TOCTOU feature (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
` (5 preceding siblings ...)
2020-07-02 5:15 ` [PATCH v2 6/9] SecurityPkg/Tcg2Pei: Use " Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-03 12:48 ` [edk2-devel] " Laszlo Ersek
2020-07-02 5:15 ` [PATCH v2 8/9] UefiCpuPkg/SecMigrationPei: Add switch to control if produce PPI (CVE-2019-11098) Guomin Jiang
` (2 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel
Cc: Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao, Debkumar De,
Harry Han, Catharine West
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
Add total switch to enable or disable TOCTOU feature, the vulnerability is
critical, so the switch is on normally but if you can disable it according
to your needs.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Debkumar De <debkumar.de@intel.com>
Cc: Harry Han <harry.han@intel.com>
Cc: Catharine West <catharine.west@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
MdeModulePkg/Core/Pei/PeiMain.inf | 1 +
MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 5 +++--
MdeModulePkg/MdeModulePkg.dec | 5 +++++
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf
index c80d16b4efa6..0cf357371a16 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.inf
+++ b/MdeModulePkg/Core/Pei/PeiMain.inf
@@ -111,6 +111,7 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnS3Boot ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnBoot ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes ## CONSUMES
# [BootMode]
# S3_RESUME ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index 802cd239e2eb..bc78c3f8ad59 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -419,8 +419,9 @@ PeiCore (
}
} else {
if (
- (!(PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnBoot)) ||
- ((PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot))
+ ((!(PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnBoot)) ||
+ ((PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot))) &&
+ PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)
) {
DEBUG ((DEBUG_VERBOSE, "PPI lists before temporary RAM evacuation:\n"));
DumpPpiList (&PrivateData);
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 5e25cbe98ada..0a5a167f3e8b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1223,6 +1223,11 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# @Prompt Shadow Peim and PeiCore on boot
gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnBoot|TRUE|BOOLEAN|0x30001029
+ ## Indicate if to evacuate from temporary to permanent memory.
+ # TRUE - Evacuate from temporary memory
+ # FALSE - Keep the original behavior
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes|TRUE|BOOLEAN|0x3000102A
+
## The mask is used to control memory profile behavior.<BR><BR>
# BIT0 - Enable UEFI memory profile.<BR>
# BIT1 - Enable SMRAM profile.<BR>
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 7/9] MdeModulePkg/Core: Add switch to enable or disable TOCTOU feature (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 7/9] MdeModulePkg/Core: Add switch to enable or disable TOCTOU feature (CVE-2019-11098) Guomin Jiang
@ 2020-07-03 12:48 ` Laszlo Ersek
0 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 12:48 UTC (permalink / raw)
To: devel, guomin.jiang
Cc: Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao, Debkumar De,
Harry Han, Catharine West
On 07/02/20 07:15, Guomin Jiang wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> Add total switch to enable or disable TOCTOU feature, the vulnerability is
> critical, so the switch is on normally but if you can disable it according
> to your needs.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Harry Han <harry.han@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
> ---
> MdeModulePkg/Core/Pei/PeiMain.inf | 1 +
> MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 5 +++--
> MdeModulePkg/MdeModulePkg.dec | 5 +++++
> 3 files changed, 9 insertions(+), 2 deletions(-)
(1) The subject line of the patch is wrong. The expression "TOCTOU
feature" makes no sense.
Instead, the patch adds a PCD for controlling the "temporary RAM
evacuation" feature that is implemented in patch#1 in this series.
Please fix both the subject line, and the commit message -- as both
contain the wrong expression "TOCTOU feature".
>
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf
> index c80d16b4efa6..0cf357371a16 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.inf
> +++ b/MdeModulePkg/Core/Pei/PeiMain.inf
> @@ -111,6 +111,7 @@ [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnS3Boot ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnBoot ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes ## CONSUMES
>
> # [BootMode]
> # S3_RESUME ## SOMETIMES_CONSUMES
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 802cd239e2eb..bc78c3f8ad59 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -419,8 +419,9 @@ PeiCore (
> }
> } else {
> if (
> - (!(PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnBoot)) ||
> - ((PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot))
> + ((!(PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnBoot)) ||
> + ((PrivateData.HobList.HandoffInformationTable->BootMode == BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot))) &&
> + PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)
(2) The indentation of the new code is wrong. Before the patch, we have
(A) ||
(B)
After the patch, we have
((A) ||
(B)) &&
C
The indentation of the line with "B" is wrong. It should be:
((A) ||
(B)) &&
C
But, anyway, I've suggested under patch#1 a different way for expressing
the same condition. So ultimately, in this patch, we should produce:
BOOLEAN Shadow;
Shadow = FALSE;
if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) {
if (PrivateData.HobList.HandoffInformationTable->BootMode ==
BOOT_ON_S3_RESUME) {
Shadow = PcdGetBool (PcdShadowPeimOnS3Boot);
} else {
Shadow = PcdGetBool (PcdShadowPeimOnBoot);
}
}
if (Shadow) {
//
// ...
//
}
> ) {
> DEBUG ((DEBUG_VERBOSE, "PPI lists before temporary RAM evacuation:\n"));
> DumpPpiList (&PrivateData);
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 5e25cbe98ada..0a5a167f3e8b 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1223,6 +1223,11 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
> # @Prompt Shadow Peim and PeiCore on boot
> gEfiMdeModulePkgTokenSpaceGuid.PcdShadowPeimOnBoot|TRUE|BOOLEAN|0x30001029
>
> + ## Indicate if to evacuate from temporary to permanent memory.
> + # TRUE - Evacuate from temporary memory
(3) Please drop the word "from".
> + # FALSE - Keep the original behavior
(4) You mean "original" as "before patch#1". Because, if the PCD is set
to FALSE, then the feature introduced in patch#1 is disabled.
But the word "original" lacks context when someone looks at the DEC
file, later.
Please explain unambiguously what happens when the PCD is set to FALSE.
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes|TRUE|BOOLEAN|0x3000102A
> +
> ## The mask is used to control memory profile behavior.<BR><BR>
> # BIT0 - Enable UEFI memory profile.<BR>
> # BIT1 - Enable SMRAM profile.<BR>
>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 8/9] UefiCpuPkg/SecMigrationPei: Add switch to control if produce PPI (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
` (6 preceding siblings ...)
2020-07-02 5:15 ` [PATCH v2 7/9] MdeModulePkg/Core: Add switch to enable or disable TOCTOU feature (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-03 14:05 ` [edk2-devel] " Laszlo Ersek
2020-07-02 5:15 ` [PATCH v2 9/9] UefiCpuPkg/CpuMpPei: Enable paging and set NP flag to avoid TOCTOU (CVE-2019-11098) Guomin Jiang
2020-07-03 14:06 ` [edk2-devel] [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Laszlo Ersek
9 siblings, 1 reply; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
SecMigrationPei create RepublishSecPpi, if the TOCTOU switch is off,
the Ppi is meaningless, so relate it with TOCTOU switch to avoid
producing useless PPI.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
UefiCpuPkg/SecMigrationPei/SecMigrationPei.c | 8 +++++---
UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf | 4 ++++
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
index f96013b09b21..ab8066e8e0de 100644
--- a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
+++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
@@ -363,10 +363,12 @@ SecMigrationPeiInitialize (
IN CONST EFI_PEI_SERVICES **PeiServices
)
{
- EFI_STATUS Status;
+ EFI_STATUS Status = EFI_SUCCESS;
- Status = PeiServicesInstallPpi (&mEdkiiRepublishSecPpiDescriptor);
- ASSERT_EFI_ERROR (Status);
+ if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) {
+ Status = PeiServicesInstallPpi (&mEdkiiRepublishSecPpiDescriptor);
+ ASSERT_EFI_ERROR (Status);
+ }
return Status;
}
diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
index e29c04710941..8edbd3aa23a9 100644
--- a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
+++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
@@ -60,5 +60,9 @@ [Ppis]
## SOMETIMES_PRODUCES
gEfiSecPlatformInformation2PpiGuid
+[Pcd]
+ ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes
+
[Depex]
TRUE
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 8/9] UefiCpuPkg/SecMigrationPei: Add switch to control if produce PPI (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 8/9] UefiCpuPkg/SecMigrationPei: Add switch to control if produce PPI (CVE-2019-11098) Guomin Jiang
@ 2020-07-03 14:05 ` Laszlo Ersek
0 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 14:05 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Eric Dong, Ray Ni, Rahul Kumar
On 07/02/20 07:15, Guomin Jiang wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> SecMigrationPei create RepublishSecPpi, if the TOCTOU switch is off,
> the Ppi is meaningless, so relate it with TOCTOU switch to avoid
> producing useless PPI.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
> ---
> UefiCpuPkg/SecMigrationPei/SecMigrationPei.c | 8 +++++---
> UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf | 4 ++++
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
> index f96013b09b21..ab8066e8e0de 100644
> --- a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
> +++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
> @@ -363,10 +363,12 @@ SecMigrationPeiInitialize (
> IN CONST EFI_PEI_SERVICES **PeiServices
> )
> {
> - EFI_STATUS Status;
> + EFI_STATUS Status = EFI_SUCCESS;
>
> - Status = PeiServicesInstallPpi (&mEdkiiRepublishSecPpiDescriptor);
> - ASSERT_EFI_ERROR (Status);
> + if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) {
> + Status = PeiServicesInstallPpi (&mEdkiiRepublishSecPpiDescriptor);
> + ASSERT_EFI_ERROR (Status);
> + }
>
> return Status;
> }
> diff --git a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
> index e29c04710941..8edbd3aa23a9 100644
> --- a/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
> +++ b/UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
> @@ -60,5 +60,9 @@ [Ppis]
> ## SOMETIMES_PRODUCES
> gEfiSecPlatformInformation2PpiGuid
>
> +[Pcd]
> + ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdMigrateTemporaryRamFirmwareVolumes
> +
> [Depex]
> TRUE
>
(1) This patch should be squashed into:
"UefiCpuPkg/SecMigrationPei: Add initial PEIM"
Thanks.
Laszlo
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 9/9] UefiCpuPkg/CpuMpPei: Enable paging and set NP flag to avoid TOCTOU (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
` (7 preceding siblings ...)
2020-07-02 5:15 ` [PATCH v2 8/9] UefiCpuPkg/SecMigrationPei: Add switch to control if produce PPI (CVE-2019-11098) Guomin Jiang
@ 2020-07-02 5:15 ` Guomin Jiang
2020-07-03 13:11 ` [edk2-devel] " Laszlo Ersek
2020-07-03 14:06 ` [edk2-devel] [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Laszlo Ersek
9 siblings, 1 reply; 25+ messages in thread
From: Guomin Jiang @ 2020-07-02 5:15 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
To avoid the TOCTOU, enable paging and set Not Present flag so when
access any code in the flash range, it will trigger #NP exception.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 3 +++
UefiCpuPkg/CpuMpPei/CpuPaging.c | 17 +++++++++++++++--
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
index caead3ce34d4..fd50b55f06cb 100644
--- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
@@ -46,6 +46,9 @@ [LibraryClasses]
BaseMemoryLib
CpuLib
+[Guids]
+ gEdkiiMigratedFvInfoGuid ## SOMETIMES_CONSUMES ## HOB
+
[Ppis]
gEfiPeiMpServicesPpiGuid ## PRODUCES
gEfiSecPlatformInformationPpiGuid ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
index d0cbebf70bbf..af4069b42cdb 100644
--- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
+++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
@@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/MemoryAllocationLib.h>
#include <Library/CpuLib.h>
#include <Library/BaseLib.h>
+#include <Guid/MigratedFvInfo.h>
#include "CpuMpPei.h"
@@ -605,6 +606,8 @@ MemoryDiscoveredPpiNotifyCallback (
EFI_STATUS Status;
BOOLEAN InitStackGuard;
BOOLEAN InterruptState;
+ EDKII_MIGRATED_FV_INFO *MigratedFvInfo;
+ EFI_PEI_HOB_POINTERS Hob;
InterruptState = SaveAndDisableInterrupts ();
Status = MigrateGdt ();
@@ -617,9 +620,9 @@ MemoryDiscoveredPpiNotifyCallback (
// the task switch (for the sake of stack switch).
//
InitStackGuard = FALSE;
- if (IsIa32PaeSupported () && PcdGetBool (PcdCpuStackGuard)) {
+ if (IsIa32PaeSupported ()) {
EnablePaging ();
- InitStackGuard = TRUE;
+ InitStackGuard = PcdGetBool (PcdCpuStackGuard);
}
Status = InitializeCpuMpWorker ((CONST EFI_PEI_SERVICES **)PeiServices);
@@ -629,6 +632,16 @@ MemoryDiscoveredPpiNotifyCallback (
SetupStackGuardPage ();
}
+ Hob.Raw = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
+ while (Hob.Raw != NULL) {
+ MigratedFvInfo = GET_GUID_HOB_DATA (Hob);
+ ConvertMemoryPageAttributes (MigratedFvInfo->FvOrgBase, MigratedFvInfo->FvLength, 0);
+
+ Hob.Raw = GET_NEXT_HOB (Hob);
+ Hob.Raw = GetNextGuidHob (&gEdkiiMigratedFvInfoGuid, Hob.Raw);
+ }
+ CpuFlushTlb ();
+
return Status;
}
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [edk2-devel] [PATCH v2 9/9] UefiCpuPkg/CpuMpPei: Enable paging and set NP flag to avoid TOCTOU (CVE-2019-11098)
2020-07-02 5:15 ` [PATCH v2 9/9] UefiCpuPkg/CpuMpPei: Enable paging and set NP flag to avoid TOCTOU (CVE-2019-11098) Guomin Jiang
@ 2020-07-03 13:11 ` Laszlo Ersek
0 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 13:11 UTC (permalink / raw)
To: devel, guomin.jiang; +Cc: Eric Dong, Ray Ni, Rahul Kumar
On 07/02/20 07:15, Guomin Jiang wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614
>
> To avoid the TOCTOU, enable paging and set Not Present flag so when
> access any code in the flash range, it will trigger #NP exception.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
> ---
> UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 3 +++
> UefiCpuPkg/CpuMpPei/CpuPaging.c | 17 +++++++++++++++--
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> index caead3ce34d4..fd50b55f06cb 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> @@ -46,6 +46,9 @@ [LibraryClasses]
> BaseMemoryLib
> CpuLib
>
> +[Guids]
> + gEdkiiMigratedFvInfoGuid ## SOMETIMES_CONSUMES ## HOB
> +
> [Ppis]
> gEfiPeiMpServicesPpiGuid ## PRODUCES
> gEfiSecPlatformInformationPpiGuid ## SOMETIMES_CONSUMES
> diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> index d0cbebf70bbf..af4069b42cdb 100644
> --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c
> +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c
> @@ -12,6 +12,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Library/MemoryAllocationLib.h>
> #include <Library/CpuLib.h>
> #include <Library/BaseLib.h>
> +#include <Guid/MigratedFvInfo.h>
>
> #include "CpuMpPei.h"
>
> @@ -605,6 +606,8 @@ MemoryDiscoveredPpiNotifyCallback (
> EFI_STATUS Status;
> BOOLEAN InitStackGuard;
> BOOLEAN InterruptState;
> + EDKII_MIGRATED_FV_INFO *MigratedFvInfo;
> + EFI_PEI_HOB_POINTERS Hob;
>
> InterruptState = SaveAndDisableInterrupts ();
> Status = MigrateGdt ();
> @@ -617,9 +620,9 @@ MemoryDiscoveredPpiNotifyCallback (
> // the task switch (for the sake of stack switch).
> //
> InitStackGuard = FALSE;
> - if (IsIa32PaeSupported () && PcdGetBool (PcdCpuStackGuard)) {
> + if (IsIa32PaeSupported ()) {
> EnablePaging ();
> - InitStackGuard = TRUE;
> + InitStackGuard = PcdGetBool (PcdCpuStackGuard);
> }
>
> Status = InitializeCpuMpWorker ((CONST EFI_PEI_SERVICES **)PeiServices);
> @@ -629,6 +632,16 @@ MemoryDiscoveredPpiNotifyCallback (
> SetupStackGuardPage ();
> }
>
> + Hob.Raw = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
> + while (Hob.Raw != NULL) {
> + MigratedFvInfo = GET_GUID_HOB_DATA (Hob);
> + ConvertMemoryPageAttributes (MigratedFvInfo->FvOrgBase, MigratedFvInfo->FvLength, 0);
> +
> + Hob.Raw = GET_NEXT_HOB (Hob);
> + Hob.Raw = GetNextGuidHob (&gEdkiiMigratedFvInfoGuid, Hob.Raw);
> + }
> + CpuFlushTlb ();
> +
> return Status;
> }
>
>
(1) This patch calls EnablePaging() even if no
"gEdkiiMigratedFvInfoGuid" HOB exists.
In other words, assume "PcdMigrateTemporaryRamFirmwareVolumes" is FALSE.
- In that case, PeiCore() will not call EvacuateTempRam().
- Consequently, zero "gEdkiiMigratedFvInfoGuid" HOBs are going to be built.
- Consequently, this patch will never call ConvertMemoryPageAttributes()
Why do we call EnablePaging() then (assuming IsIa32PaeSupported()
returns TRUE)?
(2) Consider the opposite case. Assume IsIa32PaeSupported() returns
FALSE. Further assume that at least one "gEdkiiMigratedFvInfoGuid" HOB
exists.
Then ConvertMemoryPageAttributes() will be called, without us ever
calling EnablePaging(). That doesn't seem right.
I suggest:
BOOLEAN InitStackGuard;
EFI_PEI_HOB_POINTERS Hob;
InitStackGuard = FALSE;
Hob.Raw = NULL;
//
// Both the "stack guard" feature and the "temp RAM evacuation"
// feature depend on IA32 PAE support.
//
if (IsIa32PaeSupported ()) {
InitStackGuard = PcdGetBool (PcdCpuStackGuard);
Hob.Raw = GetFirstGuidHob (&gEdkiiMigratedFvInfoGuid);
}
//
// If either feature is active, then paging is required; enable it.
//
if (InitStackGuard || Hob.Raw != NULL) {
EnablePaging ();
}
/* ... */
if (InitStackGuard) {
SetupStackGuardPage ();
}
/* note: no need to call GetFirstGuidHob() again! */
while (Hob.Raw != NULL) {
/* ... */
}
CpuFlushTlb ();
Thanks
Laszlo
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098)
2020-07-02 5:15 [PATCH v2 0/9] Migrate Pointer from flash to permanent memory (CVE-2019-11098) Guomin Jiang
` (8 preceding siblings ...)
2020-07-02 5:15 ` [PATCH v2 9/9] UefiCpuPkg/CpuMpPei: Enable paging and set NP flag to avoid TOCTOU (CVE-2019-11098) Guomin Jiang
@ 2020-07-03 14:06 ` Laszlo Ersek
9 siblings, 0 replies; 25+ messages in thread
From: Laszlo Ersek @ 2020-07-03 14:06 UTC (permalink / raw)
To: devel, guomin.jiang, Michael Kubacki
Cc: Jian J Wang, Hao A Wu, Dandan Bi, Liming Gao, Debkumar De,
Harry Han, Catharine West, Eric Dong, Ray Ni, Rahul Kumar,
Jiewen Yao, Chao Zhang, Qi Zhang, Ard Biesheuvel (ARM address)
Hi,
I'm adding Michael Kubacki's new email address to the "To:" list, as
Michael has authored a significant portion of this code, plus he seems
to have contributed a large part of the design, in
<https://bugzilla.tianocore.org/show_bug.cgi?id=1614>.
Also CC'ing Ard.
Comments below.
On 07/02/20 07:15, Guomin Jiang wrote:
> The TOCTOU vulnerability allow that the physical present person to replace the code with the normal BootGuard check and PCR0 value.
> The issue occur when BootGuard measure IBB and access flash code after NEM disable.
> the reason why we access the flash code is that we have some pointer to flash.
> To avoid this vulnerability, we need to convert those pointers, the patch series do this work and make sure that no code will access flash address.
I've now read through the comments in
<https://bugzilla.tianocore.org/show_bug.cgi?id=1614>, and I've also
checked the slides at:
[1]
https://conference.hitb.org/hitbsecconf2019ams/sessions/now-you-see-it-toctou-attacks-against-secure-boot-and-bootguard/
https://conference.hitb.org/hitbsecconf2019ams/materials/D1T1%20-%20Toctou%20Attacks%20Against%20Secure%20Boot%20-%20Trammell%20Hudson%20&%20Peter%20Bosch.pdf
My understanding is that this vulnerability (and fix) do not apply to
virtualization, or even most other emulation platforms (such as
EmulatorPkg).
Therefore I'm requesting that the approach seen in this patch series be
reversed, as follows.
* The patch v3 #1 should introduce the new PCD called
"PcdMigrateTemporaryRamFirmwareVolumes". (Currently: patch v2 #7.)
The comments in the DEC file (and in the UNI file) should *very* clearly
explain what the PCD controls. It should provide a *concise* description
of the entire feature. The default value of the PCD should be TRUE.
* The next patches in the series (v3 patches #2, #3, #4) should set the
PCD to FALSE in at least the following platform DSC files:
- ArmVirtPkg [v3 #2]
- EmulatorPkg [v3 #3],
- OvmfPkg [v3 #4].
The commit messages on these patches should explain that the
vulnerability simply doesn't exist on those platforms, as BootGuard is
undefined on them in the first place. "CAR" (Cache-As-RAM) is also
undefined on them.
In particular, slide #8 in the presentation, titled "Chain of Trust
(simplified)", presents two "cascades" (sub-chains of trust). My
understanding is that the attack targets the left hand side cascade
("Fused OEM key", "Signed by Intel"). And that cascade doesn't seem to
exist at all on virtualized platforms.
Furthermore, the abstract at [1] writes,
"These protections are supposed to be secure against physical attacks on
the SPI flash"
foreshadowing that the attack is indeed a physical machine attack.
Doesn't apply to virtualization.
Therefore both the attack and the mitigation appear moot, on virtual
platforms. This should be stated in the commit messages of v3 patches
#2, #3, #4.
* Patch v3 #5 should be the current (v2) patch #1, namely:
MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore
This patch should *immediately* depend on
"PcdMigrateTemporaryRamFirmwareVolumes". That is, please let's not
introduce the feature first as unconditionally active, and then gate it
on "PcdMigrateTemporaryRamFirmwareVolumes" separately. The feature
should honor "PcdMigrateTemporaryRamFirmwareVolumes" right off the bat.
Technically, this more or less means squashing part of patch v2 #7 into
v2 #1.
The commit message should point out that EvacuateTempRam() is never
called if "PcdMigrateTemporaryRamFirmwareVolumes" is FALSE.
* I've made the rest of my comments under the individual patches.
Thanks,
Laszlo
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Debkumar De <debkumar.de@intel.com>
> Cc: Harry Han <harry.han@intel.com>
> Cc: Catharine West <catharine.west@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Qi Zhang <qi1.zhang@intel.com>
>
> Guomin Jiang (5):
> MdeModulePkg/Core: Create Migrated FV Info Hob for calculating hash
> (CVE-2019-11098)
> SecurityPkg/Tcg2Pei: Use Migrated FV Info Hob for calculating hash
> (CVE-2019-11098)
> MdeModulePkg/Core: Add switch to enable or disable TOCTOU feature
> (CVE-2019-11098)
> UefiCpuPkg/SecMigrationPei: Add switch to control if produce PPI
> (CVE-2019-11098)
> UefiCpuPkg/CpuMpPei: Enable paging and set NP flag to avoid TOCTOU
> (CVE-2019-11098)
>
> Jian J Wang (1):
> MdeModulePkg/DxeIplPeim: Register for shadow on S3 shadowed boot
> (CVE-2019-11098)
>
> Michael Kubacki (3):
> MdeModulePkg/PeiCore: Enable T-RAM evacuation in PeiCore
> (CVE-2019-11098)
> UefiCpuPkg/CpuMpPei: Add GDT and IDT migration support
> (CVE-2019-11098)
> UefiCpuPkg/SecMigrationPei: Add initial PEIM (CVE-2019-11098)
>
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 3 +
> MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 2 +-
> MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 417 ++++++++++++++++++
> MdeModulePkg/Core/Pei/Image/Image.c | 115 +++++
> MdeModulePkg/Core/Pei/Memory/MemoryServices.c | 82 ++++
> MdeModulePkg/Core/Pei/PeiMain.h | 169 +++++++
> MdeModulePkg/Core/Pei/PeiMain.inf | 3 +
> MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 17 +
> MdeModulePkg/Core/Pei/Ppi/Ppi.c | 287 ++++++++++++
> MdeModulePkg/Include/Guid/MigratedFvInfo.h | 22 +
> MdeModulePkg/MdeModulePkg.dec | 8 +
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 31 +-
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 +
> UefiCpuPkg/CpuMpPei/CpuMpPei.c | 40 +-
> UefiCpuPkg/CpuMpPei/CpuMpPei.h | 13 +
> UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 3 +
> UefiCpuPkg/CpuMpPei/CpuPaging.c | 31 +-
> UefiCpuPkg/Include/Ppi/RepublishSecPpi.h | 54 +++
> .../Ia32/ArchExceptionHandler.c | 4 +-
> .../SecPeiCpuException.c | 2 +-
> UefiCpuPkg/SecCore/SecCore.inf | 2 +
> UefiCpuPkg/SecCore/SecMain.c | 26 +-
> UefiCpuPkg/SecCore/SecMain.h | 1 +
> UefiCpuPkg/SecMigrationPei/SecMigrationPei.c | 374 ++++++++++++++++
> UefiCpuPkg/SecMigrationPei/SecMigrationPei.h | 170 +++++++
> .../SecMigrationPei/SecMigrationPei.inf | 68 +++
> .../SecMigrationPei/SecMigrationPei.uni | 13 +
> UefiCpuPkg/UefiCpuPkg.dec | 4 +
> UefiCpuPkg/UefiCpuPkg.dsc | 1 +
> 29 files changed, 1947 insertions(+), 16 deletions(-)
> create mode 100644 MdeModulePkg/Include/Guid/MigratedFvInfo.h
> create mode 100644 UefiCpuPkg/Include/Ppi/RepublishSecPpi.h
> create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.c
> create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.h
> create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.inf
> create mode 100644 UefiCpuPkg/SecMigrationPei/SecMigrationPei.uni
>
^ permalink raw reply [flat|nested] 25+ messages in thread