Reviewed-by: Ray Ni <ray...@intel.com>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Thursday, June 8, 2023 10:28 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Kumar,
> Rahul R <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>
> Subject: [edk2-devel] [Patch V5 05/14] UefiCpuPkg/PiSmmCpuDxeSmm:
> Avoid setting non-present range to RO/NX
>
> In PiSmmCpuDxeSmm code, SetMemMapAttributes() marks memory ranges
> in SmmMemoryAttributesTable to RO/NX. There may exist non-present
> range in these memory ranges. Set other attributes for a non-present
> range is not permitted in CpuPageTableMapLib. So add code to handle
> this case. Only map the present ranges in SmmMemoryAttributesTable
> to RO or NX.
>
> Signed-off-by: Dun Tan <dun....@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Cc: Gerd Hoffmann <kra...@redhat.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 129
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +++++++++++++++++++++++++++++++++++++++++++++++++---------------
> -------
> 1 file changed, 107 insertions(+), 22 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 862b3e9720..3c79927c7b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -918,6 +918,70 @@ PatchGdtIdtMap (
> );
> }
>
> +/**
> + This function set [Base, Limit] to the input MemoryAttribute.
> +
> + @param Base Start address of range.
> + @param Limit Limit address of range.
> + @param Attribute The bit mask of attributes to modify for the memory
> region.
> + @param Map Pointer to the array of Cr3 IA32_MAP_ENTRY.
> + @param Count Count of IA32_MAP_ENTRY in Map.
> +**/
> +VOID
> +SetMemMapWithNonPresentRange (
> + UINT64 Base,
> + UINT64 Limit,
> + UINT64 Attribute,
> + IA32_MAP_ENTRY *Map,
> + UINTN Count
> + )
> +{
> + UINTN Index;
> + UINT64 NonPresentRangeStart;
> +
> + NonPresentRangeStart = 0;
> + for (Index = 0; Index < Count; Index++) {
> + if ((Map[Index].LinearAddress > NonPresentRangeStart) &&
> + (Base < Map[Index].LinearAddress) && (Limit > NonPresentRangeStart))
> + {
> + //
> + // We should NOT set attributes for non-present ragne.
> + //
> + //
> + // There is a non-present ( [NonPresentStart,
> Map[Index].LinearAddress] ) range before current Map[Index]
> + // and it is overlapped with [Base, Limit].
> + //
> + if (Base < NonPresentRangeStart) {
> + SmmSetMemoryAttributes (
> + Base,
> + NonPresentRangeStart - Base,
> + Attribute
> + );
> + }
> +
> + Base = Map[Index].LinearAddress;
> + }
> +
> + NonPresentRangeStart = Map[Index].LinearAddress + Map[Index].Length;
> + if (NonPresentRangeStart >= Limit) {
> + break;
> + }
> + }
> +
> + Limit = MIN (NonPresentRangeStart, Limit);
> +
> + if (Base < Limit) {
> + //
> + // There is no non-present range in current [Base, Limit] anymore.
> + //
> + SmmSetMemoryAttributes (
> + Base,
> + Limit - Base,
> + Attribute
> + );
> + }
> +}
> +
> /**
> This function sets memory attribute according to MemoryAttributesTable.
> **/
> @@ -932,6 +996,11 @@ SetMemMapAttributes (
> UINTN DescriptorSize;
> UINTN Index;
> EDKII_PI_SMM_MEMORY_ATTRIBUTES_TABLE *MemoryAttributesTable;
> + UINTN PageTable;
> + EFI_STATUS Status;
> + IA32_MAP_ENTRY *Map;
> + UINTN Count;
> + UINT64 MemoryAttribute;
>
> SmmGetSystemConfigurationTable
> (&gEdkiiPiSmmMemoryAttributesTableGuid, (VOID
> **)&MemoryAttributesTable);
> if (MemoryAttributesTable == NULL) {
> @@ -958,36 +1027,52 @@ SetMemMapAttributes (
> MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap,
> DescriptorSize);
> }
>
> + Count = 0;
> + Map = NULL;
> + PageTable = AsmReadCr3 ();
> + Status = PageTableParse (PageTable, mPagingMode, NULL, &Count);
> + while (Status == RETURN_BUFFER_TOO_SMALL) {
> + if (Map != NULL) {
> + FreePool (Map);
> + }
> +
> + Map = AllocatePool (Count * sizeof (IA32_MAP_ENTRY));
> + ASSERT (Map != NULL);
> + Status = PageTableParse (PageTable, mPagingMode, Map, &Count);
> + }
> +
> + ASSERT_RETURN_ERROR (Status);
> +
> MemoryMap = MemoryMapStart;
> for (Index = 0; Index < MemoryMapEntryCount; Index++) {
> DEBUG ((DEBUG_VERBOSE, "SetAttribute: Memory Entry - 0x%lx, 0x%x\n",
> MemoryMap->PhysicalStart, MemoryMap->NumberOfPages));
> - switch (MemoryMap->Type) {
> - case EfiRuntimeServicesCode:
> - SmmSetMemoryAttributes (
> - MemoryMap->PhysicalStart,
> - EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
> - EFI_MEMORY_RO
> - );
> - break;
> - case EfiRuntimeServicesData:
> - SmmSetMemoryAttributes (
> - MemoryMap->PhysicalStart,
> - EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
> - EFI_MEMORY_XP
> - );
> - break;
> - default:
> - SmmSetMemoryAttributes (
> - MemoryMap->PhysicalStart,
> - EFI_PAGES_TO_SIZE ((UINTN)MemoryMap->NumberOfPages),
> - EFI_MEMORY_XP
> - );
> - break;
> + if (MemoryMap->Type == EfiRuntimeServicesCode) {
> + MemoryAttribute = EFI_MEMORY_RO;
> + } else {
> + ASSERT ((MemoryMap->Type == EfiRuntimeServicesData) ||
> (MemoryMap->Type == EfiConventionalMemory));
> + //
> + // Set other type memory as NX.
> + //
> + MemoryAttribute = EFI_MEMORY_XP;
> }
>
> + //
> + // There may exist non-present range overlaps with the MemoryMap
> range.
> + // Do not change other attributes of non-present range while still
> remaining it as non-present
> + //
> + SetMemMapWithNonPresentRange (
> + MemoryMap->PhysicalStart,
> + MemoryMap->PhysicalStart + EFI_PAGES_TO_SIZE
> ((UINTN)MemoryMap->NumberOfPages),
> + MemoryAttribute,
> + Map,
> + Count
> + );
> +
> MemoryMap = NEXT_MEMORY_DESCRIPTOR (MemoryMap,
> DescriptorSize);
> }
>
> + FreePool (Map);
> +
> PatchSmmSaveStateMap ();
> PatchGdtIdtMap ();
>
> --
> 2.31.1.windows.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105928): https://edk2.groups.io/g/devel/message/105928
Mute This Topic: https://groups.io/mt/99399229/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
