Skip to content

Commit

Permalink
UefiCpuPkg/CpuDxe: prevent recursive calling of InitializePageTablePool
Browse files Browse the repository at this point in the history
> v3 changes:
> a. split from tianocore#2 patch
> b. refine the commit message and title
> c. remove "else" branch

The freed-memory guard feature will cause an recursive calling
of InitializePageTablePool(). This is due to a fact that
AllocateAlignedPages() is used to allocate page table pool memory.
This function will most likely call gBS->FreePages to free unaligned
pages and then cause another round of page attributes change, like
below (freed pages will be always marked not-present if freed-memory
guard is enabled)

   FreePages() <===============|
=> CpuSetMemoryAttributes()    |
=> <if out of page table>      |
=> InitializePageTablePool()   |
=> AllocateAlignedPages()      |
=> FreePages() ================|

The solution is add a global variable as a lock in page table pool
allocation function and fail any other requests if it has not been
done.

Since this issue will only happen if free-memory guard is enabled,
it won't affect CpuSetMemoryAttributes() in default build of a BIOS.

If free-memory guard is enabled, it only affect the pages
(failed to mark them as not-present) freed in AllocateAlignedPages().

Since those freed pages haven't been used yet (their addresses not
yet exposed to code outside AllocateAlignedPages), it won't compromise
the freed-memory guard feature.

This change will just fail the CpuSetMemoryAttributes() called from
FreePages() but it won't fail the FreePages(). So the error status
won't be propagated upper layer of code.

Cc: Laszlo Ersek <[email protected]>
Cc: Star Zeng <[email protected]>
Cc: Michael D Kinney <[email protected]>
Cc: Jiewen Yao <[email protected]>
Cc: Ruiyu Ni <[email protected]>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <[email protected]>
  • Loading branch information
Jian J Wang committed Oct 24, 2018
1 parent c207ae6 commit 982bb0d
Showing 1 changed file with 21 additions and 2 deletions.
23 changes: 21 additions & 2 deletions UefiCpuPkg/CpuDxe/CpuPageTable.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
};

PAGE_TABLE_POOL *mPageTablePool = NULL;
BOOLEAN mPageTablePoolLock = FALSE;
PAGE_TABLE_LIB_PAGING_CONTEXT mPagingContext;
EFI_SMM_BASE2_PROTOCOL *mSmmBase2 = NULL;

Expand Down Expand Up @@ -1046,6 +1047,16 @@ InitializePageTablePool (
VOID *Buffer;
BOOLEAN IsModified;

//
// Do not allow re-entrance.
//
if (mPageTablePoolLock) {
return FALSE;
}

mPageTablePoolLock = TRUE;
IsModified = FALSE;

//
// Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page for
// header.
Expand All @@ -1056,9 +1067,15 @@ InitializePageTablePool (
Buffer = AllocateAlignedPages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT);
if (Buffer == NULL) {
DEBUG ((DEBUG_ERROR, "ERROR: Out of aligned pages\r\n"));
return FALSE;
goto Done;
}

DEBUG ((
DEBUG_INFO,
"Paging: added %lu pages to page table pool\r\n",
(UINT64)PoolPages
));

//
// Link all pools into a list for easier track later.
//
Expand Down Expand Up @@ -1092,7 +1109,9 @@ InitializePageTablePool (
);
ASSERT (IsModified == TRUE);

return TRUE;
Done:
mPageTablePoolLock = FALSE;
return IsModified;
}

/**
Expand Down

0 comments on commit 982bb0d

Please sign in to comment.