public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jian J Wang <jian.j.wang@intel.com>
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek <lersek@redhat.com>, Star Zeng <star.zeng@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Jiewen Yao <jiewen.yao@intel.com>, Ruiyu Ni <ruiyu.ni@intel.com>
Subject: [PATCH v3 4/6] UefiCpuPkg/CpuDxe: prevent recursive calling of InitializePageTablePool
Date: Wed, 24 Oct 2018 13:26:18 +0800	[thread overview]
Message-ID: <20181024052620.4088-5-jian.j.wang@intel.com> (raw)
In-Reply-To: <20181024052620.4088-1-jian.j.wang@intel.com>

> v3 changes:
> a. split from #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 to upper layer of code.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index 33e8ee2d2c..4bee8c7772 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -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;
 
@@ -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.
@@ -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.
   //
@@ -1092,7 +1109,9 @@ InitializePageTablePool (
     );
   ASSERT (IsModified == TRUE);
 
-  return TRUE;
+Done:
+  mPageTablePoolLock = FALSE;
+  return IsModified;
 }
 
 /**
-- 
2.16.2.windows.1



  parent reply	other threads:[~2018-10-24  5:27 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-24  5:26 [PATCH v3 0/6] Introduce freed-memory guard feature Jian J Wang
2018-10-24  5:26 ` [PATCH v3 1/6] MdeModulePkg: cleanup Heap Guard pool/page type PCD documentation Jian J Wang
2018-10-25  2:55   ` Zeng, Star
2018-10-25  4:21     ` Wang, Jian J
2018-10-24  5:26 ` [PATCH v3 2/6] MdeModulePkg: introduce UEFI freed-memory guard bit in HeapGuard PCD Jian J Wang
2018-10-25  3:02   ` Zeng, Star
2018-10-25  4:23     ` Wang, Jian J
2018-10-24  5:26 ` [PATCH v3 3/6] UefiCpuPkg/CpuDxe: consider freed-memory guard in non-stop mode Jian J Wang
2018-10-24  5:26 ` Jian J Wang [this message]
2018-10-24  5:26 ` [PATCH v3 5/6] MdeModulePkg/Core: prevent re-acquire GCD memory lock Jian J Wang
2018-10-25  3:22   ` Zeng, Star
2018-10-25  4:24     ` Wang, Jian J
2018-10-24  5:26 ` [PATCH v3 6/6] MdeModulePkg/Core: add freed-memory guard feature Jian J Wang
2018-10-25  3:37   ` Zeng, Star
2018-10-25  4:29     ` Wang, Jian J
2018-10-25  6:28     ` Wang, Jian J

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181024052620.4088-5-jian.j.wang@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox