Zhiguang, The commit message should not have more than one Signed-off-by.
Use of Suggested-by may be more appropriate here. Mike > -----Original Message----- > From: Liu, Zhiguang <zhiguang....@intel.com> > Sent: Tuesday, June 16, 2020 2:05 AM > To: devel@edk2.groups.io > Cc: Tian, Feng <feng.t...@intel.com>; Zeng, Star > <star.z...@intel.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Laszlo Ersek > <ler...@redhat.com>; Yao, Jiewen <jiewen....@intel.com> > Subject: [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI > LOADED_IMAGE from SMM. > > 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 (#61366): https://edk2.groups.io/g/devel/message/61366 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] -=-=-=-=-=-=-=-=-=-=-=-
