REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317
This patch removes EFI_LOADED_IMAGE_PROTOCOL from DXE database. The SmmCore only install EFI_LOADED_IMAGE_PROTOCOL to SMM database. Exposing LOADED_IMAGE_PROTOCOL may cause SMM information leakage to non-SMM component. Cc: Feng Tian <feng.t...@intel.com> Cc: Star Zeng <star.z...@intel.com> Cc: Michael D Kinney <michael.d.kin...@intel.com> Cc: Laszlo Ersek <ler...@redhat.com> Signed-off-by: Jiewen Yao <jiewen....@intel.com> Signed-off-by: Zhiguang Liu <zhiguang....@intel.com> --- MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 104 +++++++++++--------------------------------------------------------------------------------------------- MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 34 ---------------------------------- MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 4 ++-- 3 files changed, 13 insertions(+), 129 deletions(-) diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c index 76ee9e0b89..2be2866234 100644 --- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c +++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c @@ -316,7 +316,7 @@ SmmLoadImage ( EFI_FIRMWARE_VOLUME2_PROTOCOL *Fv; PE_COFF_LOADER_IMAGE_CONTEXT ImageContext; - PERF_LOAD_IMAGE_BEGIN (DriverEntry->ImageHandle); + PERF_LOAD_IMAGE_BEGIN (DriverEntry->SmmImageHandle); Buffer = NULL; Size = 0; @@ -546,51 +546,14 @@ SmmLoadImage ( DriverEntry->ImageBuffer = DstBuffer; DriverEntry->NumberOfPage = PageCount; - // - // Allocate a Loaded Image Protocol in EfiBootServicesData - // - Status = gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_LOADED_IMAGE_PROTOCOL), (VOID **)&DriverEntry->LoadedImage); - if (EFI_ERROR (Status)) { - if (Buffer != NULL) { - gBS->FreePool (Buffer); - } - SmmFreePages (DstBuffer, PageCount); - return Status; - } - - ZeroMem (DriverEntry->LoadedImage, sizeof (EFI_LOADED_IMAGE_PROTOCOL)); // // Fill in the remaining fields of the Loaded Image Protocol instance. - // Note: ImageBase is an SMRAM address that can not be accessed outside of SMRAM if SMRAM window is closed. // - DriverEntry->LoadedImage->Revision = EFI_LOADED_IMAGE_PROTOCOL_REVISION; - DriverEntry->LoadedImage->ParentHandle = gSmmCorePrivate->SmmIplImageHandle; - DriverEntry->LoadedImage->SystemTable = gST; - DriverEntry->LoadedImage->DeviceHandle = DeviceHandle; - DriverEntry->SmmLoadedImage.Revision = EFI_LOADED_IMAGE_PROTOCOL_REVISION; DriverEntry->SmmLoadedImage.ParentHandle = gSmmCorePrivate->SmmIplImageHandle; DriverEntry->SmmLoadedImage.SystemTable = gST; DriverEntry->SmmLoadedImage.DeviceHandle = DeviceHandle; - // - // Make an EfiBootServicesData buffer copy of FilePath - // - Status = gBS->AllocatePool (EfiBootServicesData, GetDevicePathSize (FilePath), (VOID **)&DriverEntry->LoadedImage->FilePath); - if (EFI_ERROR (Status)) { - if (Buffer != NULL) { - gBS->FreePool (Buffer); - } - SmmFreePages (DstBuffer, PageCount); - return Status; - } - CopyMem (DriverEntry->LoadedImage->FilePath, FilePath, GetDevicePathSize (FilePath)); - - DriverEntry->LoadedImage->ImageBase = (VOID *)(UINTN) ImageContext.ImageAddress; - DriverEntry->LoadedImage->ImageSize = ImageContext.ImageSize; - DriverEntry->LoadedImage->ImageCodeType = EfiRuntimeServicesCode; - DriverEntry->LoadedImage->ImageDataType = EfiRuntimeServicesData; - // // Make a buffer copy of FilePath // @@ -599,7 +562,6 @@ SmmLoadImage ( if (Buffer != NULL) { gBS->FreePool (Buffer); } - gBS->FreePool (DriverEntry->LoadedImage->FilePath); SmmFreePages (DstBuffer, PageCount); return Status; } @@ -610,16 +572,6 @@ SmmLoadImage ( DriverEntry->SmmLoadedImage.ImageCodeType = EfiRuntimeServicesCode; DriverEntry->SmmLoadedImage.ImageDataType = EfiRuntimeServicesData; - // - // Create a new image handle in the UEFI handle database for the SMM Driver - // - DriverEntry->ImageHandle = NULL; - Status = gBS->InstallMultipleProtocolInterfaces ( - &DriverEntry->ImageHandle, - &gEfiLoadedImageProtocolGuid, DriverEntry->LoadedImage, - NULL - ); - // // Create a new image handle in the SMM handle database for the SMM Driver // @@ -631,7 +583,7 @@ SmmLoadImage ( &DriverEntry->SmmLoadedImage ); - PERF_LOAD_IMAGE_END (DriverEntry->ImageHandle); + PERF_LOAD_IMAGE_END (DriverEntry->SmmImageHandle); // // Print the load address and the PDB file name if it is available @@ -856,7 +808,7 @@ SmmDispatcher ( // Untrused to Scheduled it would have already been loaded so we may need to // skip the LoadImage // - if (DriverEntry->ImageHandle == NULL) { + if (DriverEntry->SmmImageHandle == NULL) { Status = SmmLoadImage (DriverEntry); // @@ -885,8 +837,8 @@ SmmDispatcher ( REPORT_STATUS_CODE_WITH_EXTENDED_DATA ( EFI_PROGRESS_CODE, EFI_SOFTWARE_SMM_DRIVER | EFI_SW_PC_INIT_BEGIN, - &DriverEntry->ImageHandle, - sizeof (DriverEntry->ImageHandle) + &DriverEntry->SmmImageHandle, + sizeof (DriverEntry->SmmImageHandle) ); // @@ -898,9 +850,10 @@ SmmDispatcher ( // For each SMM driver, pass NULL as ImageHandle // RegisterSmramProfileImage (DriverEntry, TRUE); - PERF_START_IMAGE_BEGIN (DriverEntry->ImageHandle); - Status = ((EFI_IMAGE_ENTRY_POINT)(UINTN)DriverEntry->ImageEntryPoint)(DriverEntry->ImageHandle, gST); - PERF_START_IMAGE_END (DriverEntry->ImageHandle); + DEBUG ((DEBUG_INFO, "SMM StartImage - 0x%11p\n", DriverEntry->ImageEntryPoint)); + PERF_START_IMAGE_BEGIN (DriverEntry->SmmImageHandle); + Status = ((EFI_IMAGE_ENTRY_POINT)(UINTN)DriverEntry->ImageEntryPoint)(DriverEntry->SmmImageHandle, gST); + PERF_START_IMAGE_END (DriverEntry->SmmImageHandle); if (EFI_ERROR(Status)){ DEBUG (( DEBUG_ERROR, @@ -910,20 +863,6 @@ SmmDispatcher ( )); UnregisterSmramProfileImage (DriverEntry, TRUE); SmmFreePages(DriverEntry->ImageBuffer, DriverEntry->NumberOfPage); - // - // Uninstall LoadedImage - // - Status = gBS->UninstallProtocolInterface ( - DriverEntry->ImageHandle, - &gEfiLoadedImageProtocolGuid, - DriverEntry->LoadedImage - ); - if (!EFI_ERROR (Status)) { - if (DriverEntry->LoadedImage->FilePath != NULL) { - gBS->FreePool (DriverEntry->LoadedImage->FilePath); - } - gBS->FreePool (DriverEntry->LoadedImage); - } Status = SmmUninstallProtocolInterface ( DriverEntry->SmmImageHandle, &gEfiLoadedImageProtocolGuid, @@ -939,8 +878,8 @@ SmmDispatcher ( REPORT_STATUS_CODE_WITH_EXTENDED_DATA ( EFI_PROGRESS_CODE, EFI_SOFTWARE_SMM_DRIVER | EFI_SW_PC_INIT_END, - &DriverEntry->ImageHandle, - sizeof (DriverEntry->ImageHandle) + &DriverEntry->SmmImageHandle, + sizeof (DriverEntry->SmmImageHandle) ); if (!PreviousSmmEntryPointRegistered && gSmmCorePrivate->SmmEntryPointRegistered) { @@ -1344,27 +1283,6 @@ SmmDriverDispatchHandler ( // // If this is the SMM core fill in it's DevicePath & DeviceHandle // - if (mSmmCoreLoadedImage->FilePath == NULL) { - // - // Maybe one special FV contains only one SMM_CORE module, so its device path must - // be initialized completely. - // - EfiInitializeFwVolDevicepathNode (&mFvDevicePath.File, &NameGuid); - SetDevicePathEndNode (&mFvDevicePath.End); - - // - // Make an EfiBootServicesData buffer copy of FilePath - // - Status = gBS->AllocatePool ( - EfiBootServicesData, - GetDevicePathSize ((EFI_DEVICE_PATH_PROTOCOL *)&mFvDevicePath), - (VOID **)&mSmmCoreLoadedImage->FilePath - ); - ASSERT_EFI_ERROR (Status); - CopyMem (mSmmCoreLoadedImage->FilePath, &mFvDevicePath, GetDevicePathSize ((EFI_DEVICE_PATH_PROTOCOL *)&mFvDevicePath)); - - mSmmCoreLoadedImage->DeviceHandle = FvHandle; - } if (mSmmCoreDriverEntry->SmmLoadedImage.FilePath == NULL) { // // Maybe one special FV contains only one SMM_CORE module, so its device path must diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c index cfa9922cbd..c1b18b047e 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c @@ -104,8 +104,6 @@ EFI_SMRAM_DESCRIPTOR *mFullSmramRanges; EFI_SMM_DRIVER_ENTRY *mSmmCoreDriverEntry; -EFI_LOADED_IMAGE_PROTOCOL *mSmmCoreLoadedImage; - /** Place holder function until all the SMM System Table Service are available. @@ -756,38 +754,6 @@ SmmCoreInstallLoadedImage ( ) { EFI_STATUS Status; - EFI_HANDLE Handle; - - // - // Allocate a Loaded Image Protocol in EfiBootServicesData - // - Status = gBS->AllocatePool (EfiBootServicesData, sizeof(EFI_LOADED_IMAGE_PROTOCOL), (VOID **)&mSmmCoreLoadedImage); - ASSERT_EFI_ERROR (Status); - - ZeroMem (mSmmCoreLoadedImage, sizeof (EFI_LOADED_IMAGE_PROTOCOL)); - // - // Fill in the remaining fields of the Loaded Image Protocol instance. - // Note: ImageBase is an SMRAM address that can not be accessed outside of SMRAM if SMRAM window is closed. - // - mSmmCoreLoadedImage->Revision = EFI_LOADED_IMAGE_PROTOCOL_REVISION; - mSmmCoreLoadedImage->ParentHandle = gSmmCorePrivate->SmmIplImageHandle; - mSmmCoreLoadedImage->SystemTable = gST; - - mSmmCoreLoadedImage->ImageBase = (VOID *)(UINTN)gSmmCorePrivate->PiSmmCoreImageBase; - mSmmCoreLoadedImage->ImageSize = gSmmCorePrivate->PiSmmCoreImageSize; - mSmmCoreLoadedImage->ImageCodeType = EfiRuntimeServicesCode; - mSmmCoreLoadedImage->ImageDataType = EfiRuntimeServicesData; - - // - // Create a new image handle in the UEFI handle database for the SMM Driver - // - Handle = NULL; - Status = gBS->InstallMultipleProtocolInterfaces ( - &Handle, - &gEfiLoadedImageProtocolGuid, mSmmCoreLoadedImage, - NULL - ); - ASSERT_EFI_ERROR (Status); // // Allocate a Loaded Image Protocol in SMM diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h index 50a7fc0000..3bbd1e1603 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h @@ -122,8 +122,8 @@ typedef struct { BOOLEAN Initialized; BOOLEAN DepexProtocolError; - EFI_HANDLE ImageHandle; - EFI_LOADED_IMAGE_PROTOCOL *LoadedImage; + EFI_HANDLE ImageHandle_Reserved1; + EFI_LOADED_IMAGE_PROTOCOL *LoadedImage_Reserved2; // // Image EntryPoint in SMRAM // -- 2.25.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61325): https://edk2.groups.io/g/devel/message/61325 Mute This Topic: https://groups.io/mt/74912558/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-