merged. Thanks, Ray ________________________________ From: Wu, Jiaxin Sent: Friday, November 10, 2023 8:43 AM To: Kinney, Michael D ; Laszlo Ersek ; devel@edk2.groups.io ; Gao, Liming Cc: Dong, Eric ; Ni, Ray ; Zeng, Star ; Gerd Hoffmann ; Kumar, Rahul R Subject: RE: [edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Fix CP Exception when CET enable Thank you, Mike, the PR (https://github.com/tianocore/edk2/pull/4867) has been synced & updated with reviewed by tag, and we can merge once pass the CI check. > -----Original Message----- > From: Kinney, Michael D > Sent: Friday, November 10, 2023 8:25 AM > To: Wu, Jiaxin ; Laszlo Ersek ; > devel@edk2.groups.io; Gao, Liming > Cc: Dong, Eric ; Ni, Ray ; Zeng, Star > ; Gerd Hoffmann ; Kumar, Rahul R > ; Kinney, Michael D > Subject: RE: [edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Fix CP > Exception when CET enable > > I approve this change for edk2-stable202311 > > The PR looks out of sync with this email patch. > > Can you please update PR with latest patch and commit > message that was reviewed and add review tags? > > Mike > > > -----Original Message----- > > From: Wu, Jiaxin > > Sent: Thursday, November 9, 2023 4:01 PM > > To: Laszlo Ersek ; devel@edk2.groups.io; Gao, > > Liming ; Kinney, Michael D > > > > Cc: Dong, Eric ; Ni, Ray ; > > Zeng, Star ; Gerd Hoffmann ; > > Kumar, Rahul R > > Subject: RE: [edk2-devel] [PATCH v4] UefiCpuPkg/PiSmmCpuDxeSmm: Fix > CP > > Exception when CET enable > > > > Hi Liming & Mike, > > > > Could you help approve & merge this patch into stable tag? It has got > > below reviewed-by: > > > > Reviewed-by: Laszlo Ersek > > Reviewed-by: Ray Ni > > Reviewed-by: Eric Dong > > > > 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 ; devel@edk2.groups.io; Gao, > > Liming > > > ; Kinney, Michael D > > > > > > Cc: Dong, Eric ; Ni, Ray ; > > Zeng, Star > > > ; Gerd Hoffmann ; Kumar, > > Rahul R > > > > > > 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 > > > > Sent: Wednesday, November 8, 2023 2:57 AM > > > > To: devel@edk2.groups.io; Wu, Jiaxin > > > > Cc: Dong, Eric ; Ni, Ray ; > > Zeng, Star > > > > ; Gerd Hoffmann ; Kumar, > > Rahul > > > R > > > > > > > > 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 > > > > > Cc: Ray Ni > > > > > Cc: Zeng Star > > > > > Cc: Gerd Hoffmann > > > > > Cc: Rahul Kumar > > > > > Cc: Laszlo Ersek > > > > > Signed-off-by: Jiaxin Wu > > > > > --- > > > > > 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111022): https://edk2.groups.io/g/devel/message/111022 Mute This Topic: https://groups.io/mt/102434876/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-