From: "Yao, Jiewen" <jiewen.yao@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
"Tian, Feng" <feng.tian@intel.com>,
"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.
Date: Mon, 6 Feb 2017 14:51:23 +0000 [thread overview]
Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A8E9938@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <CAKv+Gu_d-XVNnix9_+NjZgtuHPufZ54ThawOu+=hkCbyE1CSfw@mail.gmail.com>
Hi Ard
That is a good question.
We (Intel) discussed this internally.
We do not know if there is any need to make it configurable, so we start from non-configurable, and see if there is any feedback from other one.
Glad to hear your voice. We can define a policy like below: (I borrow some definition from security pkg)
[PcdsFixedAtBuild, PcdsPatchableInModule]
## Set Image protection policy. The policy is bitwise.
# If a bit is set, the image will be protected by DxeCore if it is aligned.
# The code section becomes read-only, and the data section becomes non-executable.
# If a bit is clear, the image will not be protected.<BR><BR>
# BIT0 - Image from unknown device. <BR>
# BIT1 - Image from firmware volume.<BR>
# BIT2 - Image from OptionRom.<BR>
# BIT3 - Image from removable media which includes CD-ROM, Floppy, USB and network.<BR>
# BIT4 - Image from fixed media device which includes hard disk.<BR>
# @Prompt Set Image protection policy.
# @ValidRange 0x80000002 | 0x00000000 - 0x0000001F
gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x00000002|UINT32|0x30001047
I will use 0x2 as default policy, to maximize the compatibility. And you can set 0x1A or 0x1F for ARM. Is that OK?
Thank you
Yao Jiewen
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Monday, February 6, 2017 6:35 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection.
On 26 January 2017 at 08:39, Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
> If the UEFI image is page aligned, the image code section is set to read
> only and the image data section is set to non-executable.
>
> 1) This policy is applied for all UEFI image including boot service driver,
> runtime driver or application.
> 2) This policy is applied only if the UEFI image meets the page alignment
> requirement.
> 3) This policy is applied only if the UEFI image is from firmware volume.
> If the image is from OPROM, file system, or network, this policy is not
> applied, even if the image meets the page alignment requirement.
Is this policy configurable? I would like to start enforcing this on
ARM even for file system images.
> 4) This policy is not applied to the non-PE image region.
>
> The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
> the image. If the CpuArch protocol is not installed yet, the DxeCore
> enqueues the protection request. Once the CpuArch is installed, the
> DxeCore dequeues the protection request and applies policy.
>
> Once the image is unloaded, the protection is removed automatically.
>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>>
> Cc: Michael Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> ---
> MdeModulePkg/Core/Dxe/DxeMain.h | 53 ++
> MdeModulePkg/Core/Dxe/DxeMain.inf | 2 +
> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 1 +
> MdeModulePkg/Core/Dxe/Image/Image.c | 5 +
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 822 ++++++++++++++++++++
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 22 -
> 6 files changed, 883 insertions(+), 22 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
> index ae35fbb..67b5a5a 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> @@ -267,6 +267,26 @@ typedef struct {
> #define LOADED_IMAGE_PRIVATE_DATA_FROM_THIS(a) \
> CR(a, LOADED_IMAGE_PRIVATE_DATA, Info, LOADED_IMAGE_PRIVATE_DATA_SIGNATURE)
>
> +#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE SIGNATURE_32 ('I','P','R','C')
> +
> +typedef struct {
> + UINT32 Signature;
> + LIST_ENTRY Link;
> + EFI_PHYSICAL_ADDRESS CodeSegmentBase;
> + UINT64 CodeSegmentSize;
> +} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
> +
> +#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32 ('I','P','R','D')
> +
> +typedef struct {
> + UINT32 Signature;
> + LIST_ENTRY Link;
> + EFI_PHYSICAL_ADDRESS ImageBase;
> + UINT64 ImageSize;
> + UINTN CodeSegmentCount;
> + LIST_ENTRY CodeSegmentList;
> +} IMAGE_PROPERTIES_RECORD;
> +
> //
> // DXE Core Global Variables
> //
> @@ -2859,6 +2879,15 @@ CoreInitializeMemoryAttributesTable (
> );
>
> /**
> + Initialize Memory Protection support.
> +**/
> +VOID
> +EFIAPI
> +CoreInitializeMemoryProtection (
> + VOID
> + );
> +
> +/**
> Install MemoryAttributesTable on memory allocation.
>
> @param[in] MemoryType EFI memory type.
> @@ -2888,4 +2917,28 @@ RemoveImageRecord (
> IN EFI_RUNTIME_IMAGE_ENTRY *RuntimeImage
> );
>
> +/**
> + Protect UEFI image.
> +
> + @param[in] LoadedImage The loaded image protocol
> + @param[in] LoadedImageDevicePath The loaded image device path protocol
> +**/
> +VOID
> +ProtectUefiImage (
> + IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
> + IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
> + );
> +
> +/**
> + Unprotect UEFI image.
> +
> + @param[in] LoadedImage The loaded image protocol
> + @param[in] LoadedImageDevicePath The loaded image device path protocol
> +**/
> +VOID
> +UnprotectUefiImage (
> + IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
> + IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
> + );
> +
> #endif
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 13a2381..e62bc42 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -42,6 +42,7 @@
> Misc/InstallConfigurationTable.c
> Misc/PropertiesTable.c
> Misc/MemoryAttributesTable.c
> + Misc/MemoryProtection.c
> Library/Library.c
> Hand/DriverSupport.c
> Hand/Notify.c
> @@ -159,6 +160,7 @@
> gEfiHiiPackageListProtocolGuid ## SOMETIMES_PRODUCES
> gEfiEbcProtocolGuid ## SOMETIMES_CONSUMES
> gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES
>
> # Arch Protocols
> gEfiBdsArchProtocolGuid ## CONSUMES
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> index 21cd61a..7304f69 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
> @@ -398,6 +398,7 @@ DxeMain (
>
> CoreInitializePropertiesTable ();
> CoreInitializeMemoryAttributesTable ();
> + CoreInitializeMemoryProtection ();
>
> //
> // Get persisted vector hand-off info from GUIDeed HOB again due to HobStart may be updated,
> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
> index 4a8a16d..1c930ef 100644
> --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> @@ -203,6 +203,8 @@ CoreInitializeImageServices (
> );
> }
>
> + ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
> +
> return Status;
> }
>
> @@ -862,6 +864,8 @@ CoreUnloadAndCloseImage (
> UnregisterMemoryProfileImage (Image);
> }
>
> + UnprotectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
> +
> if (Image->Ebc != NULL) {
> //
> // If EBC protocol exists we must perform cleanups for this image.
> @@ -1341,6 +1345,7 @@ CoreLoadImageCommon (
> goto Done;
> }
> }
> + ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
>
> //
> // Success. Return the image handle
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> new file mode 100644
> index 0000000..fe412a0
> --- /dev/null
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -0,0 +1,822 @@
> +/** @file
> + UEFI Memory Protection support.
> +
> + If the UEFI image is page aligned, the image code section is set to read only
> + and the image data section is set to non-executable.
> +
> + 1) This policy is applied for all UEFI image including boot service driver,
> + runtime driver or application.
> + 2) This policy is applied only if the UEFI image meets the page alignment
> + requirement.
> + 3) This policy is applied only if the UEFI image is from firmware volume.
> + If the image is from OPROM, file system, or network, this policy is not
> + applied, even if the image meets the page alignment requirement.
> + 4) This policy is not applied to the non-PE image region.
> +
> + The DxeCore calls CpuArchProtocol->SetMemoryAttributes() to protect
> + the image. If the CpuArch protocol is not installed yet, the DxeCore
> + enqueues the protection request. Once the CpuArch is installed, the
> + DxeCore dequeues the protection request and applies policy.
> +
> + Once the image is unloaded, the protection is removed automatically.
> +
> +Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution. The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <PiDxe.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include <Guid/EventGroup.h>
> +#include <Guid/MemoryAttributesTable.h>
> +#include <Guid/PropertiesTable.h>
> +
> +#include <Protocol/FirmwareVolume2.h>
> +#include <Protocol/BlockIo.h>
> +#include <Protocol/SimpleFileSystem.h>
> +
> +#include "DxeMain.h"
> +
> +#define CACHE_ATTRIBUTE_MASK (EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_WP)
> +#define MEMORY_ATTRIBUTE_MASK (EFI_MEMORY_RP | EFI_MEMORY_XP | EFI_MEMORY_RO)
> +
> +//
> +// Image type definitions
> +//
> +#define IMAGE_UNKNOWN 0x00000001
> +#define IMAGE_FROM_FV 0x00000002
> +#define IMAGE_FROM_OPTION_ROM 0x00000004
> +#define IMAGE_FROM_REMOVABLE_MEDIA 0x00000008
> +#define IMAGE_FROM_FIXED_MEDIA 0x00000010
> +
> +//
> +// Protection policy bit definition
> +//
> +#define DO_NOT_PROTECT 0x00000000
> +#define PROTECT_IF_ALIGNED_ELSE_ALLOW 0x00000001
> +
> +UINT32 mDefaultProtectionPolicy[] = {
> + DO_NOT_PROTECT, // IMAGE_UNKNOWN
> + PROTECT_IF_ALIGNED_ELSE_ALLOW, // IMAGE_FROM_FV
> + DO_NOT_PROTECT, // IMAGE_FROM_OPTION_ROM
> + DO_NOT_PROTECT, // IMAGE_FROM_REMOVABLE_MEDIA
> + DO_NOT_PROTECT, // IMAGE_FROM_FIXED_MEDIA
> +};
> +
> +/**
> + Sort code section in image record, based upon CodeSegmentBase from low to high.
> +
> + @param ImageRecord image record to be sorted
> +**/
> +VOID
> +SortImageRecordCodeSection (
> + IN IMAGE_PROPERTIES_RECORD *ImageRecord
> + );
> +
> +/**
> + Check if code section in image record is valid.
> +
> + @param ImageRecord image record to be checked
> +
> + @retval TRUE image record is valid
> + @retval FALSE image record is invalid
> +**/
> +BOOLEAN
> +IsImageRecordCodeSectionValid (
> + IN IMAGE_PROPERTIES_RECORD *ImageRecord
> + );
> +
> +/**
> + Get the image type.
> +
> + @param[in] File This is a pointer to the device path of the file that is
> + being dispatched.
> +
> + @return UINT32 Image Type
> +**/
> +UINT32
> +GetImageType (
> + IN CONST EFI_DEVICE_PATH_PROTOCOL *File
> + )
> +{
> + EFI_STATUS Status;
> + EFI_HANDLE DeviceHandle;
> + EFI_DEVICE_PATH_PROTOCOL *TempDevicePath;
> + EFI_BLOCK_IO_PROTOCOL *BlockIo;
> +
> + if (File == NULL) {
> + return IMAGE_UNKNOWN;
> + }
> +
> + //
> + // First check to see if File is from a Firmware Volume
> + //
> + DeviceHandle = NULL;
> + TempDevicePath = (EFI_DEVICE_PATH_PROTOCOL *) File;
> + Status = gBS->LocateDevicePath (
> + &gEfiFirmwareVolume2ProtocolGuid,
> + &TempDevicePath,
> + &DeviceHandle
> + );
> + if (!EFI_ERROR (Status)) {
> + Status = gBS->OpenProtocol (
> + DeviceHandle,
> + &gEfiFirmwareVolume2ProtocolGuid,
> + NULL,
> + NULL,
> + NULL,
> + EFI_OPEN_PROTOCOL_TEST_PROTOCOL
> + );
> + if (!EFI_ERROR (Status)) {
> + return IMAGE_FROM_FV;
> + }
> + }
> +
> + //
> + // Next check to see if File is from a Block I/O device
> + //
> + DeviceHandle = NULL;
> + TempDevicePath = (EFI_DEVICE_PATH_PROTOCOL *) File;
> + Status = gBS->LocateDevicePath (
> + &gEfiBlockIoProtocolGuid,
> + &TempDevicePath,
> + &DeviceHandle
> + );
> + if (!EFI_ERROR (Status)) {
> + BlockIo = NULL;
> + Status = gBS->OpenProtocol (
> + DeviceHandle,
> + &gEfiBlockIoProtocolGuid,
> + (VOID **) &BlockIo,
> + NULL,
> + NULL,
> + EFI_OPEN_PROTOCOL_GET_PROTOCOL
> + );
> + if (!EFI_ERROR (Status) && BlockIo != NULL) {
> + if (BlockIo->Media != NULL) {
> + if (BlockIo->Media->RemovableMedia) {
> + //
> + // Block I/O is present and specifies the media is removable
> + //
> + return IMAGE_FROM_REMOVABLE_MEDIA;
> + } else {
> + //
> + // Block I/O is present and specifies the media is not removable
> + //
> + return IMAGE_FROM_FIXED_MEDIA;
> + }
> + }
> + }
> + }
> +
> + //
> + // File is not in a Firmware Volume or on a Block I/O device, so check to see if
> + // the device path supports the Simple File System Protocol.
> + //
> + DeviceHandle = NULL;
> + TempDevicePath = (EFI_DEVICE_PATH_PROTOCOL *) File;
> + Status = gBS->LocateDevicePath (
> + &gEfiSimpleFileSystemProtocolGuid,
> + &TempDevicePath,
> + &DeviceHandle
> + );
> + if (!EFI_ERROR (Status)) {
> + //
> + // Simple File System is present without Block I/O, so assume media is fixed.
> + //
> + return IMAGE_FROM_FIXED_MEDIA;
> + }
> +
> + //
> + // File is not from an FV, Block I/O or Simple File System, so the only options
> + // left are a PCI Option ROM and a Load File Protocol such as a PXE Boot from a NIC.
> + //
> + TempDevicePath = (EFI_DEVICE_PATH_PROTOCOL *) File;
> + while (!IsDevicePathEndType (TempDevicePath)) {
> + switch (DevicePathType (TempDevicePath)) {
> +
> + case MEDIA_DEVICE_PATH:
> + if (DevicePathSubType (TempDevicePath) == MEDIA_RELATIVE_OFFSET_RANGE_DP) {
> + return IMAGE_FROM_OPTION_ROM;
> + }
> + break;
> +
> + case MESSAGING_DEVICE_PATH:
> + if (DevicePathSubType(TempDevicePath) == MSG_MAC_ADDR_DP) {
> + return IMAGE_FROM_REMOVABLE_MEDIA;
> + }
> + break;
> +
> + default:
> + break;
> + }
> + TempDevicePath = NextDevicePathNode (TempDevicePath);
> + }
> + return IMAGE_UNKNOWN;
> +}
> +
> +/**
> + Get UEFI image protection policy based upon image type.
> +
> + @param[in] ImageType The UEFI image type
> +
> + @return UEFI image protection policy
> +**/
> +UINT32
> +GetProtectionPolicyFromImageType (
> + IN UINT32 ImageType
> + )
> +{
> + UINTN Index;
> +
> + for (Index = 0; Index < ARRAY_SIZE(mDefaultProtectionPolicy); Index++) {
> + if ((UINT32)(1 << Index) == ImageType) {
> + return mDefaultProtectionPolicy[Index];
> + }
> + }
> + return DO_NOT_PROTECT;
> +}
> +
> +/**
> + Get UEFI image protection policy based upon loaded image device path.
> +
> + @param[in] LoadedImage The loaded image protocol
> + @param[in] LoadedImageDevicePath The loaded image device path protocol
> +
> + @return UEFI image protection policy
> +**/
> +UINT32
> +GetUefiImageProtectionPolicy (
> + IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
> + IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
> + )
> +{
> + BOOLEAN InSmm;
> + UINT32 ImageType;
> + UINT32 ProtectionPolicy;
> +
> + //
> + // Check SMM
> + //
> + InSmm = FALSE;
> + if (gSmmBase2 != NULL) {
> + gSmmBase2->InSmm (gSmmBase2, &InSmm);
> + }
> + if (InSmm) {
> + return FALSE;
> + }
> +
> + //
> + // Check DevicePath
> + //
> + if (LoadedImage == gDxeCoreLoadedImage) {
> + ImageType = IMAGE_FROM_FV;
> + } else {
> + ImageType = GetImageType (LoadedImageDevicePath);
> + }
> + ProtectionPolicy = GetProtectionPolicyFromImageType (ImageType);
> + return ProtectionPolicy;
> +}
> +
> +
> +/**
> + Set UEFI image memory attributes.
> +
> + @param[in] BaseAddress Specified start address
> + @param[in] Length Specified length
> + @param[in] Attributes Specified attributes
> +**/
> +VOID
> +SetUefiImageMemoryAttributes (
> + IN UINT64 BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 Attributes
> + )
> +{
> + EFI_STATUS Status;
> + EFI_GCD_MEMORY_SPACE_DESCRIPTOR Descriptor;
> + UINT64 FinalAttributes;
> +
> + Status = CoreGetMemorySpaceDescriptor(BaseAddress, &Descriptor);
> + ASSERT_EFI_ERROR(Status);
> +
> + FinalAttributes = (Descriptor.Attributes & CACHE_ATTRIBUTE_MASK) | (Attributes & MEMORY_ATTRIBUTE_MASK);
> +
> + DEBUG ((DEBUG_INFO, "SetUefiImageMemoryAttributes - 0x%016lx - 0x%016lx (0x%016lx)\n", BaseAddress, Length, FinalAttributes));
> +
> + ASSERT(gCpu != NULL);
> + gCpu->SetMemoryAttributes (gCpu, BaseAddress, Length, FinalAttributes);
> +}
> +
> +/**
> + Set UEFI image protection attributes.
> +
> + @param[in] ImageRecord A UEFI image record
> + @param[in] Protect TRUE: Protect the UEFI image.
> + FALSE: Unprotect the UEFI image.
> +**/
> +VOID
> +SetUefiImageProtectionAttributes (
> + IN IMAGE_PROPERTIES_RECORD *ImageRecord,
> + IN BOOLEAN Protect
> + )
> +{
> + IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
> + LIST_ENTRY *ImageRecordCodeSectionLink;
> + LIST_ENTRY *ImageRecordCodeSectionEndLink;
> + LIST_ENTRY *ImageRecordCodeSectionList;
> + UINT64 CurrentBase;
> + UINT64 ImageEnd;
> + UINT64 Attribute;
> +
> + ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
> +
> + CurrentBase = ImageRecord->ImageBase;
> + ImageEnd = ImageRecord->ImageBase + ImageRecord->ImageSize;
> +
> + ImageRecordCodeSectionLink = ImageRecordCodeSectionList->ForwardLink;
> + ImageRecordCodeSectionEndLink = ImageRecordCodeSectionList;
> + while (ImageRecordCodeSectionLink != ImageRecordCodeSectionEndLink) {
> + ImageRecordCodeSection = CR (
> + ImageRecordCodeSectionLink,
> + IMAGE_PROPERTIES_RECORD_CODE_SECTION,
> + Link,
> + IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
> + );
> + ImageRecordCodeSectionLink = ImageRecordCodeSectionLink->ForwardLink;
> +
> + ASSERT (CurrentBase <= ImageRecordCodeSection->CodeSegmentBase);
> + if (CurrentBase < ImageRecordCodeSection->CodeSegmentBase) {
> + //
> + // DATA
> + //
> + if (Protect) {
> + Attribute = EFI_MEMORY_XP;
> + } else {
> + Attribute = 0;
> + }
> + SetUefiImageMemoryAttributes (
> + CurrentBase,
> + ImageRecordCodeSection->CodeSegmentBase - CurrentBase,
> + Attribute
> + );
> + }
> + //
> + // CODE
> + //
> + if (Protect) {
> + Attribute = EFI_MEMORY_RO;
> + } else {
> + Attribute = 0;
> + }
> + SetUefiImageMemoryAttributes (
> + ImageRecordCodeSection->CodeSegmentBase,
> + ImageRecordCodeSection->CodeSegmentSize,
> + Attribute
> + );
> + CurrentBase = ImageRecordCodeSection->CodeSegmentBase + ImageRecordCodeSection->CodeSegmentSize;
> + }
> + //
> + // Last DATA
> + //
> + ASSERT (CurrentBase <= ImageEnd);
> + if (CurrentBase < ImageEnd) {
> + //
> + // DATA
> + //
> + if (Protect) {
> + Attribute = EFI_MEMORY_XP;
> + } else {
> + Attribute = 0;
> + }
> + SetUefiImageMemoryAttributes (
> + CurrentBase,
> + ImageEnd - CurrentBase,
> + Attribute
> + );
> + }
> + return ;
> +}
> +
> +/**
> + Return if the PE image section is aligned.
> +
> + @param[in] SectionAlignment PE/COFF section alignment
> + @param[in] MemoryType PE/COFF image memory type
> +
> + @retval TRUE The PE image section is aligned.
> + @retval FALSE The PE image section is not aligned.
> +**/
> +BOOLEAN
> +IsMemoryProtectionSectionAligned (
> + IN UINT32 SectionAlignment,
> + IN EFI_MEMORY_TYPE MemoryType
> + )
> +{
> + UINT32 PageAlignment;
> +
> + switch (MemoryType) {
> + case EfiRuntimeServicesCode:
> + case EfiACPIMemoryNVS:
> + PageAlignment = EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT;
> + break;
> + case EfiRuntimeServicesData:
> + case EfiACPIReclaimMemory:
> + ASSERT (FALSE);
> + PageAlignment = EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT;
> + break;
> + case EfiBootServicesCode:
> + case EfiLoaderCode:
> + case EfiReservedMemoryType:
> + PageAlignment = EFI_PAGE_SIZE;
> + break;
> + default:
> + ASSERT (FALSE);
> + PageAlignment = EFI_PAGE_SIZE;
> + break;
> + }
> +
> + if ((SectionAlignment & (PageAlignment - 1)) != 0) {
> + return FALSE;
> + } else {
> + return TRUE;
> + }
> +}
> +
> +/**
> + Free Image record.
> +
> + @param[in] ImageRecord A UEFI image record
> +**/
> +VOID
> +FreeImageRecord (
> + IN IMAGE_PROPERTIES_RECORD *ImageRecord
> + )
> +{
> + LIST_ENTRY *CodeSegmentListHead;
> + IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
> +
> + CodeSegmentListHead = &ImageRecord->CodeSegmentList;
> + while (!IsListEmpty (CodeSegmentListHead)) {
> + ImageRecordCodeSection = CR (
> + CodeSegmentListHead->ForwardLink,
> + IMAGE_PROPERTIES_RECORD_CODE_SECTION,
> + Link,
> + IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
> + );
> + RemoveEntryList (&ImageRecordCodeSection->Link);
> + FreePool (ImageRecordCodeSection);
> + }
> +
> + if (ImageRecord->Link.ForwardLink != NULL) {
> + RemoveEntryList (&ImageRecord->Link);
> + }
> + FreePool (ImageRecord);
> +}
> +
> +/**
> + Protect or unprotect UEFI image common function.
> +
> + @param[in] LoadedImage The loaded image protocol
> + @param[in] LoadedImageDevicePath The loaded image device path protocol
> + @param[in] Protect TRUE: Protect the UEFI image.
> + FALSE: Unprotect the UEFI image.
> +**/
> +VOID
> +ProtectUefiImageCommon (
> + IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
> + IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath,
> + IN BOOLEAN Protect
> + )
> +{
> + VOID *ImageAddress;
> + EFI_IMAGE_DOS_HEADER *DosHdr;
> + UINT32 PeCoffHeaderOffset;
> + UINT32 SectionAlignment;
> + EFI_IMAGE_SECTION_HEADER *Section;
> + EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
> + UINT8 *Name;
> + UINTN Index;
> + IMAGE_PROPERTIES_RECORD *ImageRecord;
> + CHAR8 *PdbPointer;
> + IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
> + UINT16 Magic;
> + BOOLEAN IsAligned;
> + UINT32 ProtectionPolicy;
> +
> + DEBUG ((DEBUG_INFO, "ProtectUefiImageCommon - 0x%x\n", LoadedImage));
> + DEBUG ((DEBUG_INFO, " - 0x%016lx - 0x%016lx\n", (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase, LoadedImage->ImageSize));
> +
> + if (gCpu == NULL) {
> + return ;
> + }
> +
> + ProtectionPolicy = GetUefiImageProtectionPolicy (LoadedImage, LoadedImageDevicePath);
> + switch (ProtectionPolicy) {
> + case DO_NOT_PROTECT:
> + return ;
> + case PROTECT_IF_ALIGNED_ELSE_ALLOW:
> + break;
> + default:
> + ASSERT(FALSE);
> + return ;
> + }
> +
> + ImageRecord = AllocateZeroPool (sizeof(*ImageRecord));
> + if (ImageRecord == NULL) {
> + return ;
> + }
> + ImageRecord->Signature = IMAGE_PROPERTIES_RECORD_SIGNATURE;
> +
> + //
> + // Step 1: record whole region
> + //
> + ImageRecord->ImageBase = (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase;
> + ImageRecord->ImageSize = LoadedImage->ImageSize;
> +
> + ImageAddress = LoadedImage->ImageBase;
> +
> + PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
> + if (PdbPointer != NULL) {
> + DEBUG ((DEBUG_VERBOSE, " Image - %a\n", PdbPointer));
> + }
> +
> + //
> + // Check PE/COFF image
> + //
> + DosHdr = (EFI_IMAGE_DOS_HEADER *) (UINTN) ImageAddress;
> + PeCoffHeaderOffset = 0;
> + if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
> + PeCoffHeaderOffset = DosHdr->e_lfanew;
> + }
> +
> + Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *) (UINTN) ImageAddress + PeCoffHeaderOffset);
> + if (Hdr.Pe32->Signature != EFI_IMAGE_NT_SIGNATURE) {
> + DEBUG ((DEBUG_VERBOSE, "Hdr.Pe32->Signature invalid - 0x%x\n", Hdr.Pe32->Signature));
> + // It might be image in SMM.
> + goto Finish;
> + }
> +
> + //
> + // Get SectionAlignment
> + //
> + if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + //
> + // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> + // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> + // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> + // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> + //
> + Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> + } else {
> + //
> + // Get the magic value from the PE/COFF Optional Header
> + //
> + Magic = Hdr.Pe32->OptionalHeader.Magic;
> + }
> + if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
> + } else {
> + SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
> + }
> +
> + IsAligned = IsMemoryProtectionSectionAligned (SectionAlignment, LoadedImage->ImageCodeType);
> + if (!IsAligned) {
> + DEBUG ((DEBUG_VERBOSE, "!!!!!!!! ProtectUefiImageCommon - Section Alignment(0x%x) is incorrect !!!!!!!!\n",
> + SectionAlignment));
> + PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
> + if (PdbPointer != NULL) {
> + DEBUG ((DEBUG_VERBOSE, "!!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
> + }
> + goto Finish;
> + }
> +
> + Section = (EFI_IMAGE_SECTION_HEADER *) (
> + (UINT8 *) (UINTN) ImageAddress +
> + PeCoffHeaderOffset +
> + sizeof(UINT32) +
> + sizeof(EFI_IMAGE_FILE_HEADER) +
> + Hdr.Pe32->FileHeader.SizeOfOptionalHeader
> + );
> + ImageRecord->CodeSegmentCount = 0;
> + InitializeListHead (&ImageRecord->CodeSegmentList);
> + for (Index = 0; Index < Hdr.Pe32->FileHeader.NumberOfSections; Index++) {
> + Name = Section[Index].Name;
> + DEBUG ((
> + DEBUG_VERBOSE,
> + " Section - '%c%c%c%c%c%c%c%c'\n",
> + Name[0],
> + Name[1],
> + Name[2],
> + Name[3],
> + Name[4],
> + Name[5],
> + Name[6],
> + Name[7]
> + ));
> +
> + if ((Section[Index].Characteristics & EFI_IMAGE_SCN_CNT_CODE) != 0) {
> + DEBUG ((DEBUG_VERBOSE, " VirtualSize - 0x%08x\n", Section[Index].Misc.VirtualSize));
> + DEBUG ((DEBUG_VERBOSE, " VirtualAddress - 0x%08x\n", Section[Index].VirtualAddress));
> + DEBUG ((DEBUG_VERBOSE, " SizeOfRawData - 0x%08x\n", Section[Index].SizeOfRawData));
> + DEBUG ((DEBUG_VERBOSE, " PointerToRawData - 0x%08x\n", Section[Index].PointerToRawData));
> + DEBUG ((DEBUG_VERBOSE, " PointerToRelocations - 0x%08x\n", Section[Index].PointerToRelocations));
> + DEBUG ((DEBUG_VERBOSE, " PointerToLinenumbers - 0x%08x\n", Section[Index].PointerToLinenumbers));
> + DEBUG ((DEBUG_VERBOSE, " NumberOfRelocations - 0x%08x\n", Section[Index].NumberOfRelocations));
> + DEBUG ((DEBUG_VERBOSE, " NumberOfLinenumbers - 0x%08x\n", Section[Index].NumberOfLinenumbers));
> + DEBUG ((DEBUG_VERBOSE, " Characteristics - 0x%08x\n", Section[Index].Characteristics));
> +
> + //
> + // Step 2: record code section
> + //
> + ImageRecordCodeSection = AllocatePool (sizeof(*ImageRecordCodeSection));
> + if (ImageRecordCodeSection == NULL) {
> + return ;
> + }
> + ImageRecordCodeSection->Signature = IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE;
> +
> + ImageRecordCodeSection->CodeSegmentBase = (UINTN)ImageAddress + Section[Index].VirtualAddress;
> + ImageRecordCodeSection->CodeSegmentSize = ALIGN_VALUE(Section[Index].SizeOfRawData, SectionAlignment);
> +
> + DEBUG ((DEBUG_VERBOSE, "ImageCode: 0x%016lx - 0x%016lx\n", ImageRecordCodeSection->CodeSegmentBase, ImageRecordCodeSection->CodeSegmentSize));
> +
> + InsertTailList (&ImageRecord->CodeSegmentList, &ImageRecordCodeSection->Link);
> + ImageRecord->CodeSegmentCount++;
> + }
> + }
> +
> + if (ImageRecord->CodeSegmentCount == 0) {
> + DEBUG ((DEBUG_ERROR, "!!!!!!!! ProtectUefiImageCommon - CodeSegmentCount is 0 !!!!!!!!\n"));
> + PdbPointer = PeCoffLoaderGetPdbPointer ((VOID*) (UINTN) ImageAddress);
> + if (PdbPointer != NULL) {
> + DEBUG ((DEBUG_ERROR, "!!!!!!!! Image - %a !!!!!!!!\n", PdbPointer));
> + }
> + goto Finish;
> + }
> +
> + //
> + // Final
> + //
> + SortImageRecordCodeSection (ImageRecord);
> + //
> + // Check overlap all section in ImageBase/Size
> + //
> + if (!IsImageRecordCodeSectionValid (ImageRecord)) {
> + DEBUG ((DEBUG_ERROR, "IsImageRecordCodeSectionValid - FAIL\n"));
> + goto Finish;
> + }
> +
> + //
> + // CPU ARCH present. Update memory attribute directly.
> + //
> + SetUefiImageProtectionAttributes (ImageRecord, Protect);
> +
> + //
> + // Clean up
> + //
> + FreeImageRecord (ImageRecord);
> +
> +Finish:
> + return ;
> +}
> +
> +/**
> + Protect UEFI image.
> +
> + @param[in] LoadedImage The loaded image protocol
> + @param[in] LoadedImageDevicePath The loaded image device path protocol
> +**/
> +VOID
> +ProtectUefiImage (
> + IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
> + IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
> + )
> +{
> + ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, TRUE);
> +}
> +
> +/**
> + Unprotect UEFI image.
> +
> + @param[in] LoadedImage The loaded image protocol
> + @param[in] LoadedImageDevicePath The loaded image device path protocol
> +**/
> +VOID
> +UnprotectUefiImage (
> + IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
> + IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
> + )
> +{
> + ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, FALSE);
> +}
> +
> +/**
> + A notification for CPU_ARCH protocol.
> +
> + @param[in] Event Event whose notification function is being invoked.
> + @param[in] Context Pointer to the notification function's context,
> + which is implementation-dependent.
> +
> +**/
> +VOID
> +EFIAPI
> +MemoryProtectionCpuArchProtocolNotify (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + EFI_STATUS Status;
> + EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
> + EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath;
> + UINTN NoHandles;
> + EFI_HANDLE *HandleBuffer;
> + UINTN Index;
> +
> + DEBUG ((DEBUG_INFO, "MemoryProtectionCpuArchProtocolNotify:\n"));
> + Status = CoreLocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&gCpu);
> + if (EFI_ERROR (Status)) {
> + return;
> + }
> +
> + Status = gBS->LocateHandleBuffer (
> + ByProtocol,
> + &gEfiLoadedImageProtocolGuid,
> + NULL,
> + &NoHandles,
> + &HandleBuffer
> + );
> + if (EFI_ERROR (Status) && (NoHandles == 0)) {
> + return ;
> + }
> +
> + for (Index = 0; Index < NoHandles; Index++) {
> + Status = gBS->HandleProtocol (
> + HandleBuffer[Index],
> + &gEfiLoadedImageProtocolGuid,
> + (VOID **)&LoadedImage
> + );
> + if (EFI_ERROR(Status)) {
> + continue;
> + }
> + Status = gBS->HandleProtocol (
> + HandleBuffer[Index],
> + &gEfiLoadedImageDevicePathProtocolGuid,
> + (VOID **)&LoadedImageDevicePath
> + );
> + if (EFI_ERROR(Status)) {
> + LoadedImageDevicePath = NULL;
> + }
> +
> + ProtectUefiImage (LoadedImage, LoadedImageDevicePath);
> + }
> +
> + CoreCloseEvent (Event);
> + return;
> +}
> +
> +/**
> + Initialize Memory Protection support.
> +**/
> +VOID
> +EFIAPI
> +CoreInitializeMemoryProtection (
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> + EFI_EVENT Event;
> + VOID *Registration;
> +
> + Status = CoreCreateEvent (
> + EVT_NOTIFY_SIGNAL,
> + TPL_CALLBACK,
> + MemoryProtectionCpuArchProtocolNotify,
> + NULL,
> + &Event
> + );
> + ASSERT_EFI_ERROR(Status);
> +
> + //
> + // Register for protocol notifactions on this event
> + //
> + Status = CoreRegisterProtocolNotify (
> + &gEfiCpuArchProtocolGuid,
> + Event,
> + &Registration
> + );
> + ASSERT_EFI_ERROR(Status);
> + return ;
> +}
> diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> index 7ecad89..a18c02d 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> @@ -36,26 +36,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \
> ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size)))
>
> -#define IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE SIGNATURE_32 ('I','P','R','C')
> -
> -typedef struct {
> - UINT32 Signature;
> - LIST_ENTRY Link;
> - EFI_PHYSICAL_ADDRESS CodeSegmentBase;
> - UINT64 CodeSegmentSize;
> -} IMAGE_PROPERTIES_RECORD_CODE_SECTION;
> -
> -#define IMAGE_PROPERTIES_RECORD_SIGNATURE SIGNATURE_32 ('I','P','R','D')
> -
> -typedef struct {
> - UINT32 Signature;
> - LIST_ENTRY Link;
> - EFI_PHYSICAL_ADDRESS ImageBase;
> - UINT64 ImageSize;
> - UINTN CodeSegmentCount;
> - LIST_ENTRY CodeSegmentList;
> -} IMAGE_PROPERTIES_RECORD;
> -
> #define IMAGE_PROPERTIES_PRIVATE_DATA_SIGNATURE SIGNATURE_32 ('I','P','P','D')
>
> typedef struct {
> @@ -864,7 +844,6 @@ SwapImageRecordCodeSection (
>
> @param ImageRecord image record to be sorted
> **/
> -STATIC
> VOID
> SortImageRecordCodeSection (
> IN IMAGE_PROPERTIES_RECORD *ImageRecord
> @@ -915,7 +894,6 @@ SortImageRecordCodeSection (
> @retval TRUE image record is valid
> @retval FALSE image record is invalid
> **/
> -STATIC
> BOOLEAN
> IsImageRecordCodeSectionValid (
> IN IMAGE_PROPERTIES_RECORD *ImageRecord
> --
> 2.7.4.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2017-02-06 14:51 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-26 8:39 [PATCH 0/3] DXE Memory Protection Jiewen Yao
2017-01-26 8:39 ` [PATCH 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting Jiewen Yao
2017-02-03 8:51 ` Gao, Liming
2017-02-04 0:42 ` Yao, Jiewen
2017-02-06 3:27 ` Fan, Jeff
2017-02-06 3:58 ` Yao, Jiewen
2017-02-06 5:26 ` Fan, Jeff
2017-02-06 5:28 ` Yao, Jiewen
2017-01-26 8:39 ` [PATCH 2/3] ArmPkg/CpuDxe: Correct EFI_MEMORY_RO usage Jiewen Yao
2017-01-26 12:11 ` Leif Lindholm
2017-01-26 12:17 ` Yao, Jiewen
2017-01-26 8:39 ` [PATCH 3/3] MdeModulePkg/DxeCore: Add UEFI image protection Jiewen Yao
2017-02-03 8:39 ` Gao, Liming
2017-02-04 0:41 ` Yao, Jiewen
2017-02-04 3:39 ` Gao, Liming
2017-02-06 1:07 ` Tian, Feng
2017-02-06 1:39 ` Yao, Jiewen
2017-02-06 14:34 ` Ard Biesheuvel
2017-02-06 14:51 ` Yao, Jiewen [this message]
2017-02-06 14:52 ` Ard Biesheuvel
2017-02-06 17:52 ` Kinney, Michael D
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74D8A39837DF1E4DA445A8C0B3885C503A8E9938@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox