Reviewed-by: Jiewen Yao <jiewen....@intel.com> > -----Original Message----- > From: Sheng, W <w.sh...@intel.com> > Sent: Friday, February 26, 2021 4:03 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 v6 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Fix SMM stack offset is > not correct > > In function InitGdt(), SmiPFHandler() and Gen4GPageTable(), it uses > CpuIndex * mSmmStackSize to get the SMM stack address offset for > multi processor. It misses the SMM Shadow Stack Size. Each processor > will use mSmmStackSize + mSmmShadowStackSize in the memory. > It should use CpuIndex * (mSmmStackSize + mSmmShadowStackSize) to get > this SMM stack address offset. If mSmmShadowStackSize > 0 and multi > processor enabled, it will get the wrong offset value. > CET shadow stack feature will set the value of mSmmShadowStackSize. > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3237 > > 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/MpService.c | 6 ++++-- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 4 +++- > UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 2 +- > 3 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > index 4bcd217917..6227b2428a 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c > @@ -23,6 +23,8 @@ SPIN_LOCK *mPFLock = NULL; > SMM_CPU_SYNC_MODE mCpuSmmSyncMode; > BOOLEAN mMachineCheckSupported = FALSE; > > +extern UINTN mSmmShadowStackSize; > + > /** > Performs an atomic compare exchange operation to get semaphore. > The compare exchange operation must be performed using > @@ -920,7 +922,7 @@ Gen4GPageTable ( > // Add two more pages for known good stack and stack guard page, > // then find the lower 2MB aligned address. > // > - High2MBoundary = (mSmmStackArrayEnd - mSmmStackSize + EFI_PAGE_SIZE > * 2) & ~(SIZE_2MB-1); > + High2MBoundary = (mSmmStackArrayEnd - mSmmStackSize - > mSmmShadowStackSize + EFI_PAGE_SIZE * 2) & ~(SIZE_2MB-1); > PagesNeeded = ((High2MBoundary - Low2MBoundary) / SIZE_2MB) + 1; > } > // > @@ -971,7 +973,7 @@ Gen4GPageTable ( > // Mark the guard page as non-present > // > Pte[Index] = PageAddress | mAddressEncMask; > - GuardPage += mSmmStackSize; > + GuardPage += (mSmmStackSize + mSmmShadowStackSize); > if (GuardPage > mSmmStackArrayEnd) { > GuardPage = 0; > } > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > index cdc1fcefc5..07e7ea70de 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c > @@ -13,6 +13,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #define PAGE_TABLE_PAGES 8 > #define ACC_MAX_BIT BIT3 > > +extern UINTN mSmmShadowStackSize; > + > LIST_ENTRY mPagePool = INITIALIZE_LIST_HEAD_VARIABLE > (mPagePool); > BOOLEAN m1GPageTableSupport = FALSE; > BOOLEAN mCpuSmmRestrictedMemoryAccess; > @@ -1037,7 +1039,7 @@ SmiPFHandler ( > (PFAddress < (mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize))) { > DumpCpuContext (InterruptType, SystemContext); > CpuIndex = GetCpuIndex (); > - GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * > mSmmStackSize); > + GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE + CpuIndex * > (mSmmStackSize + mSmmShadowStackSize)); > if ((FeaturePcdGet (PcdCpuSmmStackGuard)) && > (PFAddress >= GuardPageAddress) && > (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) { > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > index 7ef3b1d488..661c1ba294 100644 > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c > @@ -93,7 +93,7 @@ InitGdt ( > // > // Setup top of known good stack as IST1 for each processor. > // > - *(UINTN *)(TssBase + TSS_X64_IST1_OFFSET) = (mSmmStackArrayBase + > EFI_PAGE_SIZE + Index * mSmmStackSize); > + *(UINTN *)(TssBase + TSS_X64_IST1_OFFSET) = (mSmmStackArrayBase + > EFI_PAGE_SIZE + Index * (mSmmStackSize + mSmmShadowStackSize)); > } > } > > -- > 2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#72231): https://edk2.groups.io/g/devel/message/72231 Mute This Topic: https://groups.io/mt/80922789/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
