From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com []) by mx.groups.io with SMTP id smtpd.web10.6584.1592298291128789206 for ; Tue, 16 Jun 2020 02:04:51 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=fail (domain: intel.com, ip: , mailfrom: zhiguang.liu@intel.com) IronPort-SDR: fSxN9VZMrvTdN3tyKmA1u5SS9kST1bKMNgdc6aNza89khy5M4codMLXZ+WEghPRkkALv88TfE/ nGGJPaZxsGCA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Jun 2020 02:04:44 -0700 IronPort-SDR: UOpNwtO0bcLunDRllvAjz0ewVgjVUQ6yJRHrfQDGAHHeXCW/vdEI2qU0+uKKQ28G4UrPmnvVwT 9ZWKi+XMsI3Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,518,1583222400"; d="scan'208";a="290992313" Received: from fieedk002.ccr.corp.intel.com ([10.239.158.178]) by orsmga002.jf.intel.com with ESMTP; 16 Jun 2020 02:04:40 -0700 From: "Zhiguang Liu" To: devel@edk2.groups.io Cc: Feng Tian , Star Zeng , Michael D Kinney , Laszlo Ersek , Jiewen Yao Subject: [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI LOADED_IMAGE from SMM. Date: Tue, 16 Jun 2020 17:04:31 +0800 Message-Id: <20200616090434.1201-2-zhiguang.liu@intel.com> X-Mailer: git-send-email 2.25.1.windows.1 In-Reply-To: <20200616090434.1201-1-zhiguang.liu@intel.com> References: <20200616090434.1201-1-zhiguang.liu@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2317 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 Cc: Star Zeng Cc: Michael D Kinney Cc: Laszlo Ersek Signed-off-by: Jiewen Yao Signed-off-by: Zhiguang Liu --- 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/P= iSmmCore/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;=0D PE_COFF_LOADER_IMAGE_CONTEXT ImageContext;=0D =0D - PERF_LOAD_IMAGE_BEGIN (DriverEntry->ImageHandle);=0D + PERF_LOAD_IMAGE_BEGIN (DriverEntry->SmmImageHandle);=0D =0D Buffer =3D NULL;=0D Size =3D 0;=0D @@ -546,51 +546,14 @@ SmmLoadImage ( DriverEntry->ImageBuffer =3D DstBuffer;=0D DriverEntry->NumberOfPage =3D PageCount;=0D =0D - //=0D - // Allocate a Loaded Image Protocol in EfiBootServicesData=0D - //=0D - Status =3D gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_LOADED_IM= AGE_PROTOCOL), (VOID **)&DriverEntry->LoadedImage);=0D - if (EFI_ERROR (Status)) {=0D - if (Buffer !=3D NULL) {=0D - gBS->FreePool (Buffer);=0D - }=0D - SmmFreePages (DstBuffer, PageCount);=0D - return Status;=0D - }=0D -=0D - ZeroMem (DriverEntry->LoadedImage, sizeof (EFI_LOADED_IMAGE_PROTOCOL));= =0D //=0D // Fill in the remaining fields of the Loaded Image Protocol instance.=0D - // Note: ImageBase is an SMRAM address that can not be accessed outside = of SMRAM if SMRAM window is closed.=0D //=0D - DriverEntry->LoadedImage->Revision =3D EFI_LOADED_IMAGE_PROTOCOL_RE= VISION;=0D - DriverEntry->LoadedImage->ParentHandle =3D gSmmCorePrivate->SmmIplImage= Handle;=0D - DriverEntry->LoadedImage->SystemTable =3D gST;=0D - DriverEntry->LoadedImage->DeviceHandle =3D DeviceHandle;=0D -=0D DriverEntry->SmmLoadedImage.Revision =3D EFI_LOADED_IMAGE_PROTOCOL_R= EVISION;=0D DriverEntry->SmmLoadedImage.ParentHandle =3D gSmmCorePrivate->SmmIplImag= eHandle;=0D DriverEntry->SmmLoadedImage.SystemTable =3D gST;=0D DriverEntry->SmmLoadedImage.DeviceHandle =3D DeviceHandle;=0D =0D - //=0D - // Make an EfiBootServicesData buffer copy of FilePath=0D - //=0D - Status =3D gBS->AllocatePool (EfiBootServicesData, GetDevicePathSize (Fi= lePath), (VOID **)&DriverEntry->LoadedImage->FilePath);=0D - if (EFI_ERROR (Status)) {=0D - if (Buffer !=3D NULL) {=0D - gBS->FreePool (Buffer);=0D - }=0D - SmmFreePages (DstBuffer, PageCount);=0D - return Status;=0D - }=0D - CopyMem (DriverEntry->LoadedImage->FilePath, FilePath, GetDevicePathSize= (FilePath));=0D -=0D - DriverEntry->LoadedImage->ImageBase =3D (VOID *)(UINTN) ImageContext= .ImageAddress;=0D - DriverEntry->LoadedImage->ImageSize =3D ImageContext.ImageSize;=0D - DriverEntry->LoadedImage->ImageCodeType =3D EfiRuntimeServicesCode;=0D - DriverEntry->LoadedImage->ImageDataType =3D EfiRuntimeServicesData;=0D -=0D //=0D // Make a buffer copy of FilePath=0D //=0D @@ -599,7 +562,6 @@ SmmLoadImage ( if (Buffer !=3D NULL) {=0D gBS->FreePool (Buffer);=0D }=0D - gBS->FreePool (DriverEntry->LoadedImage->FilePath);=0D SmmFreePages (DstBuffer, PageCount);=0D return Status;=0D }=0D @@ -610,16 +572,6 @@ SmmLoadImage ( DriverEntry->SmmLoadedImage.ImageCodeType =3D EfiRuntimeServicesCode;=0D DriverEntry->SmmLoadedImage.ImageDataType =3D EfiRuntimeServicesData;=0D =0D - //=0D - // Create a new image handle in the UEFI handle database for the SMM Dri= ver=0D - //=0D - DriverEntry->ImageHandle =3D NULL;=0D - Status =3D gBS->InstallMultipleProtocolInterfaces (=0D - &DriverEntry->ImageHandle,=0D - &gEfiLoadedImageProtocolGuid, DriverEntry->LoadedImage,= =0D - NULL=0D - );=0D -=0D //=0D // Create a new image handle in the SMM handle database for the SMM Driv= er=0D //=0D @@ -631,7 +583,7 @@ SmmLoadImage ( &DriverEntry->SmmLoadedImage=0D );=0D =0D - PERF_LOAD_IMAGE_END (DriverEntry->ImageHandle);=0D + PERF_LOAD_IMAGE_END (DriverEntry->SmmImageHandle);=0D =0D //=0D // Print the load address and the PDB file name if it is available=0D @@ -856,7 +808,7 @@ SmmDispatcher ( // Untrused to Scheduled it would have already been loaded so we may= need to=0D // skip the LoadImage=0D //=0D - if (DriverEntry->ImageHandle =3D=3D NULL) {=0D + if (DriverEntry->SmmImageHandle =3D=3D NULL) {=0D Status =3D SmmLoadImage (DriverEntry);=0D =0D //=0D @@ -885,8 +837,8 @@ SmmDispatcher ( REPORT_STATUS_CODE_WITH_EXTENDED_DATA (=0D EFI_PROGRESS_CODE,=0D EFI_SOFTWARE_SMM_DRIVER | EFI_SW_PC_INIT_BEGIN,=0D - &DriverEntry->ImageHandle,=0D - sizeof (DriverEntry->ImageHandle)=0D + &DriverEntry->SmmImageHandle,=0D + sizeof (DriverEntry->SmmImageHandle)=0D );=0D =0D //=0D @@ -898,9 +850,10 @@ SmmDispatcher ( // For each SMM driver, pass NULL as ImageHandle=0D //=0D RegisterSmramProfileImage (DriverEntry, TRUE);=0D - PERF_START_IMAGE_BEGIN (DriverEntry->ImageHandle);=0D - Status =3D ((EFI_IMAGE_ENTRY_POINT)(UINTN)DriverEntry->ImageEntryPoi= nt)(DriverEntry->ImageHandle, gST);=0D - PERF_START_IMAGE_END (DriverEntry->ImageHandle);=0D + DEBUG ((DEBUG_INFO, "SMM StartImage - 0x%11p\n", DriverEntry->ImageE= ntryPoint));=0D + PERF_START_IMAGE_BEGIN (DriverEntry->SmmImageHandle);=0D + Status =3D ((EFI_IMAGE_ENTRY_POINT)(UINTN)DriverEntry->ImageEntryPoi= nt)(DriverEntry->SmmImageHandle, gST);=0D + PERF_START_IMAGE_END (DriverEntry->SmmImageHandle);=0D if (EFI_ERROR(Status)){=0D DEBUG ((=0D DEBUG_ERROR,=0D @@ -910,20 +863,6 @@ SmmDispatcher ( ));=0D UnregisterSmramProfileImage (DriverEntry, TRUE);=0D SmmFreePages(DriverEntry->ImageBuffer, DriverEntry->NumberOfPage);= =0D - //=0D - // Uninstall LoadedImage=0D - //=0D - Status =3D gBS->UninstallProtocolInterface (=0D - DriverEntry->ImageHandle,=0D - &gEfiLoadedImageProtocolGuid,=0D - DriverEntry->LoadedImage=0D - );=0D - if (!EFI_ERROR (Status)) {=0D - if (DriverEntry->LoadedImage->FilePath !=3D NULL) {=0D - gBS->FreePool (DriverEntry->LoadedImage->FilePath);=0D - }=0D - gBS->FreePool (DriverEntry->LoadedImage);=0D - }=0D Status =3D SmmUninstallProtocolInterface (=0D DriverEntry->SmmImageHandle,=0D &gEfiLoadedImageProtocolGuid,=0D @@ -939,8 +878,8 @@ SmmDispatcher ( REPORT_STATUS_CODE_WITH_EXTENDED_DATA (=0D EFI_PROGRESS_CODE,=0D EFI_SOFTWARE_SMM_DRIVER | EFI_SW_PC_INIT_END,=0D - &DriverEntry->ImageHandle,=0D - sizeof (DriverEntry->ImageHandle)=0D + &DriverEntry->SmmImageHandle,=0D + sizeof (DriverEntry->SmmImageHandle)=0D );=0D =0D if (!PreviousSmmEntryPointRegistered && gSmmCorePrivate->SmmEntryPoi= ntRegistered) {=0D @@ -1344,27 +1283,6 @@ SmmDriverDispatchHandler ( //=0D // If this is the SMM core fill in it's DevicePath & DeviceHan= dle=0D //=0D - if (mSmmCoreLoadedImage->FilePath =3D=3D NULL) {=0D - //=0D - // Maybe one special FV contains only one SMM_CORE module, s= o its device path must=0D - // be initialized completely.=0D - //=0D - EfiInitializeFwVolDevicepathNode (&mFvDevicePath.File, &Name= Guid);=0D - SetDevicePathEndNode (&mFvDevicePath.End);=0D -=0D - //=0D - // Make an EfiBootServicesData buffer copy of FilePath=0D - //=0D - Status =3D gBS->AllocatePool (=0D - EfiBootServicesData,=0D - GetDevicePathSize ((EFI_DEVICE_PATH_PROTOCOL= *)&mFvDevicePath),=0D - (VOID **)&mSmmCoreLoadedImage->FilePath=0D - );=0D - ASSERT_EFI_ERROR (Status);=0D - CopyMem (mSmmCoreLoadedImage->FilePath, &mFvDevicePath, GetD= evicePathSize ((EFI_DEVICE_PATH_PROTOCOL *)&mFvDevicePath));=0D -=0D - mSmmCoreLoadedImage->DeviceHandle =3D FvHandle;=0D - }=0D if (mSmmCoreDriverEntry->SmmLoadedImage.FilePath =3D=3D NULL) = {=0D //=0D // Maybe one special FV contains only one SMM_CORE module, s= o its device path must=0D diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/Pi= SmmCore/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; =0D EFI_SMM_DRIVER_ENTRY *mSmmCoreDriverEntry;=0D =0D -EFI_LOADED_IMAGE_PROTOCOL *mSmmCoreLoadedImage;=0D -=0D /**=0D Place holder function until all the SMM System Table Service are availab= le.=0D =0D @@ -756,38 +754,6 @@ SmmCoreInstallLoadedImage ( )=0D {=0D EFI_STATUS Status;=0D - EFI_HANDLE Handle;=0D -=0D - //=0D - // Allocate a Loaded Image Protocol in EfiBootServicesData=0D - //=0D - Status =3D gBS->AllocatePool (EfiBootServicesData, sizeof(EFI_LOADED_IMA= GE_PROTOCOL), (VOID **)&mSmmCoreLoadedImage);=0D - ASSERT_EFI_ERROR (Status);=0D -=0D - ZeroMem (mSmmCoreLoadedImage, sizeof (EFI_LOADED_IMAGE_PROTOCOL));=0D - //=0D - // Fill in the remaining fields of the Loaded Image Protocol instance.=0D - // Note: ImageBase is an SMRAM address that can not be accessed outside = of SMRAM if SMRAM window is closed.=0D - //=0D - mSmmCoreLoadedImage->Revision =3D EFI_LOADED_IMAGE_PROTOCOL_REVISIO= N;=0D - mSmmCoreLoadedImage->ParentHandle =3D gSmmCorePrivate->SmmIplImageHandl= e;=0D - mSmmCoreLoadedImage->SystemTable =3D gST;=0D -=0D - mSmmCoreLoadedImage->ImageBase =3D (VOID *)(UINTN)gSmmCorePrivate->P= iSmmCoreImageBase;=0D - mSmmCoreLoadedImage->ImageSize =3D gSmmCorePrivate->PiSmmCoreImageSi= ze;=0D - mSmmCoreLoadedImage->ImageCodeType =3D EfiRuntimeServicesCode;=0D - mSmmCoreLoadedImage->ImageDataType =3D EfiRuntimeServicesData;=0D -=0D - //=0D - // Create a new image handle in the UEFI handle database for the SMM Dri= ver=0D - //=0D - Handle =3D NULL;=0D - Status =3D gBS->InstallMultipleProtocolInterfaces (=0D - &Handle,=0D - &gEfiLoadedImageProtocolGuid, mSmmCoreLoadedImage,=0D - NULL=0D - );=0D - ASSERT_EFI_ERROR (Status);=0D =0D //=0D // Allocate a Loaded Image Protocol in SMM=0D diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/Pi= SmmCore/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;=0D BOOLEAN DepexProtocolError;=0D =0D - EFI_HANDLE ImageHandle;=0D - EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;=0D + EFI_HANDLE ImageHandle_Reserved1;=0D + EFI_LOADED_IMAGE_PROTOCOL *LoadedImage_Reserved2;=0D //=0D // Image EntryPoint in SMRAM=0D //=0D --=20 2.25.1.windows.1