[Patch V2 0/2] Remove clearing CR0.WP when protecting pagetable

[Patch V2 0/2] Remove clearing CR0.WP when protecting pagetable

[duntan]

Splited previos patch to two patches for different pkg.

Dun Tan (2):
  MdeModulePkg/DxeIpl: Remove clearing CR0.WP when protecting pagetable
  UefiPayloadPkg: Remove clearing CR0.WP when protecting pagetable

 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c    | 7 +------
 UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c | 7 +------
 2 files changed, 2 insertions(+), 12 deletions(-)

-- 
2.31.1.windows.1

[Patch V2 1/2] MdeModulePkg/DxeIpl: Remove clearing CR0.WP when protecting pagetable

[duntan]

Remove clearing CR0.WP when marking the memory used for page table
as read-only in the page table itself created by DxeIpl. This page
table address is written to Cr3 after these protection steps. Till
this, the memory used for page table is always RW.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
---
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index a451ca1604..18b121d768 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -626,12 +626,7 @@ EnablePageTableProtection (
   }
 
   //
-  // Disable write protection, because we need to mark page table to be write
-  // protected.
-  //
-  AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
-
-  //
+  // No need to clear CR0.WP since PageTableBase has't been written to CR3 yet.
   // SetPageTablePoolReadOnly might update mPageTablePool. It's safer to
   // remember original one in advance.
   //
-- 
2.31.1.windows.1

Re: [Patch V2 1/2] MdeModulePkg/DxeIpl: Remove clearing CR0.WP when protecting pagetable

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Wednesday, August 10, 2022 1:34 PM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>
> Subject: [Patch V2 1/2] MdeModulePkg/DxeIpl: Remove clearing CR0.WP
> when protecting pagetable
> 
> Remove clearing CR0.WP when marking the memory used for page table
> as read-only in the page table itself created by DxeIpl. This page
> table address is written to Cr3 after these protection steps. Till
> this, the memory used for page table is always RW.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> ---
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> index a451ca1604..18b121d768 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> @@ -626,12 +626,7 @@ EnablePageTableProtection (
>    }
> 
>    //
> -  // Disable write protection, because we need to mark page table to be
> write
> -  // protected.
> -  //
> -  AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
> -
> -  //
> +  // No need to clear CR0.WP since PageTableBase has't been written to CR3
> yet.
>    // SetPageTablePoolReadOnly might update mPageTablePool. It's safer to
>    // remember original one in advance.
>    //
> --
> 2.31.1.windows.1

[Patch V2 2/2] UefiPayloadPkg: Remove clearing CR0.WP when protecting pagetable

[duntan]

Remove clearing CR0.WP when marking the memory used for page table
as read-only in the page table itself created by UefiPayloadEntry.
This page table address is written to Cr3 after these protection
steps. Till this, the memory used for page table is always RW.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
---
 UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
index 74b667a62a..a586941352 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
@@ -622,12 +622,7 @@ EnablePageTableProtection (
   }
 
   //
-  // Disable write protection, because we need to mark page table to be write
-  // protected.
-  //
-  AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
-
-  //
+  // No need to clear CR0.WP since PageTableBase has't been written to CR3 yet.
   // SetPageTablePoolReadOnly might update mPageTablePool. It's safer to
   // remember original one in advance.
   //
-- 
2.31.1.windows.1

Re: [Patch V2 2/2] UefiPayloadPkg: Remove clearing CR0.WP when protecting pagetable

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Wednesday, August 10, 2022 1:34 PM
> To: devel@edk2.groups.io
> Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Maurice
> Ma <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>;
> Rhodes, Sean <sean@starlabs.systems>
> Subject: [Patch V2 2/2] UefiPayloadPkg: Remove clearing CR0.WP when
> protecting pagetable
> 
> Remove clearing CR0.WP when marking the memory used for page table
> as read-only in the page table itself created by UefiPayloadEntry.
> This page table address is written to Cr3 after these protection
> steps. Till this, the memory used for page table is always RW.
> 
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> Cc: Sean Rhodes <sean@starlabs.systems>
> ---
>  UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
> b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
> index 74b667a62a..a586941352 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
> +++ b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
> @@ -622,12 +622,7 @@ EnablePageTableProtection (
>    }
> 
>    //
> -  // Disable write protection, because we need to mark page table to be
> write
> -  // protected.
> -  //
> -  AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
> -
> -  //
> +  // No need to clear CR0.WP since PageTableBase has't been written to CR3
> yet.
>    // SetPageTablePoolReadOnly might update mPageTablePool. It's safer to
>    // remember original one in advance.
>    //
> --
> 2.31.1.windows.1

Re: [Patch V2 2/2] UefiPayloadPkg: Remove clearing CR0.WP when protecting pagetable

[Guo Dong]

Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: Tan, Dun <dun.tan@intel.com> 
Sent: Tuesday, August 9, 2022 10:34 PM
To: devel@edk2.groups.io
Cc: Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Maurice Ma <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>; Rhodes, Sean <sean@starlabs.systems>
Subject: [Patch V2 2/2] UefiPayloadPkg: Remove clearing CR0.WP when protecting pagetable

Remove clearing CR0.WP when marking the memory used for page table as read-only in the page table itself created by UefiPayloadEntry.
This page table address is written to Cr3 after these protection steps. Till this, the memory used for page table is always RW.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
---
 UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
index 74b667a62a..a586941352 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
@@ -622,12 +622,7 @@ EnablePageTableProtection (
   }
 
   //
-  // Disable write protection, because we need to mark page table to be write
-  // protected.
-  //
-  AsmWriteCr0 (AsmReadCr0 () & ~CR0_WP);
-
-  //
+  // No need to clear CR0.WP since PageTableBase has't been written to CR3 yet.
   // SetPageTablePoolReadOnly might update mPageTablePool. It's safer to
   // remember original one in advance.
   //
--
2.31.1.windows.1

Re: [edk2-devel] [Patch V2 0/2] Remove clearing CR0.WP when protecting pagetable

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of duntan
> Sent: Wednesday, August 10, 2022 1:34 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [Patch V2 0/2] Remove clearing CR0.WP when
> protecting pagetable
> 
> Splited previos patch to two patches for different pkg.
> 
> Dun Tan (2):
>   MdeModulePkg/DxeIpl: Remove clearing CR0.WP when protecting
> pagetable
>   UefiPayloadPkg: Remove clearing CR0.WP when protecting pagetable
> 
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c    | 7 +------
>  UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c | 7 +------
>  2 files changed, 2 insertions(+), 12 deletions(-)
> 
> --
> 2.31.1.windows.1
> 
> 
> 
> 
>