Hi Liming & Mike, Could you help approve & merge this patch into stable tag? It has got below reviewed-by:
Reviewed-by: Laszlo Ersek <ler...@redhat.com> Reviewed-by: Ray Ni <ray...@intel.com> Reviewed-by: Eric Dong <eric.d...@intel.com> I also created the PR: https://github.com/tianocore/edk2/pull/4867 Thanks, Jiaxin > -----Original Message----- > From: Wu, Jiaxin > Sent: Wednesday, November 8, 2023 9:17 AM > To: Laszlo Ersek <ler...@redhat.com>; devel@edk2.groups.io; Gao, Liming > <gaolim...@byosoft.com.cn>; Kinney, Michael D > <michael.d.kin...@intel.com> > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Zeng, Star > <star.z...@intel.com>; Gerd Hoffmann <kra...@redhat.com>; Kumar, Rahul R > <rahul.r.ku...@intel.com> > Subject: RE: [edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Fix CP > Exception when CET enable > > Hi Liming & Mike & Ray, > > Could you help approve this change for the coming edk2 stable tag? This is > critical bug fix in smm cpu driver to handler the CET check failure, I think > we > need this change for the stable tag. > > Thanks, > Jiaxin > > > -----Original Message----- > > From: Laszlo Ersek <ler...@redhat.com> > > Sent: Wednesday, November 8, 2023 2:57 AM > > To: devel@edk2.groups.io; Wu, Jiaxin <jiaxin...@intel.com> > > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Zeng, Star > > <star.z...@intel.com>; Gerd Hoffmann <kra...@redhat.com>; Kumar, Rahul > R > > <rahul.r.ku...@intel.com> > > Subject: Re: [edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Fix > CP > > Exception when CET enable > > > > On 11/7/23 02:24, Wu, Jiaxin wrote: > > > Root cause: > > > 1. Before DisableReadonlyPageWriteProtect() is called, the return > > > address (#1) is pushed in shadow stack. > > > 2. CET is disabled. > > > 3. DisableReadonlyPageWriteProtect() returns to #1. > > > 4. Page table is modified. > > > 5. EnableReadonlyPageWriteProtect() is called, but the return > > > address (#2) is not pushed in shadow stack. > > > 6. CET is enabled. > > > 7. EnableReadonlyPageWriteProtect() returns to #2. > > > #CP exception happens because the actual return address (#2) > > > doesn't match the return address stored in shadow stack (#1). > > > > > > Analysis: > > > Shadow stack will stop update after CET disable (DisableCet() in > > > DisableReadOnlyPageWriteProtect), but normal smi stack will be > > > continue updated with the function called and return > > > (DisableReadOnlyPageWriteProtect & EnableReadOnlyPageWriteProtect), > > > thus leading stack mismatch after CET re-enabled (EnableCet() in > > > EnableReadOnlyPageWriteProtect). > > > > > > According SDM Vol 3, 6.15-Control Protection Exception: > > > Normal smi stack and shadow stack must be matched when CET enable, > > > otherwise CP Exception will happen, which is caused by a near RET > > > instruction. > > > > > > CET is disabled in DisableCet(), while can be enabled in > > > EnableCet(). This way won't cause the problem because they are > > > implemented in a way that return address of DisableCet() is > > > poped out from shadow stack (Incsspq performs a pop to increases > > > the shadow stack) and EnableCet() doesn't use "RET" but "JMP" to > > > return to caller. So calling EnableCet() and DisableCet() doesn't > > > have the same issue as calling DisableReadonlyPageWriteProtect() > > > and EnableReadonlyPageWriteProtect(). > > > > > > With above root cause & analysis, define below 2 macros instead of > > > functions for WP & CET operation: > > > WRITE_UNPROTECT_RO_PAGES (Wp, Cet) > > > WRITE_PROTECT_RO_PAGES (Wp, Cet) > > > Because DisableCet() & EnableCet() must be in the same function > > > to avoid shadow stack and normal SMI stack mismatch. > > > > > > Note: WRITE_UNPROTECT_RO_PAGES () must be called pair with > > > WRITE_PROTECT_RO_PAGES () in same function. > > > > > > Cc: Eric Dong <eric.d...@intel.com> > > > Cc: Ray Ni <ray...@intel.com> > > > Cc: Zeng Star <star.z...@intel.com> > > > Cc: Gerd Hoffmann <kra...@redhat.com> > > > Cc: Rahul Kumar <rahul1.ku...@intel.com> > > > Cc: Laszlo Ersek <ler...@redhat.com> > > > Signed-off-by: Jiaxin Wu <jiaxin...@intel.com> > > > --- > > > UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 59 > > +++++++++++++---- > > > UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 73 > > +++++++++------------- > > > UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 7 ++- > > > 3 files changed, 81 insertions(+), 58 deletions(-) > > > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111001): https://edk2.groups.io/g/devel/message/111001 Mute This Topic: https://groups.io/mt/102434876/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
