[PATCH] MdeModulePkg/DxeIplPeim: fix incorrect page table split during protecting

[PATCH] MdeModulePkg/DxeIplPeim: fix incorrect page table split during protecting

[Jian J Wang]

The root cause of this issue is that, during splitting page table, the page
size should be the value of next level (smaller one) instead of current level.
The wrong page size will then cause wrong page table introduced, which will
break the normal boot.

Validation works include booting to Windows 10 and Fedora 26 on real Intel
platform and OVMF emulated platform in addition to manual checks on page
table with JTAG tool.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 26116e420c..bbdfa2bb8e 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -472,6 +472,8 @@ SetPageTablePoolReadOnly (
       //
       // The smaller granularity of page must be needed.
       //
+      ASSERT (Level > 1);
+
       NewPageTable = AllocatePageTableMemory (1);
       ASSERT (NewPageTable != NULL);
 
@@ -481,10 +483,10 @@ SetPageTablePoolReadOnly (
             ++EntryIndex) {
         NewPageTable[EntryIndex] = PhysicalAddress  | AddressEncMask |
                                    IA32_PG_P | IA32_PG_RW;
-        if (Level > 1) {
+        if (Level > 2) {
           NewPageTable[EntryIndex] |= IA32_PG_PS;
         }
-        PhysicalAddress += LevelSize[Level];
+        PhysicalAddress += LevelSize[Level - 1];
       }
 
       PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask |
-- 
2.14.1.windows.1

Re: [PATCH] MdeModulePkg/DxeIplPeim: fix incorrect page table split during protecting

[Zeng, Star]

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: Wang, Jian J 
Sent: Friday, December 15, 2017 10:39 AM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: [PATCH] MdeModulePkg/DxeIplPeim: fix incorrect page table split during protecting

The root cause of this issue is that, during splitting page table, the page size should be the value of next level (smaller one) instead of current level.
The wrong page size will then cause wrong page table introduced, which will break the normal boot.

Validation works include booting to Windows 10 and Fedora 26 on real Intel platform and OVMF emulated platform in addition to manual checks on page table with JTAG tool.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 26116e420c..bbdfa2bb8e 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -472,6 +472,8 @@ SetPageTablePoolReadOnly (
       //
       // The smaller granularity of page must be needed.
       //
+      ASSERT (Level > 1);
+
       NewPageTable = AllocatePageTableMemory (1);
       ASSERT (NewPageTable != NULL);
 
@@ -481,10 +483,10 @@ SetPageTablePoolReadOnly (
             ++EntryIndex) {
         NewPageTable[EntryIndex] = PhysicalAddress  | AddressEncMask |
                                    IA32_PG_P | IA32_PG_RW;
-        if (Level > 1) {
+        if (Level > 2) {
           NewPageTable[EntryIndex] |= IA32_PG_PS;
         }
-        PhysicalAddress += LevelSize[Level];
+        PhysicalAddress += LevelSize[Level - 1];
       }
 
       PageTable[Index] = (UINT64)(UINTN)NewPageTable | AddressEncMask |
--
2.14.1.windows.1