UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow

[edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow

Posted by Sheng Wei 2 weeks, 5 days ago

Use SMM stack guard feature to detect SMM shadow stack overflow.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3280

Signed-off-by: Sheng Wei <w.sheng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Roger Feng <roger.feng@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index 07e7ea70de..6902584b1f 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -1016,6 +1016,7 @@ SmiPFHandler (
 {
   UINTN             PFAddress;
   UINTN             GuardPageAddress;
+  UINTN             ShadowStackGuardPageAddress;
   UINTN             CpuIndex;
 
   ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT);
@@ -1032,7 +1033,7 @@ SmiPFHandler (
   }
 
   //
-  // If a page fault occurs in SMRAM range, it might be in a SMM stack guard page,
+  // If a page fault occurs in SMRAM range, it might be in a SMM stack/shadow stack guard page,
   // or SMM page protection violation.
   //
   if ((PFAddress >= mCpuHotPlugData.SmrrBase) &&
@@ -1040,10 +1041,16 @@ SmiPFHandler (
     DumpCpuContext (InterruptType, SystemContext);
     CpuIndex = GetCpuIndex ();
     GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * (mSmmStackSize + mSmmShadowStackSize));
+    ShadowStackGuardPageAddress = (mSmmStackArrayBase + mSmmStackSize + EFI_PAGE_SIZE + CpuIndex * (mSmmStackSize + mSmmShadowStackSize));
     if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
         (PFAddress >= GuardPageAddress) &&
         (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) {
       DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n"));
+    } else if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
+        (mSmmShadowStackSize > 0) &&
+        (PFAddress >= ShadowStackGuardPageAddress) &&
+        (PFAddress < (ShadowStackGuardPageAddress + EFI_PAGE_SIZE))) {
+      DEBUG ((DEBUG_ERROR, "SMM shadow stack overflow!\n"));
     } else {
       if ((SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) != 0) {
         DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n", PFAddress));
-- 
2.16.2.windows.1



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

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow

Posted by Yao, Jiewen 2 weeks, 5 days ago

Hi
Would you please share the info on how you do unit test for the new added code?

Thank you

> -----Original Message-----
> From: Sheng, W <w.sheng@intel.com>
> Sent: Friday, March 26, 2021 2:04 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Feng, Roger <roger.feng@intel.com>
> Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow
> stack overflow
> 
> Use SMM stack guard feature to detect SMM shadow stack overflow.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3280
> 
> Signed-off-by: Sheng Wei <w.sheng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Roger Feng <roger.feng@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 07e7ea70de..6902584b1f 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -1016,6 +1016,7 @@ SmiPFHandler (
>  {
>    UINTN             PFAddress;
>    UINTN             GuardPageAddress;
> +  UINTN             ShadowStackGuardPageAddress;
>    UINTN             CpuIndex;
> 
>    ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT);
> @@ -1032,7 +1033,7 @@ SmiPFHandler (
>    }
> 
>    //
> -  // If a page fault occurs in SMRAM range, it might be in a SMM stack guard
> page,
> +  // If a page fault occurs in SMRAM range, it might be in a SMM stack/shadow
> stack guard page,
>    // or SMM page protection violation.
>    //
>    if ((PFAddress >= mCpuHotPlugData.SmrrBase) &&
> @@ -1040,10 +1041,16 @@ SmiPFHandler (
>      DumpCpuContext (InterruptType, SystemContext);
>      CpuIndex = GetCpuIndex ();
>      GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex *
> (mSmmStackSize + mSmmShadowStackSize));
> +    ShadowStackGuardPageAddress = (mSmmStackArrayBase + mSmmStackSize
> + EFI_PAGE_SIZE + CpuIndex * (mSmmStackSize + mSmmShadowStackSize));
>      if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
>          (PFAddress >= GuardPageAddress) &&
>          (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) {
>        DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n"));
> +    } else if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
> +        (mSmmShadowStackSize > 0) &&
> +        (PFAddress >= ShadowStackGuardPageAddress) &&
> +        (PFAddress < (ShadowStackGuardPageAddress + EFI_PAGE_SIZE))) {
> +      DEBUG ((DEBUG_ERROR, "SMM shadow stack overflow!\n"));
>      } else {
>        if ((SystemContext.SystemContextX64->ExceptionData & IA32_PF_EC_ID) !=
> 0) {
>          DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n",
> PFAddress));
> --
> 2.16.2.windows.1



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

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow

Posted by Sheng Wei 2 weeks, 5 days ago

Hi Jiewen,
In current code, if SMM stack guard is enabled, there is a guard page at the top of SMM shadow stack.
If SMM shadow stack overflow Happens, it will touch the guard page, and trigger the #PF exception.
In this patch, I will check the PFAddress in SmiPFHandler(), if it belongs to the range of SMM shadow stack guard page, I will show the error message.

unit test:
I use recursive function to do the test. In each function call, it will push the return address to the SMM shadow stack.
When the loop reaches to a certain amount, it will finally touch the guard page, and trigger #PF exception.

Thank you
BR
Sheng Wei

> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: 2021年3月26日 14:14
> To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>;
> Feng, Roger <roger.feng@intel.com>
> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
> shadow stack overflow
> 
> Hi
> Would you please share the info on how you do unit test for the new added
> code?
> 
> Thank you
> 
> > -----Original Message-----
> > From: Sheng, W <w.sheng@intel.com>
> > Sent: Friday, March 26, 2021 2:04 PM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
> > <rahul1.kumar@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Feng,
> > Roger <roger.feng@intel.com>
> > Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
> shadow
> > stack overflow
> >
> > Use SMM stack guard feature to detect SMM shadow stack overflow.
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3280
> >
> > Signed-off-by: Sheng Wei <w.sheng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Roger Feng <roger.feng@intel.com>
> > ---
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > index 07e7ea70de..6902584b1f 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > @@ -1016,6 +1016,7 @@ SmiPFHandler (
> >  {
> >    UINTN             PFAddress;
> >    UINTN             GuardPageAddress;
> > +  UINTN             ShadowStackGuardPageAddress;
> >    UINTN             CpuIndex;
> >
> >    ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); @@ -1032,7
> > +1033,7 @@ SmiPFHandler (
> >    }
> >
> >    //
> > -  // If a page fault occurs in SMRAM range, it might be in a SMM
> > stack guard page,
> > +  // If a page fault occurs in SMRAM range, it might be in a SMM
> > + stack/shadow
> > stack guard page,
> >    // or SMM page protection violation.
> >    //
> >    if ((PFAddress >= mCpuHotPlugData.SmrrBase) && @@ -1040,10 +1041,16
> > @@ SmiPFHandler (
> >      DumpCpuContext (InterruptType, SystemContext);
> >      CpuIndex = GetCpuIndex ();
> >      GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE +
> CpuIndex
> > * (mSmmStackSize + mSmmShadowStackSize));
> > +    ShadowStackGuardPageAddress = (mSmmStackArrayBase +
> mSmmStackSize
> > + EFI_PAGE_SIZE + CpuIndex * (mSmmStackSize +
> mSmmShadowStackSize));
> >      if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
> >          (PFAddress >= GuardPageAddress) &&
> >          (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) {
> >        DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n"));
> > +    } else if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
> > +        (mSmmShadowStackSize > 0) &&
> > +        (PFAddress >= ShadowStackGuardPageAddress) &&
> > +        (PFAddress < (ShadowStackGuardPageAddress + EFI_PAGE_SIZE))) {
> > +      DEBUG ((DEBUG_ERROR, "SMM shadow stack overflow!\n"));
> >      } else {
> >        if ((SystemContext.SystemContextX64->ExceptionData &
> > IA32_PF_EC_ID) !=
> > 0) {
> >          DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n",
> > PFAddress));
> > --
> > 2.16.2.windows.1



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

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow

Posted by Yao, Jiewen 2 weeks, 2 days ago

Thank you very much!

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Sheng, W <w.sheng@intel.com>
> Sent: Friday, March 26, 2021 2:33 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Feng, Roger
> <roger.feng@intel.com>
> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
> shadow stack overflow
> 
> Hi Jiewen,
> In current code, if SMM stack guard is enabled, there is a guard page at the top
> of SMM shadow stack.
> If SMM shadow stack overflow Happens, it will touch the guard page, and
> trigger the #PF exception.
> In this patch, I will check the PFAddress in SmiPFHandler(), if it belongs to the
> range of SMM shadow stack guard page, I will show the error message.
> 
> unit test:
> I use recursive function to do the test. In each function call, it will push the
> return address to the SMM shadow stack.
> When the loop reaches to a certain amount, it will finally touch the guard page,
> and trigger #PF exception.
> 
> Thank you
> BR
> Sheng Wei
> 
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: 2021年3月26日 14:14
> > To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> > Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>;
> > Feng, Roger <roger.feng@intel.com>
> > Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
> > shadow stack overflow
> >
> > Hi
> > Would you please share the info on how you do unit test for the new added
> > code?
> >
> > Thank you
> >
> > > -----Original Message-----
> > > From: Sheng, W <w.sheng@intel.com>
> > > Sent: Friday, March 26, 2021 2:04 PM
> > > To: devel@edk2.groups.io
> > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > > Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
> > > <rahul1.kumar@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Feng,
> > > Roger <roger.feng@intel.com>
> > > Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
> > shadow
> > > stack overflow
> > >
> > > Use SMM stack guard feature to detect SMM shadow stack overflow.
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3280
> > >
> > > Signed-off-by: Sheng Wei <w.sheng@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Roger Feng <roger.feng@intel.com>
> > > ---
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > index 07e7ea70de..6902584b1f 100644
> > > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> > > @@ -1016,6 +1016,7 @@ SmiPFHandler (
> > >  {
> > >    UINTN             PFAddress;
> > >    UINTN             GuardPageAddress;
> > > +  UINTN             ShadowStackGuardPageAddress;
> > >    UINTN             CpuIndex;
> > >
> > >    ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); @@ -1032,7
> > > +1033,7 @@ SmiPFHandler (
> > >    }
> > >
> > >    //
> > > -  // If a page fault occurs in SMRAM range, it might be in a SMM
> > > stack guard page,
> > > +  // If a page fault occurs in SMRAM range, it might be in a SMM
> > > + stack/shadow
> > > stack guard page,
> > >    // or SMM page protection violation.
> > >    //
> > >    if ((PFAddress >= mCpuHotPlugData.SmrrBase) && @@ -1040,10 +1041,16
> > > @@ SmiPFHandler (
> > >      DumpCpuContext (InterruptType, SystemContext);
> > >      CpuIndex = GetCpuIndex ();
> > >      GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE +
> > CpuIndex
> > > * (mSmmStackSize + mSmmShadowStackSize));
> > > +    ShadowStackGuardPageAddress = (mSmmStackArrayBase +
> > mSmmStackSize
> > > + EFI_PAGE_SIZE + CpuIndex * (mSmmStackSize +
> > mSmmShadowStackSize));
> > >      if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
> > >          (PFAddress >= GuardPageAddress) &&
> > >          (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) {
> > >        DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n"));
> > > +    } else if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
> > > +        (mSmmShadowStackSize > 0) &&
> > > +        (PFAddress >= ShadowStackGuardPageAddress) &&
> > > +        (PFAddress < (ShadowStackGuardPageAddress + EFI_PAGE_SIZE))) {
> > > +      DEBUG ((DEBUG_ERROR, "SMM shadow stack overflow!\n"));
> > >      } else {
> > >        if ((SystemContext.SystemContextX64->ExceptionData &
> > > IA32_PF_EC_ID) !=
> > > 0) {
> > >          DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n",
> > > PFAddress));
> > > --
> > > 2.16.2.windows.1



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

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow

Posted by Laszlo Ersek 1 week ago

Ray,

On 03/29/21 07:13, Yao, Jiewen wrote:
> Thank you very much!
> 
> Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

can you please review and merge this patch? You were the UefiCpuPkg
reviewer on the following two commits as well:

3eb69b081c68 ("UefiCpuPkg/PiSmmCpu: Add Shadow Stack Support for X86
SMM.", 2019-02-28)

ef91b07388e1 ("UefiCpuPkg/PiSmmCpuDxeSmm: Fix SMM stack offset is not
correct", 2021-03-02)

Thanks
Laszlo

> 
>> -----Original Message-----
>> From: Sheng, W <w.sheng@intel.com>
>> Sent: Friday, March 26, 2021 2:33 PM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
>> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Feng, Roger
>> <roger.feng@intel.com>
>> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
>> shadow stack overflow
>>
>> Hi Jiewen,
>> In current code, if SMM stack guard is enabled, there is a guard page at the top
>> of SMM shadow stack.
>> If SMM shadow stack overflow Happens, it will touch the guard page, and
>> trigger the #PF exception.
>> In this patch, I will check the PFAddress in SmiPFHandler(), if it belongs to the
>> range of SMM shadow stack guard page, I will show the error message.
>>
>> unit test:
>> I use recursive function to do the test. In each function call, it will push the
>> return address to the SMM shadow stack.
>> When the loop reaches to a certain amount, it will finally touch the guard page,
>> and trigger #PF exception.
>>
>> Thank you
>> BR
>> Sheng Wei
>>
>>> -----Original Message-----
>>> From: Yao, Jiewen <jiewen.yao@intel.com>
>>> Sent: 2021年3月26日 14:14
>>> To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
>>> Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>;
>>> Feng, Roger <roger.feng@intel.com>
>>> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
>>> shadow stack overflow
>>>
>>> Hi
>>> Would you please share the info on how you do unit test for the new added
>>> code?
>>>
>>> Thank you
>>>
>>>> -----Original Message-----
>>>> From: Sheng, W <w.sheng@intel.com>
>>>> Sent: Friday, March 26, 2021 2:04 PM
>>>> To: devel@edk2.groups.io
>>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
>>>> Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
>>>> <rahul1.kumar@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Feng,
>>>> Roger <roger.feng@intel.com>
>>>> Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
>>> shadow
>>>> stack overflow
>>>>
>>>> Use SMM stack guard feature to detect SMM shadow stack overflow.
>>>>
>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3280
>>>>
>>>> Signed-off-by: Sheng Wei <w.sheng@intel.com>
>>>> Cc: Eric Dong <eric.dong@intel.com>
>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>> Cc: Roger Feng <roger.feng@intel.com>
>>>> ---
>>>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 9 ++++++++-
>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>>>> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>>>> index 07e7ea70de..6902584b1f 100644
>>>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>>>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
>>>> @@ -1016,6 +1016,7 @@ SmiPFHandler (
>>>>  {
>>>>    UINTN             PFAddress;
>>>>    UINTN             GuardPageAddress;
>>>> +  UINTN             ShadowStackGuardPageAddress;
>>>>    UINTN             CpuIndex;
>>>>
>>>>    ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); @@ -1032,7
>>>> +1033,7 @@ SmiPFHandler (
>>>>    }
>>>>
>>>>    //
>>>> -  // If a page fault occurs in SMRAM range, it might be in a SMM
>>>> stack guard page,
>>>> +  // If a page fault occurs in SMRAM range, it might be in a SMM
>>>> + stack/shadow
>>>> stack guard page,
>>>>    // or SMM page protection violation.
>>>>    //
>>>>    if ((PFAddress >= mCpuHotPlugData.SmrrBase) && @@ -1040,10 +1041,16
>>>> @@ SmiPFHandler (
>>>>      DumpCpuContext (InterruptType, SystemContext);
>>>>      CpuIndex = GetCpuIndex ();
>>>>      GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE +
>>> CpuIndex
>>>> * (mSmmStackSize + mSmmShadowStackSize));
>>>> +    ShadowStackGuardPageAddress = (mSmmStackArrayBase +
>>> mSmmStackSize
>>>> + EFI_PAGE_SIZE + CpuIndex * (mSmmStackSize +
>>> mSmmShadowStackSize));
>>>>      if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
>>>>          (PFAddress >= GuardPageAddress) &&
>>>>          (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) {
>>>>        DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n"));
>>>> +    } else if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
>>>> +        (mSmmShadowStackSize > 0) &&
>>>> +        (PFAddress >= ShadowStackGuardPageAddress) &&
>>>> +        (PFAddress < (ShadowStackGuardPageAddress + EFI_PAGE_SIZE))) {
>>>> +      DEBUG ((DEBUG_ERROR, "SMM shadow stack overflow!\n"));
>>>>      } else {
>>>>        if ((SystemContext.SystemContextX64->ExceptionData &
>>>> IA32_PF_EC_ID) !=
>>>> 0) {
>>>>          DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n",
>>>> PFAddress));
>>>> --
>>>> 2.16.2.windows.1
> 
> 
> 
> 
> 
> 



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

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow

Posted by Ni, Ray 5 days, 3 hours ago

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

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Tuesday, April 6, 2021 10:27 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Sheng, W <w.sheng@intel.com>; Ni, Ray <ray.ni@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Feng, Roger <roger.feng@intel.com>
> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow
> 
> Ray,
> 
> On 03/29/21 07:13, Yao, Jiewen wrote:
> > Thank you very much!
> >
> > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> 
> can you please review and merge this patch? You were the UefiCpuPkg
> reviewer on the following two commits as well:
> 
> 3eb69b081c68 ("UefiCpuPkg/PiSmmCpu: Add Shadow Stack Support for X86
> SMM.", 2019-02-28)
> 
> ef91b07388e1 ("UefiCpuPkg/PiSmmCpuDxeSmm: Fix SMM stack offset is not
> correct", 2021-03-02)
> 
> Thanks
> Laszlo
> 
> >
> >> -----Original Message-----
> >> From: Sheng, W <w.sheng@intel.com>
> >> Sent: Friday, March 26, 2021 2:33 PM
> >> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> >> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
> >> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Feng, Roger
> >> <roger.feng@intel.com>
> >> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
> >> shadow stack overflow
> >>
> >> Hi Jiewen,
> >> In current code, if SMM stack guard is enabled, there is a guard page at the top
> >> of SMM shadow stack.
> >> If SMM shadow stack overflow Happens, it will touch the guard page, and
> >> trigger the #PF exception.
> >> In this patch, I will check the PFAddress in SmiPFHandler(), if it belongs to the
> >> range of SMM shadow stack guard page, I will show the error message.
> >>
> >> unit test:
> >> I use recursive function to do the test. In each function call, it will push the
> >> return address to the SMM shadow stack.
> >> When the loop reaches to a certain amount, it will finally touch the guard page,
> >> and trigger #PF exception.
> >>
> >> Thank you
> >> BR
> >> Sheng Wei
> >>
> >>> -----Original Message-----
> >>> From: Yao, Jiewen <jiewen.yao@intel.com>
> >>> Sent: 2021年3月26日 14:14
> >>> To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> >>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> >>> Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>;
> >>> Feng, Roger <roger.feng@intel.com>
> >>> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
> >>> shadow stack overflow
> >>>
> >>> Hi
> >>> Would you please share the info on how you do unit test for the new added
> >>> code?
> >>>
> >>> Thank you
> >>>
> >>>> -----Original Message-----
> >>>> From: Sheng, W <w.sheng@intel.com>
> >>>> Sent: Friday, March 26, 2021 2:04 PM
> >>>> To: devel@edk2.groups.io
> >>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> >>>> Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
> >>>> <rahul1.kumar@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Feng,
> >>>> Roger <roger.feng@intel.com>
> >>>> Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
> >>> shadow
> >>>> stack overflow
> >>>>
> >>>> Use SMM stack guard feature to detect SMM shadow stack overflow.
> >>>>
> >>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3280
> >>>>
> >>>> Signed-off-by: Sheng Wei <w.sheng@intel.com>
> >>>> Cc: Eric Dong <eric.dong@intel.com>
> >>>> Cc: Ray Ni <ray.ni@intel.com>
> >>>> Cc: Laszlo Ersek <lersek@redhat.com>
> >>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> >>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >>>> Cc: Roger Feng <roger.feng@intel.com>
> >>>> ---
> >>>>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 9 ++++++++-
> >>>>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> >>>> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> >>>> index 07e7ea70de..6902584b1f 100644
> >>>> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> >>>> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> >>>> @@ -1016,6 +1016,7 @@ SmiPFHandler (
> >>>>  {
> >>>>    UINTN             PFAddress;
> >>>>    UINTN             GuardPageAddress;
> >>>> +  UINTN             ShadowStackGuardPageAddress;
> >>>>    UINTN             CpuIndex;
> >>>>
> >>>>    ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); @@ -1032,7
> >>>> +1033,7 @@ SmiPFHandler (
> >>>>    }
> >>>>
> >>>>    //
> >>>> -  // If a page fault occurs in SMRAM range, it might be in a SMM
> >>>> stack guard page,
> >>>> +  // If a page fault occurs in SMRAM range, it might be in a SMM
> >>>> + stack/shadow
> >>>> stack guard page,
> >>>>    // or SMM page protection violation.
> >>>>    //
> >>>>    if ((PFAddress >= mCpuHotPlugData.SmrrBase) && @@ -1040,10 +1041,16
> >>>> @@ SmiPFHandler (
> >>>>      DumpCpuContext (InterruptType, SystemContext);
> >>>>      CpuIndex = GetCpuIndex ();
> >>>>      GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE +
> >>> CpuIndex
> >>>> * (mSmmStackSize + mSmmShadowStackSize));
> >>>> +    ShadowStackGuardPageAddress = (mSmmStackArrayBase +
> >>> mSmmStackSize
> >>>> + EFI_PAGE_SIZE + CpuIndex * (mSmmStackSize +
> >>> mSmmShadowStackSize));
> >>>>      if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
> >>>>          (PFAddress >= GuardPageAddress) &&
> >>>>          (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) {
> >>>>        DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n"));
> >>>> +    } else if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
> >>>> +        (mSmmShadowStackSize > 0) &&
> >>>> +        (PFAddress >= ShadowStackGuardPageAddress) &&
> >>>> +        (PFAddress < (ShadowStackGuardPageAddress + EFI_PAGE_SIZE))) {
> >>>> +      DEBUG ((DEBUG_ERROR, "SMM shadow stack overflow!\n"));
> >>>>      } else {
> >>>>        if ((SystemContext.SystemContextX64->ExceptionData &
> >>>> IA32_PF_EC_ID) !=
> >>>> 0) {
> >>>>          DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n",
> >>>> PFAddress));
> >>>> --
> >>>> 2.16.2.windows.1
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 
> 



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