UefiCpuPkg: ArmMmuLib: Check if Block Split Following Page Alloc

Currently, there is a bug in UpdateRegionMappingRecursive() when
guard pages are enabled and a large page is being split.

The code checks whether the page table is a block or table and
seeing that it is a block, allocates a new page table for the next
level. However, when it does this, it will call an additional recursive
call into the page table updating logic to make sure the new page table
page is mapped. In addition, when guard pages are enabled, it will
mark the guard page as RP. If the guard page is in the same block as
we are already trying to split, the recursive call will split the
block and mark the guard page as RP.

When we return to the original call, it will fill out the now
orphaned page table but never install it into the page table
hierarchy (and if it did, it would lose the guard page). This
has been observed to cause a driver's code section to still have
NX set on it and so crash when trying to execute.

This commit resolves the issue by checking if the block has
already been split when we return from the new page table
allocation. If it has, we simply update the existing table mapping
instead of trying to split the block.

The allocated page table page cannot be immediately freed, because
this might trigger the block to get re-merged, so a reference to
it is held until the end of updating this level and subsequent
levels, when it can be safely freed. It is possible that the mapping
extends across two large pages and this issue could exist on both
sides, so in the worst case we may have two orphaned tables to
free.

Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
This commit is contained in:
Oliver Smith-Denny 2026-04-01 13:32:04 -07:00 committed by mergify[bot]
parent 5107351f77
commit b2a149cf97

View file

@ -278,12 +278,16 @@ UpdateRegionMappingRecursive (
VOID *TranslationTable;
EFI_STATUS Status;
BOOLEAN NextTableIsLive;
VOID *TablesToFree[2];
ASSERT (((RegionStart | RegionEnd) & EFI_PAGE_MASK) == 0);
BlockShift = (Level + 1) * BITS_PER_LEVEL + MIN_T0SZ;
BlockMask = MAX_UINT64 >> BlockShift;
TablesToFree[0] = NULL;
TablesToFree[1] = NULL;
DEBUG ((
DEBUG_PAGING,
"%a(%d): %llx - %llx set %lx clr %lx\n",
@ -338,44 +342,65 @@ UpdateRegionMappingRecursive (
return EFI_OUT_OF_RESOURCES;
}
if (!ArmMmuEnabled ()) {
//
// Allocating a page may have split this block if a guard page
// was allocated in this block. Check if this is already split
// and if so skip the splitting logic
//
if (IsTableEntry (*Entry, Level)) {
//
// Make sure we are not inadvertently hitting in the caches
// when populating the page tables.
// Don't free the page table here, we may end up recreating the
// large page. This mapping may extend across the block boundary,
// so its possible we could have two pages to free in the worst case.
//
InvalidateDataCacheRange (TranslationTable, EFI_PAGE_SIZE);
}
ZeroMem (TranslationTable, EFI_PAGE_SIZE);
if (IsBlockEntry (*Entry, Level)) {
//
// We are splitting an existing block entry, so we have to populate
// the new table with the attributes of the block entry it replaces.
//
Status = UpdateRegionMappingRecursive (
RegionStart & ~BlockMask,
(RegionStart | BlockMask) + 1,
*Entry & TT_ATTRIBUTES_MASK,
0,
TranslationTable,
Level + 1,
FALSE,
FALSE,
Lpa2Enabled
);
if (EFI_ERROR (Status)) {
//
// The range we passed to UpdateRegionMappingRecursive () is block
// aligned, so it is guaranteed that no further pages were allocated
// by it, and so we only have to free the page we allocated here.
//
FreePages (TranslationTable, 1);
return Status;
if (TablesToFree[0] == NULL) {
TablesToFree[0] = TranslationTable;
} else {
TablesToFree[1] = TranslationTable;
}
}
NextTableIsLive = FALSE;
TranslationTable = (VOID *)GetOutputAddress (*Entry, Lpa2Enabled);
NextTableIsLive = TableIsLive;
} else {
if (!ArmMmuEnabled ()) {
//
// Make sure we are not inadvertently hitting in the caches
// when populating the page tables.
//
InvalidateDataCacheRange (TranslationTable, EFI_PAGE_SIZE);
}
ZeroMem (TranslationTable, EFI_PAGE_SIZE);
if (IsBlockEntry (*Entry, Level)) {
//
// We are splitting an existing block entry, so we have to populate
// the new table with the attributes of the block entry it replaces.
//
Status = UpdateRegionMappingRecursive (
RegionStart & ~BlockMask,
(RegionStart | BlockMask) + 1,
*Entry & TT_ATTRIBUTES_MASK,
0,
TranslationTable,
Level + 1,
FALSE,
FALSE,
Lpa2Enabled
);
if (EFI_ERROR (Status)) {
//
// The range we passed to UpdateRegionMappingRecursive () is block
// aligned, so it is guaranteed that no further pages were allocated
// by it, and so we only have to free the page we allocated here.
//
FreePages (TranslationTable, 1);
return Status;
}
}
NextTableIsLive = FALSE;
}
} else {
TranslationTable = (VOID *)GetOutputAddress (*Entry, Lpa2Enabled);
NextTableIsLive = TableIsLive;
@ -433,6 +458,20 @@ UpdateRegionMappingRecursive (
}
}
//
// We may have left up to two orphaned page table pages if we discovered a
// recursive call already split a block on either side of a misaligned region.
//
if (TablesToFree[0] != NULL) {
FreePages (TablesToFree[0], 1);
TablesToFree[0] = NULL;
}
if (TablesToFree[1] != NULL) {
FreePages (TablesToFree[1], 1);
TablesToFree[1] = NULL;
}
return EFI_SUCCESS;
}