Reviewed-by: Ray Ni <ray...@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....@intel.com>; Sheng, W
> <w.sh...@intel.com>; Ni, Ray <ray...@intel.com>
> Cc: Dong, Eric <eric.d...@intel.com>; Kumar, Rahul1 <rahul1.ku...@intel.com>;
> Feng, Roger <roger.f...@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....@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.sh...@intel.com>
> >> Sent: Friday, March 26, 2021 2:33 PM
> >> To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io
> >> Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Laszlo
> >> Ersek
> >> <ler...@redhat.com>; Kumar, Rahul1 <rahul1.ku...@intel.com>; Feng, Roger
> >> <roger.f...@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....@intel.com>
> >>> Sent: 2021年3月26日 14:14
> >>> To: Sheng, W <w.sh...@intel.com>; devel@edk2.groups.io
> >>> Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Laszlo
> >>> Ersek <ler...@redhat.com>; Kumar, Rahul1 <rahul1.ku...@intel.com>;
> >>> Feng, Roger <roger.f...@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.sh...@intel.com>
> >>>> Sent: Friday, March 26, 2021 2:04 PM
> >>>> To: devel@edk2.groups.io
> >>>> Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>;
> >>>> Laszlo Ersek <ler...@redhat.com>; Kumar, Rahul1
> >>>> <rahul1.ku...@intel.com>; Yao, Jiewen <jiewen....@intel.com>; Feng,
> >>>> Roger <roger.f...@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.sh...@intel.com>
> >>>> Cc: Eric Dong <eric.d...@intel.com>
> >>>> Cc: Ray Ni <ray...@intel.com>
> >>>> Cc: Laszlo Ersek <ler...@redhat.com>
> >>>> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> >>>> Cc: Jiewen Yao <jiewen....@intel.com>
> >>>> Cc: Roger Feng <roger.f...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
