[edk2-devel] [Patch V4] UefiCpuPkg/PiSmmCpuDxeSmm:Fix PF issue caused by smm page table code
Ni, Ray
ray.ni at intel.com
Tue Jan 3 06:29:35 UTC 2023
Since it fixes a critical boot hang issue, I created PR for push @ https://github.com/tianocore/edk2/pull/3845
I removed following confusing comments.
+ // Only disable/enable WP when Cr0.Bits.WP has been set to 1.
> -----Original Message-----
> From: Tan, Dun <dun.tan at intel.com>
> Sent: Tuesday, January 3, 2023 1:56 PM
> To: devel at edk2.groups.io
> Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Kumar, Rahul R <rahul.r.kumar at intel.com>
> Subject: [Patch V4] UefiCpuPkg/PiSmmCpuDxeSmm:Fix PF issue caused by smm page table code
>
> When setting new page table pool to RO, only disable/enable WP when
> Cr0.WP has been set to 1 to fix potential PF caused by b822be1a20
> (UefiCpuPkg/PiSmmCpuDxeSmm: Introduce page table pool mechanism).
> With previous code, if someone want to modify the page table and
> Cr0.WP has been cleared before modify page table, Cr0.WP may be set
> to 1 again since new pool may be generated during this process
> Then PF fault may happens.
>
> Signed-off-by: Dun Tan <dun.tan at intel.com>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Rahul Kumar <rahul1.kumar at intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 40 ++++++++++++++++++++++++++++------------
> 1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 4bb23f6920..bab7f1887b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -67,8 +67,10 @@ InitializePageTablePool (
> IN UINTN PoolPages
> )
> {
> - VOID *Buffer;
> - BOOLEAN CetEnabled;
> + VOID *Buffer;
> + BOOLEAN CetEnabled;
> + BOOLEAN WpEnabled;
> + IA32_CR0 Cr0;
>
> //
> // Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page for
> @@ -106,21 +108,35 @@ InitializePageTablePool (
> //
> if (mIsReadOnlyPageTable) {
> CetEnabled = ((AsmReadCr4 () & CR4_CET_ENABLE) != 0) ? TRUE : FALSE;
> - if (CetEnabled) {
> + Cr0.UintN = AsmReadCr0 ();
> + WpEnabled = (Cr0.Bits.WP != 0) ? TRUE : FALSE;
> + if (WpEnabled) {
> + if (CetEnabled) {
> + //
> + // CET must be disabled if WP is disabled. Disable CET before clearing CR0.WP.
> + //
> + DisableCet ();
> + }
> +
> //
> - // CET must be disabled if WP is disabled.
> + // Only disable/enable WP when Cr0.Bits.WP has been set to 1.
> //
> - DisableCet ();
> + Cr0.Bits.WP = 0;
> + AsmWriteCr0 (Cr0.UintN);
> }
>
> - AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
> SmmSetMemoryAttributes ((EFI_PHYSICAL_ADDRESS)(UINTN)Buffer, EFI_PAGES_TO_SIZE (PoolPages),
> EFI_MEMORY_RO);
> - AsmWriteCr0 (AsmReadCr0 () | CR0_WP);
> - if (CetEnabled) {
> - //
> - // re-enable CET.
> - //
> - EnableCet ();
> + if (WpEnabled) {
> + Cr0.UintN = AsmReadCr0 ();
> + Cr0.Bits.WP = 1;
> + AsmWriteCr0 (Cr0.UintN);
> +
> + if (CetEnabled) {
> + //
> + // re-enable CET.
> + //
> + EnableCet ();
> + }
> }
> }
>
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97860): https://edk2.groups.io/g/devel/message/97860
Mute This Topic: https://groups.io/mt/96023891/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list