From d390920ed41ebdf78402288416a284cf8bcde620 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Wed, 25 Mar 2020 15:54:09 +0100 Subject: ArmPkg/ArmMmuLib AARCH64: limit recursion when freeing page tables 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 Reviewed-by: Leif Lindholm Reviewed-by: Ashish Singhal Tested-by: Ashish Singhal Tested-by: Laszlo Ersek --- ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'ArmPkg') diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c index a43d468b73..d78918cf7b 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; } -- cgit v1.2.3