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

[Yao, Jiewen]

Thank you very much!

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

> -----Original Message-----
> From: Sheng, W <w.sheng at intel.com>
> Sent: Friday, March 26, 2021 2:33 PM
> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io
> Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Laszlo Ersek
> <lersek at redhat.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>; Feng, Roger
> <roger.feng at 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 at intel.com>
> > Sent: 2021年3月26日 14:14
> > To: Sheng, W <w.sheng at intel.com>; devel at edk2.groups.io
> > Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Laszlo
> > Ersek <lersek at redhat.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>;
> > Feng, Roger <roger.feng at 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 at intel.com>
> > > Sent: Friday, March 26, 2021 2:04 PM
> > > To: devel at edk2.groups.io
> > > Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>;
> > > Laszlo Ersek <lersek at redhat.com>; Kumar, Rahul1
> > > <rahul1.kumar at intel.com>; Yao, Jiewen <jiewen.yao at intel.com>; Feng,
> > > Roger <roger.feng at 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 at intel.com>
> > > Cc: Eric Dong <eric.dong at intel.com>
> > > Cc: Ray Ni <ray.ni at intel.com>
> > > Cc: Laszlo Ersek <lersek at redhat.com>
> > > Cc: Rahul Kumar <rahul1.kumar at intel.com>
> > > Cc: Jiewen Yao <jiewen.yao at intel.com>
> > > Cc: Roger Feng <roger.feng at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-