[edk2-devel] [Patch V4 04/15] MdeModulePkg: Remove RO and NX protection when unset guard page

duntan posted 15 patches 1 year, 3 months ago
There is a newer version of this series
[edk2-devel] [Patch V4 04/15] MdeModulePkg: Remove RO and NX protection when unset guard page
Posted by duntan 1 year, 3 months ago
Remove RO and NX protection when unset guard page.
When UnsetGuardPage(), remove all the memory attribute protection
for guarded page.

Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
index 8f3bab6fee..7daeeccf13 100644
--- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
+++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
@@ -553,7 +553,7 @@ UnsetGuardPage (
                                          mSmmMemoryAttribute,
                                          BaseAddress,
                                          EFI_PAGE_SIZE,
-                                         EFI_MEMORY_RP
+                                         EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
                                          );
     ASSERT_EFI_ERROR (Status);
     mOnGuarding = FALSE;
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104924): https://edk2.groups.io/g/devel/message/104924
Mute This Topic: https://groups.io/mt/98922929/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V4 04/15] MdeModulePkg: Remove RO and NX protection when unset guard page
Posted by Kun Qin 1 year, 3 months ago
Hi Dun,

I might have missed the context, but could you please explain why we 
need to clear "EFI_MEMORY_XP"?

It is understandable that you would like to clear RO. But would it make 
more sense to clear XP only when needed (i.e. code page allocation)?

Thanks,
Kun

On 5/16/2023 2:59 AM, duntan wrote:
> Remove RO and NX protection when unset guard page.
> When UnsetGuardPage(), remove all the memory attribute protection
> for guarded page.
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
>   MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index 8f3bab6fee..7daeeccf13 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -553,7 +553,7 @@ UnsetGuardPage (
>                                            mSmmMemoryAttribute,
>                                            BaseAddress,
>                                            EFI_PAGE_SIZE,
> -                                         EFI_MEMORY_RP
> +                                         EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
>                                            );
>       ASSERT_EFI_ERROR (Status);
>       mOnGuarding = FALSE;


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104951): https://edk2.groups.io/g/devel/message/104951
Mute This Topic: https://groups.io/mt/98922929/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch V4 04/15] MdeModulePkg: Remove RO and NX protection when unset guard page
Posted by duntan 1 year, 3 months ago
Hi Kun,

When we unset a guarded page to unguarded, from page table perspective, it's to set a non-present page to present.
I think it's reasonable that we should specific memory attribute to appropriate value when map non-present range to present. In smm initial page table, free page is set to present, writable and executable(with Present bit(bit0) and R/W bit(bit1) set to 1, XD(bit 63) set to 0). So that's why I also clear xp.

Thanks,
Dun

-----Original Message-----
From: Kun Qin <kuqin12@gmail.com> 
Sent: Wednesday, May 17, 2023 3:05 AM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
Subject: Re: [edk2-devel] [Patch V4 04/15] MdeModulePkg: Remove RO and NX protection when unset guard page

Hi Dun,

I might have missed the context, but could you please explain why we need to clear "EFI_MEMORY_XP"?

It is understandable that you would like to clear RO. But would it make more sense to clear XP only when needed (i.e. code page allocation)?

Thanks,
Kun

On 5/16/2023 2:59 AM, duntan wrote:
> Remove RO and NX protection when unset guard page.
> When UnsetGuardPage(), remove all the memory attribute protection for 
> guarded page.
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> ---
>   MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c 
> b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index 8f3bab6fee..7daeeccf13 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -553,7 +553,7 @@ UnsetGuardPage (
>                                            mSmmMemoryAttribute,
>                                            BaseAddress,
>                                            EFI_PAGE_SIZE,
> -                                         EFI_MEMORY_RP
> +                                         
> + EFI_MEMORY_RP|EFI_MEMORY_RO|EFI_MEMORY_XP
>                                            );
>       ASSERT_EFI_ERROR (Status);
>       mOnGuarding = FALSE;


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