[edk2-devel] [PATCH v2] UefiPayloadPkg: Always split page table entry to 4K if it covers stack.

Zhiguang Liu posted 1 patch 1 year, 9 months ago
Failed in applying to current master (apply log)
UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
[edk2-devel] [PATCH v2] UefiPayloadPkg: Always split page table entry to 4K if it covers stack.
Posted by Zhiguang Liu 1 year, 9 months ago
We observed page fault in the following situation:
1.PayloadEntry uses 2M entry in page table to cover DXE stack range.
2.In DXE phase, image protection code needs to mark some sub-range in
this 2M entry as readonly. So the the 2M page table entry is split to
512 4K entries, and some of the entries are marked as readonly.
(the entries covering stack still remain R/W)
3.Page fault exception happens when trying to access stack.

Always split the page table entry to 4K if it covers stack to avoid this
issue.
More discussion about this issue can be seen at below link
https://edk2.groups.io/g/devel/topic/91446026

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>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
index ac0d58e685..74b667a62a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
@@ -218,16 +218,8 @@ ToSplitPageTable (
     return TRUE;
   }
 
-  if (PcdGetBool (PcdCpuStackGuard)) {
-    if ((StackBase >= Address) && (StackBase < (Address + Size))) {
-      return TRUE;
-    }
-  }
-
-  if (PcdGetBool (PcdSetNxForStack)) {
-    if ((Address < StackBase + StackSize) && ((Address + Size) > StackBase)) {
-      return TRUE;
-    }
+  if ((Address < StackBase + StackSize) && ((Address + Size) > StackBase)) {
+    return TRUE;
   }
 
   if (GhcbBase != 0) {
-- 
2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90572): https://edk2.groups.io/g/devel/message/90572
Mute This Topic: https://groups.io/mt/91817347/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] UefiPayloadPkg: Always split page table entry to 4K if it covers stack.
Posted by Gerd Hoffmann 1 year, 9 months ago
On Fri, Jun 17, 2022 at 04:28:03PM +0800, Zhiguang Liu wrote:
> We observed page fault in the following situation:
> 1.PayloadEntry uses 2M entry in page table to cover DXE stack range.
> 2.In DXE phase, image protection code needs to mark some sub-range in
> this 2M entry as readonly. So the the 2M page table entry is split to
> 512 4K entries, and some of the entries are marked as readonly.
> (the entries covering stack still remain R/W)
> 3.Page fault exception happens when trying to access stack.
> 
> Always split the page table entry to 4K if it covers stack to avoid this
> issue.
> More discussion about this issue can be seen at below link
> https://edk2.groups.io/g/devel/topic/91446026

Acked-by: Gerd Hoffmann <kraxel@redhat.com>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90624): https://edk2.groups.io/g/devel/message/90624
Mute This Topic: https://groups.io/mt/91817347/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v2] UefiPayloadPkg: Always split page table entry to 4K if it covers stack.
Posted by Ni, Ray 1 year, 9 months ago
Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Liu, Zhiguang <zhiguang.liu@intel.com>
> Sent: Friday, June 17, 2022 4:28 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; 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>; Gerd Hoffmann
> <kraxel@redhat.com>
> Subject: [PATCH v2] UefiPayloadPkg: Always split page table entry to 4K if it
> covers stack.
> 
> We observed page fault in the following situation:
> 1.PayloadEntry uses 2M entry in page table to cover DXE stack range.
> 2.In DXE phase, image protection code needs to mark some sub-range in
> this 2M entry as readonly. So the the 2M page table entry is split to
> 512 4K entries, and some of the entries are marked as readonly.
> (the entries covering stack still remain R/W)
> 3.Page fault exception happens when trying to access stack.
> 
> Always split the page table entry to 4K if it covers stack to avoid this
> issue.
> More discussion about this issue can be seen at below link
> https://edk2.groups.io/g/devel/topic/91446026
> 
> 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>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
>  UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
> b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
> index ac0d58e685..74b667a62a 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
> +++ b/UefiPayloadPkg/UefiPayloadEntry/X64/VirtualMemory.c
> @@ -218,16 +218,8 @@ ToSplitPageTable (
>      return TRUE;
>    }
> 
> -  if (PcdGetBool (PcdCpuStackGuard)) {
> -    if ((StackBase >= Address) && (StackBase < (Address + Size))) {
> -      return TRUE;
> -    }
> -  }
> -
> -  if (PcdGetBool (PcdSetNxForStack)) {
> -    if ((Address < StackBase + StackSize) && ((Address + Size) > StackBase)) {
> -      return TRUE;
> -    }
> +  if ((Address < StackBase + StackSize) && ((Address + Size) > StackBase)) {
> +    return TRUE;
>    }
> 
>    if (GhcbBase != 0) {
> --
> 2.16.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#90607): https://edk2.groups.io/g/devel/message/90607
Mute This Topic: https://groups.io/mt/91817347/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-