[edk2-devel] [PATCH v3 1/3] ArmPkg/ArmMmuLib AARCH64: limit recursion when freeing page tables

[Ashish Singhal]

Reviewed-by: Ashish Singhal <ashishsingha at nvidia.com>
Tested-by: Ashish Singhal <ashishsingha at nvidia.com>
________________________________
From: Ard Biesheuvel <ard.biesheuvel at linaro.org>
Sent: Wednesday, March 25, 2020 9:29 AM
To: devel at edk2.groups.io <devel at edk2.groups.io>
Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>; Laszlo Ersek <lersek at redhat.com>; Leif Lindholm <leif at nuviainc.com>; Ashish Singhal <ashishsingha at nvidia.com>
Subject: [PATCH v3 1/3] ArmPkg/ArmMmuLib AARCH64: limit recursion when freeing page tables

External email: Use caution opening links or attachments


FreePageTablesRecursive () traverses the page table tree depth first
to free all pages that it finds, without taking into account the
level at which it is operating.

Since TT_TYPE_TABLE_ENTRY aliases TT_TYPE_BLOCK_ENTRY_LEVEL3, we cannot
distinguish table entries from block entries unless we take the level
into account, and so we may be dereferencing garbage if we happen to
try and free a hierarchy of page tables that has level 3 pages in it.

Let's fix this by passing the level into FreePageTablesRecursive (),
and limit the recursion to levels < 3.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index a43d468b73ca..d78918cf7ba8 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -142,15 +142,21 @@ ReplaceTableEntry (
 STATIC
 VOID
 FreePageTablesRecursive (
-  IN  UINT64  *TranslationTable
+  IN  UINT64  *TranslationTable,
+  IN  UINTN   Level
   )
 {
   UINTN   Index;

-  for (Index = 0; Index < TT_ENTRY_COUNT; Index++) {
-    if ((TranslationTable[Index] & TT_TYPE_MASK) == TT_TYPE_TABLE_ENTRY) {
-      FreePageTablesRecursive ((VOID *)(UINTN)(TranslationTable[Index] &
-                                               TT_ADDRESS_MASK_BLOCK_ENTRY));
+  ASSERT (Level <= 3);
+
+  if (Level < 3) {
+    for (Index = 0; Index < TT_ENTRY_COUNT; Index++) {
+      if ((TranslationTable[Index] & TT_TYPE_MASK) == TT_TYPE_TABLE_ENTRY) {
+        FreePageTablesRecursive ((VOID *)(UINTN)(TranslationTable[Index] &
+                                                 TT_ADDRESS_MASK_BLOCK_ENTRY),
+                                 Level + 1);
+      }
     }
   }
   FreePages (TranslationTable, 1);
@@ -254,7 +260,7 @@ UpdateRegionMappingRecursive (
           // possible for existing table entries, since we cannot revert the
           // modifications we made to the subhierarchy it represents.)
           //
-          FreePageTablesRecursive (TranslationTable);
+          FreePageTablesRecursive (TranslationTable, Level + 1);
         }
         return Status;
       }
--
2.17.1


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56304): https://edk2.groups.io/g/devel/message/56304
Mute This Topic: https://groups.io/mt/72543072/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20200325/647f4a55/attachment.htm>