Thanks for the Reviewed-by.
Sorry I did not check the whole thread at
https://lists.01.org/pipermail/edk2-devel/2017-July/012283.html that has the
discussion about the validity of BASE 0 + LENGTH 4G (Case1) and BASE 4G +
LENGTH 0 (Case2).
Current code has filtered Case2.
@retval RETURN_INVALID_PARAMETER Length is zero.
if (Length == 0) {
return RETURN_INVALID_PARAMETER;
}
If we agree Case1 needs to be considered, I am ok to update the patch
accordingly. :)
Thanks,
Star
From: Yao, Jiewen
Sent: Thursday, August 24, 2017 4:52 PM
To: Zeng, Star <[email protected]>; [email protected]
Cc: Laszlo Ersek <[email protected]>; Dong, Eric <[email protected]>; Zeng,
Star <[email protected]>
Subject: RE: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Fix memory protection
crash
Reviewed-by: [email protected]<mailto:[email protected]>
From: edk2-devel [mailto:[email protected]] On Behalf Of Star Zeng
Sent: Thursday, August 24, 2017 11:20 AM
To: [email protected]<mailto:[email protected]>
Cc: Laszlo Ersek <[email protected]<mailto:[email protected]>>; Yao, Jiewen
<[email protected]<mailto:[email protected]>>; Dong, Eric
<[email protected]<mailto:[email protected]>>; Zeng, Star
<[email protected]<mailto:[email protected]>>
Subject: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Fix memory protection crash
https://bugzilla.tianocore.org/show_bug.cgi?id=624 reports
memory protection crash in PiSmmCpuDxeSmm, Ia32 build with
RAM above 4GB (of which 2GB are placed in 64-bit address).
It is because UEFI builds identity mapping page tables,
>4G address is not supported at Ia32 build.
This patch is to get the PhysicalAddressBits that is used
to build in PageTbl.c(Ia32/X64), and use it to check whether
the address is supported or not in ConvertMemoryPageAttributes().
With this patch, the debug messages will be like below.
UefiMemory protection: 0x0 - 0x9F000 Success
UefiMemory protection: 0x100000 - 0x807000 Success
UefiMemory protection: 0x808000 - 0x810000 Success
UefiMemory protection: 0x818000 - 0x820000 Success
UefiMemory protection: 0x1510000 - 0x7B798000 Success
UefiMemory protection: 0x7B79B000 - 0x7E538000 Success
UefiMemory protection: 0x7E539000 - 0x7E545000 Success
UefiMemory protection: 0x7E55A000 - 0x7E61F000 Success
UefiMemory protection: 0x7E62B000 - 0x7F6AB000 Success
UefiMemory protection: 0x7F703000 - 0x7F70B000 Success
UefiMemory protection: 0x7F70F000 - 0x7F778000 Success
UefiMemory protection: 0x100000000 - 0x180000000 Unsupported
Cc: Jiewen Yao <[email protected]<mailto:[email protected]>>
Cc: Laszlo Ersek <[email protected]<mailto:[email protected]>>
Cc: Eric Dong <[email protected]<mailto:[email protected]>>
Originally-suggested-by: Jiewen Yao
<[email protected]<mailto:[email protected]>>
Reported-by: Laszlo Ersek <[email protected]<mailto:[email protected]>>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <[email protected]<mailto:[email protected]>>
---
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 4 +++
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 1 +
UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 31 +++++++++++++++++-----
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index 32ce5958c59c..e88b42d73343 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -16,6 +16,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
#include "PiSmmCpuDxeSmm.h"
+UINT8 mPhysicalAddressBits;
+
/**
Create PageTable for SMM use.
@@ -36,6 +38,8 @@ SmmInitPageTable (
//
InitializeSpinLock (mPFLock);
+ mPhysicalAddressBits = 32;
+
if (FeaturePcdGet (PcdCpuSmmProfileEnable)) {
//
// Set own Page Fault entry instead of the default one, because SMM Profile
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index dbce9ec520fe..1cf85c1481a7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -419,6 +419,7 @@ extern SPIN_LOCK
*mConfigSmmCodeAccessCheckLock;
extern SPIN_LOCK *mMemoryMappedLock;
extern EFI_SMRAM_DESCRIPTOR *mSmmCpuSmramRanges;
extern UINTN mSmmCpuSmramRangeCount;
+extern UINT8 mPhysicalAddressBits;
//
// Copy of the PcdPteMemoryEncryptionAddressOrMask
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index a535389c26ce..3ad5256f1e03 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -1,6 +1,6 @@
/** @file
-Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2017, 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
@@ -380,6 +380,7 @@ ConvertMemoryPageAttributes (
PAGE_ATTRIBUTE SplitAttribute;
RETURN_STATUS Status;
BOOLEAN IsEntryModified;
+ EFI_PHYSICAL_ADDRESS MaximumSupportMemAddress;
ASSERT (Attributes != 0);
ASSERT ((Attributes & ~(EFI_MEMORY_RP | EFI_MEMORY_RO | EFI_MEMORY_XP)) ==
0);
@@ -391,6 +392,17 @@ ConvertMemoryPageAttributes (
return RETURN_INVALID_PARAMETER;
}
+ MaximumSupportMemAddress = (EFI_PHYSICAL_ADDRESS)(UINTN)(LShiftU64 (1,
mPhysicalAddressBits) - 1);
+ if (BaseAddress > MaximumSupportMemAddress) {
+ return RETURN_UNSUPPORTED;
+ }
+ if (Length > MaximumSupportMemAddress) {
+ return RETURN_UNSUPPORTED;
+ }
+ if ((Length != 0) && (BaseAddress > MaximumSupportMemAddress - (Length -
1))) {
+ return RETURN_UNSUPPORTED;
+ }
+
// DEBUG ((DEBUG_ERROR, "ConvertMemoryPageAttributes(%x) - %016lx, %016lx,
%02lx\n", IsSet, BaseAddress, Length, Attributes));
if (IsSplitted != NULL) {
@@ -1037,6 +1049,7 @@ SetUefiMemMapAttributes (
VOID
)
{
+ EFI_STATUS Status;
EFI_MEMORY_DESCRIPTOR *MemoryMap;
UINTN MemoryMapEntryCount;
UINTN Index;
@@ -1052,12 +1065,18 @@ SetUefiMemMapAttributes (
MemoryMap = mUefiMemoryMap;
for (Index = 0; Index < MemoryMapEntryCount; Index++) {
if (IsUefiPageNotPresent(MemoryMap)) {
- DEBUG ((DEBUG_INFO, "UefiMemory protection: 0x%lx - 0x%lx\n",
MemoryMap->PhysicalStart, MemoryMap->PhysicalStart +
(UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages)));
- SmmSetMemoryAttributes (
+ Status = SmmSetMemoryAttributes (
+ MemoryMap->PhysicalStart,
+ EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),
+ EFI_MEMORY_RP
+ );
+ DEBUG ((
+ DEBUG_INFO,
+ "UefiMemory protection: 0x%lx - 0x%lx %r\n",
MemoryMap->PhysicalStart,
- EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),
- EFI_MEMORY_RP
- );
+ MemoryMap->PhysicalStart +
(UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),
+ Status
+ ));
}
MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);
}
--
2.7.0.windows.1
_______________________________________________
edk2-devel mailing list
[email protected]<mailto:[email protected]>
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
