* [PATCH 0/2] Resolve feature conflict between NX and Stack guard
@ 2018-03-06 12:11 Hao Wu
2018-03-06 12:11 ` [PATCH 1/2] MdeModulePkg/Core: Refine handling NULL detection in NX setting Hao Wu
2018-03-06 12:11 ` [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard Hao Wu
0 siblings, 2 replies; 8+ messages in thread
From: Hao Wu @ 2018-03-06 12:11 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Jian J Wang, Star Zeng, Eric Dong, Jiewen Yao, Ruiyu Ni
If enabled, NX memory protection feature will mark some types of active
memory as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the
stack guard feature is also enabled.
The series will override the attributes setting to the first page of the
stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard
feature is enabled.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Hao Wu (2):
MdeModulePkg/Core: Refine handling NULL detection in NX setting
MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 81 +++++++++++++++++++++++----
2 files changed, 74 insertions(+), 11 deletions(-)
--
2.12.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] MdeModulePkg/Core: Refine handling NULL detection in NX setting
2018-03-06 12:11 [PATCH 0/2] Resolve feature conflict between NX and Stack guard Hao Wu
@ 2018-03-06 12:11 ` Hao Wu
2018-03-06 13:05 ` Yao, Jiewen
2018-03-06 12:11 ` [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard Hao Wu
1 sibling, 1 reply; 8+ messages in thread
From: Hao Wu @ 2018-03-06 12:11 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Jian J Wang, Star Zeng, Eric Dong, Jiewen Yao, Ruiyu Ni
The commit rewrites the logic in function
InitializeDxeNxMemoryProtectionPolicy() for handling the first page
(page 0) when NULL pointer detection feature is enabled.
Instead of skip setting the page 0, the codes will now override the
attribute setting of page 0 by adding the 'EFI_MEMORY_RP' attribute.
The purpose is to make it easy for other special handlings of pages
(e.g. the first page of the stack when stack guard feature is enabled).
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@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: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 455ed35f9a..a2ea445eef 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -19,7 +19,7 @@
Once the image is unloaded, the protection is removed automatically.
-Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -846,23 +846,23 @@ InitializeDxeNxMemoryProtectionPolicy (
Attributes = GetPermissionAttributeForMemoryType (MemoryMapEntry->Type);
if (Attributes != 0) {
+ SetUefiImageMemoryAttributes (
+ MemoryMapEntry->PhysicalStart,
+ LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT),
+ Attributes);
+
if (MemoryMapEntry->PhysicalStart == 0 &&
PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0) {
ASSERT (MemoryMapEntry->NumberOfPages > 0);
//
- // Skip page 0 if NULL pointer detection is enabled to avoid attributes
- // overwritten.
+ // Add EFI_MEMORY_RP attribute for page 0 if NULL pointer detection is
+ // enabled.
//
SetUefiImageMemoryAttributes (
- MemoryMapEntry->PhysicalStart + EFI_PAGE_SIZE,
- LShiftU64 (MemoryMapEntry->NumberOfPages - 1, EFI_PAGE_SHIFT),
- Attributes);
- } else {
- SetUefiImageMemoryAttributes (
- MemoryMapEntry->PhysicalStart,
- LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT),
- Attributes);
+ 0,
+ EFI_PAGES_TO_SIZE (1),
+ EFI_MEMORY_RP | Attributes);
}
}
MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
2018-03-06 12:11 [PATCH 0/2] Resolve feature conflict between NX and Stack guard Hao Wu
2018-03-06 12:11 ` [PATCH 1/2] MdeModulePkg/Core: Refine handling NULL detection in NX setting Hao Wu
@ 2018-03-06 12:11 ` Hao Wu
2018-03-06 12:16 ` Zeng, Star
1 sibling, 1 reply; 8+ messages in thread
From: Hao Wu @ 2018-03-06 12:11 UTC (permalink / raw)
To: edk2-devel
Cc: Hao Wu, Jian J Wang, Star Zeng, Eric Dong, Jiewen Yao, Ruiyu Ni
If enabled, NX memory protection feature will mark some types of active
memory as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the
stack guard feature is also enabled.
The solution is to override the attributes setting to the first page of
the stack by adding back the 'EFI_MEMORY_RP' attribute when the stack
guard feature is enabled.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@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: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61 +++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 7334780326..d2e7360ed4 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -3,7 +3,7 @@
#
# It provides an implementation of DXE Core that is compliant with DXE CIS.
#
-# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -130,6 +130,7 @@
gEfiPropertiesTableGuid ## SOMETIMES_PRODUCES ## SystemTable
gEfiMemoryAttributesTableGuid ## SOMETIMES_PRODUCES ## SystemTable
gEfiEndOfDxeEventGroupGuid ## SOMETIMES_CONSUMES ## Event
+ gEfiHobMemoryAllocStackGuid ## SOMETIMES_CONSUMES ## SystemTable
[Ppis]
gEfiVectorHandoffInfoPpiGuid ## UNDEFINED # HOB
@@ -198,6 +199,7 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
# [Hob]
# RESOURCE_DESCRIPTOR ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index a2ea445eef..a6de22d3af 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -801,6 +801,11 @@ InitializeDxeNxMemoryProtectionPolicy (
UINT64 Attributes;
LIST_ENTRY *Link;
EFI_GCD_MAP_ENTRY *Entry;
+ VOID *HobList;
+ EFI_PEI_HOB_POINTERS Hob;
+ EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
+ EFI_PHYSICAL_ADDRESS StackBase;
+ BOOLEAN StackBaseFound;
//
// Get the EFI memory map.
@@ -832,6 +837,45 @@ InitializeDxeNxMemoryProtectionPolicy (
} while (Status == EFI_BUFFER_TOO_SMALL);
ASSERT_EFI_ERROR (Status);
+ StackBase = 0;
+ StackBaseFound = FALSE;
+ if (PcdGetBool (PcdCpuStackGuard)) {
+ //
+ // Get the base of stack from Hob.
+ //
+ Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &HobList);
+ if (!EFI_ERROR (Status)) {
+ for (Hob.Raw = HobList; !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
+ if (GET_HOB_TYPE(Hob) == EFI_HOB_TYPE_MEMORY_ALLOCATION) {
+ MemoryHob = Hob.MemoryAllocation;
+ if (CompareGuid(&gEfiHobMemoryAllocStackGuid, &MemoryHob->AllocDescriptor.Name)) {
+ DEBUG ((
+ DEBUG_INFO,
+ "%a: StackBase = 0x%016lx StackSize = 0x%016lx\n",
+ __FUNCTION__,
+ MemoryHob->AllocDescriptor.MemoryBaseAddress,
+ MemoryHob->AllocDescriptor.MemoryLength
+ ));
+
+ StackBase = MemoryHob->AllocDescriptor.MemoryBaseAddress;
+ //
+ // Ensure the base of the stack is page-size aligned.
+ //
+ ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
+ StackBaseFound = TRUE;
+ break;
+ }
+ }
+ }
+ }
+
+ //
+ // Ensure the base of stack can be found from Hob when stack guard is
+ // enabled.
+ //
+ ASSERT (StackBaseFound);
+ }
+
DEBUG ((
DEBUG_INFO,
"%a: applying strict permissions to active memory regions\n",
@@ -864,6 +908,23 @@ InitializeDxeNxMemoryProtectionPolicy (
EFI_PAGES_TO_SIZE (1),
EFI_MEMORY_RP | Attributes);
}
+
+ if (StackBaseFound &&
+ (StackBase >= MemoryMapEntry->PhysicalStart &&
+ StackBase < MemoryMapEntry->PhysicalStart +
+ LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT)) &&
+ PcdGetBool (PcdCpuStackGuard)) {
+
+ //
+ // Add EFI_MEMORY_RP attribute for the first page of the stack if stack
+ // guard is enabled.
+ //
+ SetUefiImageMemoryAttributes (
+ StackBase,
+ EFI_PAGES_TO_SIZE (1),
+ EFI_MEMORY_RP | Attributes);
+ }
+
}
MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
}
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
2018-03-06 12:11 ` [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard Hao Wu
@ 2018-03-06 12:16 ` Zeng, Star
2018-03-06 13:04 ` Yao, Jiewen
0 siblings, 1 reply; 8+ messages in thread
From: Zeng, Star @ 2018-03-06 12:16 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Cc: Wang, Jian J, Dong, Eric, Yao, Jiewen, Ni, Ruiyu, Zeng, Star
A quick minor comment.
GetHobList() could be used instead of EfiGetSystemConfigurationTable (&gEfiHobListGuid, &HobList).
Thanks,
Star
-----Original Message-----
From: Wu, Hao A
Sent: Tuesday, March 6, 2018 8:11 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
If enabled, NX memory protection feature will mark some types of active memory as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the stack guard feature is also enabled.
The solution is to override the attributes setting to the first page of the stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard feature is enabled.
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@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: Hao Wu <hao.a.wu@intel.com>
---
MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +-
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61 +++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 7334780326..d2e7360ed4 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -3,7 +3,7 @@
#
# It provides an implementation of DXE Core that is compliant with DXE CIS.
#
-# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2018, Intel Corporation. All rights
+reserved.<BR>
# This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License # which accompanies this distribution. The full text of the license may be found at @@ -130,6 +130,7 @@
gEfiPropertiesTableGuid ## SOMETIMES_PRODUCES ## SystemTable
gEfiMemoryAttributesTableGuid ## SOMETIMES_PRODUCES ## SystemTable
gEfiEndOfDxeEventGroupGuid ## SOMETIMES_CONSUMES ## Event
+ gEfiHobMemoryAllocStackGuid ## SOMETIMES_CONSUMES ## SystemTable
[Ppis]
gEfiVectorHandoffInfoPpiGuid ## UNDEFINED # HOB
@@ -198,6 +199,7 @@
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES
# [Hob]
# RESOURCE_DESCRIPTOR ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index a2ea445eef..a6de22d3af 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -801,6 +801,11 @@ InitializeDxeNxMemoryProtectionPolicy (
UINT64 Attributes;
LIST_ENTRY *Link;
EFI_GCD_MAP_ENTRY *Entry;
+ VOID *HobList;
+ EFI_PEI_HOB_POINTERS Hob;
+ EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
+ EFI_PHYSICAL_ADDRESS StackBase;
+ BOOLEAN StackBaseFound;
//
// Get the EFI memory map.
@@ -832,6 +837,45 @@ InitializeDxeNxMemoryProtectionPolicy (
} while (Status == EFI_BUFFER_TOO_SMALL);
ASSERT_EFI_ERROR (Status);
+ StackBase = 0;
+ StackBaseFound = FALSE;
+ if (PcdGetBool (PcdCpuStackGuard)) {
+ //
+ // Get the base of stack from Hob.
+ //
+ Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &HobList);
+ if (!EFI_ERROR (Status)) {
+ for (Hob.Raw = HobList; !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB (Hob)) {
+ if (GET_HOB_TYPE(Hob) == EFI_HOB_TYPE_MEMORY_ALLOCATION) {
+ MemoryHob = Hob.MemoryAllocation;
+ if (CompareGuid(&gEfiHobMemoryAllocStackGuid, &MemoryHob->AllocDescriptor.Name)) {
+ DEBUG ((
+ DEBUG_INFO,
+ "%a: StackBase = 0x%016lx StackSize = 0x%016lx\n",
+ __FUNCTION__,
+ MemoryHob->AllocDescriptor.MemoryBaseAddress,
+ MemoryHob->AllocDescriptor.MemoryLength
+ ));
+
+ StackBase = MemoryHob->AllocDescriptor.MemoryBaseAddress;
+ //
+ // Ensure the base of the stack is page-size aligned.
+ //
+ ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
+ StackBaseFound = TRUE;
+ break;
+ }
+ }
+ }
+ }
+
+ //
+ // Ensure the base of stack can be found from Hob when stack guard is
+ // enabled.
+ //
+ ASSERT (StackBaseFound);
+ }
+
DEBUG ((
DEBUG_INFO,
"%a: applying strict permissions to active memory regions\n", @@ -864,6 +908,23 @@ InitializeDxeNxMemoryProtectionPolicy (
EFI_PAGES_TO_SIZE (1),
EFI_MEMORY_RP | Attributes);
}
+
+ if (StackBaseFound &&
+ (StackBase >= MemoryMapEntry->PhysicalStart &&
+ StackBase < MemoryMapEntry->PhysicalStart +
+ LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT)) &&
+ PcdGetBool (PcdCpuStackGuard)) {
+
+ //
+ // Add EFI_MEMORY_RP attribute for the first page of the stack if stack
+ // guard is enabled.
+ //
+ SetUefiImageMemoryAttributes (
+ StackBase,
+ EFI_PAGES_TO_SIZE (1),
+ EFI_MEMORY_RP | Attributes);
+ }
+
}
MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
}
--
2.12.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
2018-03-06 12:16 ` Zeng, Star
@ 2018-03-06 13:04 ` Yao, Jiewen
2018-03-06 13:10 ` Yao, Jiewen
0 siblings, 1 reply; 8+ messages in thread
From: Yao, Jiewen @ 2018-03-06 13:04 UTC (permalink / raw)
To: Zeng, Star, Wu, Hao A, edk2-devel@lists.01.org
Cc: Wang, Jian J, Dong, Eric, Ni, Ruiyu
Agree.
With this update, reviewed-by: Jiewen.yao@intel.com
> -----Original Message-----
> From: Zeng, Star
> Sent: Tuesday, March 6, 2018 8:16 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX
> and Stack guard
>
> A quick minor comment.
> GetHobList() could be used instead of EfiGetSystemConfigurationTable
> (&gEfiHobListGuid, &HobList).
>
> Thanks,
> Star
> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, March 6, 2018 8:11 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and
> Stack guard
>
> If enabled, NX memory protection feature will mark some types of active
> memory as NX (non-executable), which includes the first page of the stack.
> This will overwrite the attributes of the first page of the stack if the stack guard
> feature is also enabled.
>
> The solution is to override the attributes setting to the first page of the stack by
> adding back the 'EFI_MEMORY_RP' attribute when the stack guard feature is
> enabled.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@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: Hao Wu <hao.a.wu@intel.com>
> ---
> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +-
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61
> +++++++++++++++++++++++++++
> 2 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 7334780326..d2e7360ed4 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -3,7 +3,7 @@
> #
> # It provides an implementation of DXE Core that is compliant with DXE CIS.
> #
> -# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2006 - 2018, Intel Corporation. All rights
> +reserved.<BR>
> # This program and the accompanying materials # are licensed and made
> available under the terms and conditions of the BSD License # which
> accompanies this distribution. The full text of the license may be found at @@
> -130,6 +130,7 @@
> gEfiPropertiesTableGuid ##
> SOMETIMES_PRODUCES ## SystemTable
> gEfiMemoryAttributesTableGuid ##
> SOMETIMES_PRODUCES ## SystemTable
> gEfiEndOfDxeEventGroupGuid ##
> SOMETIMES_CONSUMES ## Event
> + gEfiHobMemoryAllocStackGuid ##
> SOMETIMES_CONSUMES ## SystemTable
>
> [Ppis]
> gEfiVectorHandoffInfoPpiGuid ## UNDEFINED # HOB
> @@ -198,6 +199,7 @@
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType
> ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
> ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
> ## CONSUMES
>
> # [Hob]
> # RESOURCE_DESCRIPTOR ## CONSUMES
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index a2ea445eef..a6de22d3af 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -801,6 +801,11 @@ InitializeDxeNxMemoryProtectionPolicy (
> UINT64 Attributes;
> LIST_ENTRY *Link;
> EFI_GCD_MAP_ENTRY *Entry;
> + VOID *HobList;
> + EFI_PEI_HOB_POINTERS Hob;
> + EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
> + EFI_PHYSICAL_ADDRESS StackBase;
> + BOOLEAN StackBaseFound;
>
> //
> // Get the EFI memory map.
> @@ -832,6 +837,45 @@ InitializeDxeNxMemoryProtectionPolicy (
> } while (Status == EFI_BUFFER_TOO_SMALL);
> ASSERT_EFI_ERROR (Status);
>
> + StackBase = 0;
> + StackBaseFound = FALSE;
> + if (PcdGetBool (PcdCpuStackGuard)) {
> + //
> + // Get the base of stack from Hob.
> + //
> + Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &HobList);
> + if (!EFI_ERROR (Status)) {
> + for (Hob.Raw = HobList; !END_OF_HOB_LIST (Hob); Hob.Raw =
> GET_NEXT_HOB (Hob)) {
> + if (GET_HOB_TYPE(Hob) == EFI_HOB_TYPE_MEMORY_ALLOCATION)
> {
> + MemoryHob = Hob.MemoryAllocation;
> + if (CompareGuid(&gEfiHobMemoryAllocStackGuid,
> &MemoryHob->AllocDescriptor.Name)) {
> + DEBUG ((
> + DEBUG_INFO,
> + "%a: StackBase = 0x%016lx StackSize = 0x%016lx\n",
> + __FUNCTION__,
> + MemoryHob->AllocDescriptor.MemoryBaseAddress,
> + MemoryHob->AllocDescriptor.MemoryLength
> + ));
> +
> + StackBase = MemoryHob->AllocDescriptor.MemoryBaseAddress;
> + //
> + // Ensure the base of the stack is page-size aligned.
> + //
> + ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
> + StackBaseFound = TRUE;
> + break;
> + }
> + }
> + }
> + }
> +
> + //
> + // Ensure the base of stack can be found from Hob when stack guard is
> + // enabled.
> + //
> + ASSERT (StackBaseFound);
> + }
> +
> DEBUG ((
> DEBUG_INFO,
> "%a: applying strict permissions to active memory regions\n", @@ -864,6
> +908,23 @@ InitializeDxeNxMemoryProtectionPolicy (
> EFI_PAGES_TO_SIZE (1),
> EFI_MEMORY_RP | Attributes);
> }
> +
> + if (StackBaseFound &&
> + (StackBase >= MemoryMapEntry->PhysicalStart &&
> + StackBase < MemoryMapEntry->PhysicalStart +
> + LShiftU64 (MemoryMapEntry->NumberOfPages,
> EFI_PAGE_SHIFT)) &&
> + PcdGetBool (PcdCpuStackGuard)) {
> +
> + //
> + // Add EFI_MEMORY_RP attribute for the first page of the stack if
> stack
> + // guard is enabled.
> + //
> + SetUefiImageMemoryAttributes (
> + StackBase,
> + EFI_PAGES_TO_SIZE (1),
> + EFI_MEMORY_RP | Attributes);
> + }
> +
> }
> MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry,
> DescriptorSize);
> }
> --
> 2.12.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] MdeModulePkg/Core: Refine handling NULL detection in NX setting
2018-03-06 12:11 ` [PATCH 1/2] MdeModulePkg/Core: Refine handling NULL detection in NX setting Hao Wu
@ 2018-03-06 13:05 ` Yao, Jiewen
0 siblings, 0 replies; 8+ messages in thread
From: Yao, Jiewen @ 2018-03-06 13:05 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org
Cc: Wang, Jian J, Zeng, Star, Dong, Eric, Ni, Ruiyu
reviewed-by: Jiewen.yao@intel.com
> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, March 6, 2018 8:11 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH 1/2] MdeModulePkg/Core: Refine handling NULL detection in
> NX setting
>
> The commit rewrites the logic in function
> InitializeDxeNxMemoryProtectionPolicy() for handling the first page
> (page 0) when NULL pointer detection feature is enabled.
>
> Instead of skip setting the page 0, the codes will now override the
> attribute setting of page 0 by adding the 'EFI_MEMORY_RP' attribute.
>
> The purpose is to make it easy for other special handlings of pages
> (e.g. the first page of the stack when stack guard feature is enabled).
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@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: Hao Wu <hao.a.wu@intel.com>
> ---
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 22
> +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index 455ed35f9a..a2ea445eef 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -19,7 +19,7 @@
>
> Once the image is unloaded, the protection is removed automatically.
>
> -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be found
> at
> @@ -846,23 +846,23 @@ InitializeDxeNxMemoryProtectionPolicy (
>
> Attributes = GetPermissionAttributeForMemoryType
> (MemoryMapEntry->Type);
> if (Attributes != 0) {
> + SetUefiImageMemoryAttributes (
> + MemoryMapEntry->PhysicalStart,
> + LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT),
> + Attributes);
> +
> if (MemoryMapEntry->PhysicalStart == 0 &&
> PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0) {
>
> ASSERT (MemoryMapEntry->NumberOfPages > 0);
> //
> - // Skip page 0 if NULL pointer detection is enabled to avoid attributes
> - // overwritten.
> + // Add EFI_MEMORY_RP attribute for page 0 if NULL pointer
> detection is
> + // enabled.
> //
> SetUefiImageMemoryAttributes (
> - MemoryMapEntry->PhysicalStart + EFI_PAGE_SIZE,
> - LShiftU64 (MemoryMapEntry->NumberOfPages - 1,
> EFI_PAGE_SHIFT),
> - Attributes);
> - } else {
> - SetUefiImageMemoryAttributes (
> - MemoryMapEntry->PhysicalStart,
> - LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT),
> - Attributes);
> + 0,
> + EFI_PAGES_TO_SIZE (1),
> + EFI_MEMORY_RP | Attributes);
> }
> }
> MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry,
> DescriptorSize);
> --
> 2.12.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
2018-03-06 13:04 ` Yao, Jiewen
@ 2018-03-06 13:10 ` Yao, Jiewen
2018-03-06 13:11 ` Wu, Hao A
0 siblings, 1 reply; 8+ messages in thread
From: Yao, Jiewen @ 2018-03-06 13:10 UTC (permalink / raw)
To: Yao, Jiewen, Zeng, Star, Wu, Hao A, edk2-devel@lists.01.org
Cc: Ni, Ruiyu, Dong, Eric
BTW: I don't think "StackBaseFound" is really needed.
We can use ASSERT (StackBase != 0); directly. :-)
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
> Jiewen
> Sent: Tuesday, March 6, 2018 9:05 PM
> To: Zeng, Star <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict
> between NX and Stack guard
>
> Agree.
>
> With this update, reviewed-by: Jiewen.yao@intel.com
>
>
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Tuesday, March 6, 2018 8:16 PM
> > To: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Dong, Eric <eric.dong@intel.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng,
> Star
> > <star.zeng@intel.com>
> > Subject: RE: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between
> NX
> > and Stack guard
> >
> > A quick minor comment.
> > GetHobList() could be used instead of EfiGetSystemConfigurationTable
> > (&gEfiHobListGuid, &HobList).
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Tuesday, March 6, 2018 8:11 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> > Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX
> and
> > Stack guard
> >
> > If enabled, NX memory protection feature will mark some types of active
> > memory as NX (non-executable), which includes the first page of the stack.
> > This will overwrite the attributes of the first page of the stack if the stack guard
> > feature is also enabled.
> >
> > The solution is to override the attributes setting to the first page of the stack by
> > adding back the 'EFI_MEMORY_RP' attribute when the stack guard feature is
> > enabled.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@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: Hao Wu <hao.a.wu@intel.com>
> > ---
> > MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +-
> > MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61
> > +++++++++++++++++++++++++++
> > 2 files changed, 64 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > index 7334780326..d2e7360ed4 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > @@ -3,7 +3,7 @@
> > #
> > # It provides an implementation of DXE Core that is compliant with DXE CIS.
> > #
> > -# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > +reserved.<BR>
> > # This program and the accompanying materials # are licensed and
> made
> > available under the terms and conditions of the BSD License # which
> > accompanies this distribution. The full text of the license may be found at
> @@
> > -130,6 +130,7 @@
> > gEfiPropertiesTableGuid ##
> > SOMETIMES_PRODUCES ## SystemTable
> > gEfiMemoryAttributesTableGuid ##
> > SOMETIMES_PRODUCES ## SystemTable
> > gEfiEndOfDxeEventGroupGuid ##
> > SOMETIMES_CONSUMES ## Event
> > + gEfiHobMemoryAllocStackGuid ##
> > SOMETIMES_CONSUMES ## SystemTable
> >
> > [Ppis]
> > gEfiVectorHandoffInfoPpiGuid ## UNDEFINED # HOB
> > @@ -198,6 +199,7 @@
> > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType
> > ## CONSUMES
> > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
> > ## CONSUMES
> > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> > ## CONSUMES
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
> > ## CONSUMES
> >
> > # [Hob]
> > # RESOURCE_DESCRIPTOR ## CONSUMES
> > diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > index a2ea445eef..a6de22d3af 100644
> > --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > @@ -801,6 +801,11 @@ InitializeDxeNxMemoryProtectionPolicy (
> > UINT64 Attributes;
> > LIST_ENTRY *Link;
> > EFI_GCD_MAP_ENTRY *Entry;
> > + VOID *HobList;
> > + EFI_PEI_HOB_POINTERS Hob;
> > + EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
> > + EFI_PHYSICAL_ADDRESS StackBase;
> > + BOOLEAN StackBaseFound;
> >
> > //
> > // Get the EFI memory map.
> > @@ -832,6 +837,45 @@ InitializeDxeNxMemoryProtectionPolicy (
> > } while (Status == EFI_BUFFER_TOO_SMALL);
> > ASSERT_EFI_ERROR (Status);
> >
> > + StackBase = 0;
> > + StackBaseFound = FALSE;
> > + if (PcdGetBool (PcdCpuStackGuard)) {
> > + //
> > + // Get the base of stack from Hob.
> > + //
> > + Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &HobList);
> > + if (!EFI_ERROR (Status)) {
> > + for (Hob.Raw = HobList; !END_OF_HOB_LIST (Hob); Hob.Raw =
> > GET_NEXT_HOB (Hob)) {
> > + if (GET_HOB_TYPE(Hob) ==
> EFI_HOB_TYPE_MEMORY_ALLOCATION)
> > {
> > + MemoryHob = Hob.MemoryAllocation;
> > + if (CompareGuid(&gEfiHobMemoryAllocStackGuid,
> > &MemoryHob->AllocDescriptor.Name)) {
> > + DEBUG ((
> > + DEBUG_INFO,
> > + "%a: StackBase = 0x%016lx StackSize = 0x%016lx\n",
> > + __FUNCTION__,
> > + MemoryHob->AllocDescriptor.MemoryBaseAddress,
> > + MemoryHob->AllocDescriptor.MemoryLength
> > + ));
> > +
> > + StackBase =
> MemoryHob->AllocDescriptor.MemoryBaseAddress;
> > + //
> > + // Ensure the base of the stack is page-size aligned.
> > + //
> > + ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
> > + StackBaseFound = TRUE;
> > + break;
> > + }
> > + }
> > + }
> > + }
> > +
> > + //
> > + // Ensure the base of stack can be found from Hob when stack guard is
> > + // enabled.
> > + //
> > + ASSERT (StackBaseFound);
> > + }
> > +
> > DEBUG ((
> > DEBUG_INFO,
> > "%a: applying strict permissions to active memory regions\n", @@
> -864,6
> > +908,23 @@ InitializeDxeNxMemoryProtectionPolicy (
> > EFI_PAGES_TO_SIZE (1),
> > EFI_MEMORY_RP | Attributes);
> > }
> > +
> > + if (StackBaseFound &&
> > + (StackBase >= MemoryMapEntry->PhysicalStart &&
> > + StackBase < MemoryMapEntry->PhysicalStart +
> > + LShiftU64 (MemoryMapEntry->NumberOfPages,
> > EFI_PAGE_SHIFT)) &&
> > + PcdGetBool (PcdCpuStackGuard)) {
> > +
> > + //
> > + // Add EFI_MEMORY_RP attribute for the first page of the stack if
> > stack
> > + // guard is enabled.
> > + //
> > + SetUefiImageMemoryAttributes (
> > + StackBase,
> > + EFI_PAGES_TO_SIZE (1),
> > + EFI_MEMORY_RP | Attributes);
> > + }
> > +
> > }
> > MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry,
> > DescriptorSize);
> > }
> > --
> > 2.12.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
2018-03-06 13:10 ` Yao, Jiewen
@ 2018-03-06 13:11 ` Wu, Hao A
0 siblings, 0 replies; 8+ messages in thread
From: Wu, Hao A @ 2018-03-06 13:11 UTC (permalink / raw)
To: Yao, Jiewen, Zeng, Star, edk2-devel@lists.01.org; +Cc: Ni, Ruiyu, Dong, Eric
Got it, I will send out a V2 of the series according to the comments.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, March 06, 2018 9:11 PM
> To: Yao, Jiewen; Zeng, Star; Wu, Hao A; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Dong, Eric
> Subject: RE: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX
> and Stack guard
>
> BTW: I don't think "StackBaseFound" is really needed.
>
> We can use ASSERT (StackBase != 0); directly. :-)
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
> > Jiewen
> > Sent: Tuesday, March 6, 2018 9:05 PM
> > To: Zeng, Star <star.zeng@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> > edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> > Subject: Re: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict
> > between NX and Stack guard
> >
> > Agree.
> >
> > With this update, reviewed-by: Jiewen.yao@intel.com
> >
> >
> > > -----Original Message-----
> > > From: Zeng, Star
> > > Sent: Tuesday, March 6, 2018 8:16 PM
> > > To: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Dong, Eric
> <eric.dong@intel.com>;
> > > Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng,
> > Star
> > > <star.zeng@intel.com>
> > > Subject: RE: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between
> > NX
> > > and Stack guard
> > >
> > > A quick minor comment.
> > > GetHobList() could be used instead of EfiGetSystemConfigurationTable
> > > (&gEfiHobListGuid, &HobList).
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: Wu, Hao A
> > > Sent: Tuesday, March 6, 2018 8:11 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>;
> > > Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>; Yao,
> > > Jiewen <jiewen.yao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > > Subject: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX
> > and
> > > Stack guard
> > >
> > > If enabled, NX memory protection feature will mark some types of active
> > > memory as NX (non-executable), which includes the first page of the stack.
> > > This will overwrite the attributes of the first page of the stack if the stack
> guard
> > > feature is also enabled.
> > >
> > > The solution is to override the attributes setting to the first page of the
> stack by
> > > adding back the 'EFI_MEMORY_RP' attribute when the stack guard feature
> is
> > > enabled.
> > >
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Eric Dong <eric.dong@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: Hao Wu <hao.a.wu@intel.com>
> > > ---
> > > MdeModulePkg/Core/Dxe/DxeMain.inf | 4 +-
> > > MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61
> > > +++++++++++++++++++++++++++
> > > 2 files changed, 64 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > index 7334780326..d2e7360ed4 100644
> > > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > @@ -3,7 +3,7 @@
> > > #
> > > # It provides an implementation of DXE Core that is compliant with DXE
> CIS.
> > > #
> > > -# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> > > +# Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > > +reserved.<BR>
> > > # This program and the accompanying materials # are licensed and
> > made
> > > available under the terms and conditions of the BSD License # which
> > > accompanies this distribution. The full text of the license may be found at
> > @@
> > > -130,6 +130,7 @@
> > > gEfiPropertiesTableGuid ##
> > > SOMETIMES_PRODUCES ## SystemTable
> > > gEfiMemoryAttributesTableGuid ##
> > > SOMETIMES_PRODUCES ## SystemTable
> > > gEfiEndOfDxeEventGroupGuid ##
> > > SOMETIMES_CONSUMES ## Event
> > > + gEfiHobMemoryAllocStackGuid ##
> > > SOMETIMES_CONSUMES ## SystemTable
> > >
> > > [Ppis]
> > > gEfiVectorHandoffInfoPpiGuid ## UNDEFINED # HOB
> > > @@ -198,6 +199,7 @@
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType
> > > ## CONSUMES
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
> > > ## CONSUMES
> > > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> > > ## CONSUMES
> > > + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
> > > ## CONSUMES
> > >
> > > # [Hob]
> > > # RESOURCE_DESCRIPTOR ## CONSUMES
> > > diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > > b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > > index a2ea445eef..a6de22d3af 100644
> > > --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > > +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > > @@ -801,6 +801,11 @@ InitializeDxeNxMemoryProtectionPolicy (
> > > UINT64 Attributes;
> > > LIST_ENTRY *Link;
> > > EFI_GCD_MAP_ENTRY *Entry;
> > > + VOID *HobList;
> > > + EFI_PEI_HOB_POINTERS Hob;
> > > + EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
> > > + EFI_PHYSICAL_ADDRESS StackBase;
> > > + BOOLEAN StackBaseFound;
> > >
> > > //
> > > // Get the EFI memory map.
> > > @@ -832,6 +837,45 @@ InitializeDxeNxMemoryProtectionPolicy (
> > > } while (Status == EFI_BUFFER_TOO_SMALL);
> > > ASSERT_EFI_ERROR (Status);
> > >
> > > + StackBase = 0;
> > > + StackBaseFound = FALSE;
> > > + if (PcdGetBool (PcdCpuStackGuard)) {
> > > + //
> > > + // Get the base of stack from Hob.
> > > + //
> > > + Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &HobList);
> > > + if (!EFI_ERROR (Status)) {
> > > + for (Hob.Raw = HobList; !END_OF_HOB_LIST (Hob); Hob.Raw =
> > > GET_NEXT_HOB (Hob)) {
> > > + if (GET_HOB_TYPE(Hob) ==
> > EFI_HOB_TYPE_MEMORY_ALLOCATION)
> > > {
> > > + MemoryHob = Hob.MemoryAllocation;
> > > + if (CompareGuid(&gEfiHobMemoryAllocStackGuid,
> > > &MemoryHob->AllocDescriptor.Name)) {
> > > + DEBUG ((
> > > + DEBUG_INFO,
> > > + "%a: StackBase = 0x%016lx StackSize = 0x%016lx\n",
> > > + __FUNCTION__,
> > > + MemoryHob->AllocDescriptor.MemoryBaseAddress,
> > > + MemoryHob->AllocDescriptor.MemoryLength
> > > + ));
> > > +
> > > + StackBase =
> > MemoryHob->AllocDescriptor.MemoryBaseAddress;
> > > + //
> > > + // Ensure the base of the stack is page-size aligned.
> > > + //
> > > + ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
> > > + StackBaseFound = TRUE;
> > > + break;
> > > + }
> > > + }
> > > + }
> > > + }
> > > +
> > > + //
> > > + // Ensure the base of stack can be found from Hob when stack guard is
> > > + // enabled.
> > > + //
> > > + ASSERT (StackBaseFound);
> > > + }
> > > +
> > > DEBUG ((
> > > DEBUG_INFO,
> > > "%a: applying strict permissions to active memory regions\n", @@
> > -864,6
> > > +908,23 @@ InitializeDxeNxMemoryProtectionPolicy (
> > > EFI_PAGES_TO_SIZE (1),
> > > EFI_MEMORY_RP | Attributes);
> > > }
> > > +
> > > + if (StackBaseFound &&
> > > + (StackBase >= MemoryMapEntry->PhysicalStart &&
> > > + StackBase < MemoryMapEntry->PhysicalStart +
> > > + LShiftU64 (MemoryMapEntry->NumberOfPages,
> > > EFI_PAGE_SHIFT)) &&
> > > + PcdGetBool (PcdCpuStackGuard)) {
> > > +
> > > + //
> > > + // Add EFI_MEMORY_RP attribute for the first page of the stack if
> > > stack
> > > + // guard is enabled.
> > > + //
> > > + SetUefiImageMemoryAttributes (
> > > + StackBase,
> > > + EFI_PAGES_TO_SIZE (1),
> > > + EFI_MEMORY_RP | Attributes);
> > > + }
> > > +
> > > }
> > > MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry,
> > > DescriptorSize);
> > > }
> > > --
> > > 2.12.0.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-03-06 13:05 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-06 12:11 [PATCH 0/2] Resolve feature conflict between NX and Stack guard Hao Wu
2018-03-06 12:11 ` [PATCH 1/2] MdeModulePkg/Core: Refine handling NULL detection in NX setting Hao Wu
2018-03-06 13:05 ` Yao, Jiewen
2018-03-06 12:11 ` [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard Hao Wu
2018-03-06 12:16 ` Zeng, Star
2018-03-06 13:04 ` Yao, Jiewen
2018-03-06 13:10 ` Yao, Jiewen
2018-03-06 13:11 ` Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox