From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.126; helo=mga18.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel@lists.01.org Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B738F21180F29 for ; Thu, 25 Oct 2018 00:18:17 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Oct 2018 00:18:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,423,1534834800"; d="scan'208";a="275453770" Received: from shwdeopenpsi777.ccr.corp.intel.com ([10.239.158.27]) by fmsmga006.fm.intel.com with ESMTP; 25 Oct 2018 00:18:16 -0700 From: Jian J Wang To: edk2-devel@lists.01.org Cc: Eric Dong , Laszlo Ersek , Star Zeng , Michael D Kinney , Jiewen Yao , Ruiyu Ni Date: Thu, 25 Oct 2018 15:18:03 +0800 Message-Id: <20181025071805.6692-5-jian.j.wang@intel.com> X-Mailer: git-send-email 2.16.2.windows.1 In-Reply-To: <20181025071805.6692-1-jian.j.wang@intel.com> References: <20181025071805.6692-1-jian.j.wang@intel.com> Subject: [PATCH v4 4/6] UefiCpuPkg/CpuDxe: prevent recursive calling of InitializePageTablePool X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Oct 2018 07:18:17 -0000 > v4 changes: none 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() | => | => 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: Eric Dong Cc: Laszlo Ersek Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- 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