1. Patchew
  2. EDK2
  3. [edk2] [PATCH 0/5] Implement heap guard feature
  4. View patch

[edk2] [PATCH 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection

Jian J Wang posted 5 patches 7 years, 2 months ago
There is a newer version of this series
[edk2] [PATCH 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
Posted by Jian J Wang 7 years, 2 months ago 
Heap guard feature will update page attributes frequently. The page table
should not set to be read-only if heap guard feature is enabled for SMM
mode. Otherwise this feature cannot work.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Ayellet Wolman <ayellet.wolman@intel.com>
Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c      | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 099792e6ce..644709650c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -159,6 +159,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable               ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable                   ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask    ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask               ## CONSUMES
 
 [Depex]
   gEfiMpServiceProtocolGuid
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 3dde80f9ba..4debce3a0f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -902,7 +902,7 @@ SetPageTableAttributes (
   BOOLEAN               IsSplitted;
   BOOLEAN               PageTableSplitted;
 
-  if (!mCpuSmmStaticPageTable) {
+  if (!mCpuSmmStaticPageTable || (PcdGet8 (PcdHeapGuardPropertyMask) & BIT3 | BIT2) != 0) {
     return ;
   }
 
-- 
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
Posted by Dong, Eric 7 years, 2 months ago 
Hi Jian,

> +  if (!mCpuSmmStaticPageTable || (PcdGet8 (PcdHeapGuardPropertyMask)
> &
> + BIT3 | BIT2) != 0) {

I think above code logic is not correct, the "&" will be handled before the "|" which is not an expected order, right?

Thanks,
Eric

> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, October 11, 2017 11:18 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> <ayellet.wolman@intel.com>
> Subject: [PATCH 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table
> protection
> 
> Heap guard feature will update page attributes frequently. The page table
> should not set to be read-only if heap guard feature is enabled for SMM
> mode. Otherwise this feature cannot work.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Ayellet Wolman <ayellet.wolman@intel.com>
> Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c      | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index 099792e6ce..644709650c 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -159,6 +159,7 @@
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable               ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable                   ##
> CONSUMES
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrM
> ask    ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> ## CONSUMES
> 
>  [Depex]
>    gEfiMpServiceProtocolGuid
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 3dde80f9ba..4debce3a0f 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -902,7 +902,7 @@ SetPageTableAttributes (
>    BOOLEAN               IsSplitted;
>    BOOLEAN               PageTableSplitted;
> 
> -  if (!mCpuSmmStaticPageTable) {
> +  if (!mCpuSmmStaticPageTable || (PcdGet8 (PcdHeapGuardPropertyMask)
> &
> + BIT3 | BIT2) != 0) {
>      return ;
>    }
> 
> --
> 2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
Posted by Wang, Jian J 7 years, 2 months ago 
You're right. "BIT3 | BIT2" should be enclosed by parentheses. Thanks for catching this issue.

> -----Original Message-----
> From: Dong, Eric
> Sent: Friday, October 13, 2017 9:24 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Wolman, Ayellet <ayellet.wolman@intel.com>
> Subject: RE: [PATCH 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table
> protection
> 
> Hi Jian,
> 
> > +  if (!mCpuSmmStaticPageTable || (PcdGet8 (PcdHeapGuardPropertyMask)
> > &
> > + BIT3 | BIT2) != 0) {
> 
> I think above code logic is not correct, the "&" will be handled before the "|"
> which is not an expected order, right?
> 
> Thanks,
> Eric
> 
> > -----Original Message-----
> > From: Wang, Jian J
> > Sent: Wednesday, October 11, 2017 11:18 AM
> > To: edk2-devel@lists.01.org
> > Cc: Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> > Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> > <ayellet.wolman@intel.com>
> > Subject: [PATCH 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table
> > protection
> >
> > Heap guard feature will update page attributes frequently. The page table
> > should not set to be read-only if heap guard feature is enabled for SMM
> > mode. Otherwise this feature cannot work.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Michael Kinney <michael.d.kinney@intel.com>
> > Cc: Ayellet Wolman <ayellet.wolman@intel.com>
> > Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 1 +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c      | 2 +-
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > index 099792e6ce..644709650c 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> > @@ -159,6 +159,7 @@
> >    gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable               ##
> > CONSUMES
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable                   ##
> > CONSUMES
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrM
> > ask    ## CONSUMES
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> > ## CONSUMES
> >
> >  [Depex]
> >    gEfiMpServiceProtocolGuid
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > index 3dde80f9ba..4debce3a0f 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > @@ -902,7 +902,7 @@ SetPageTableAttributes (
> >    BOOLEAN               IsSplitted;
> >    BOOLEAN               PageTableSplitted;
> >
> > -  if (!mCpuSmmStaticPageTable) {
> > +  if (!mCpuSmmStaticPageTable || (PcdGet8 (PcdHeapGuardPropertyMask)
> > &
> > + BIT3 | BIT2) != 0) {
> >      return ;
> >    }
> >
> > --
> > 2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.