* [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow
@ 2021-03-26 6:04 Sheng Wei
2021-03-26 6:13 ` Yao, Jiewen
0 siblings, 1 reply; 6+ messages in thread
From: Sheng Wei @ 2021-03-26 6:04 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar, Jiewen Yao,
Roger Feng
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@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Roger Feng <roger.feng@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow
2021-03-26 6:04 [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow Sheng Wei
@ 2021-03-26 6:13 ` Yao, Jiewen
2021-03-26 6:33 ` Sheng Wei
0 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2021-03-26 6:13 UTC (permalink / raw)
To: Sheng, W, devel@edk2.groups.io
Cc: Dong, Eric, Ni, Ray, Laszlo Ersek, Kumar, Rahul1, Feng, Roger
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@intel.com>
> Sent: Friday, March 26, 2021 2:04 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Feng, Roger <roger.feng@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@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Roger Feng <roger.feng@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow
2021-03-26 6:13 ` Yao, Jiewen
@ 2021-03-26 6:33 ` Sheng Wei
2021-03-29 5:13 ` Yao, Jiewen
0 siblings, 1 reply; 6+ messages in thread
From: Sheng Wei @ 2021-03-26 6:33 UTC (permalink / raw)
To: Yao, Jiewen, devel@edk2.groups.io
Cc: Dong, Eric, Ni, Ray, Laszlo Ersek, Kumar, Rahul1, Feng, Roger
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@intel.com>
> Sent: 2021年3月26日 14:14
> To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>;
> Feng, Roger <roger.feng@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@intel.com>
> > Sent: Friday, March 26, 2021 2:04 PM
> > To: devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
> > <rahul1.kumar@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Feng,
> > Roger <roger.feng@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@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Roger Feng <roger.feng@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow
2021-03-26 6:33 ` Sheng Wei
@ 2021-03-29 5:13 ` Yao, Jiewen
2021-04-06 14:26 ` [edk2-devel] " Laszlo Ersek
0 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2021-03-29 5:13 UTC (permalink / raw)
To: Sheng, W, devel@edk2.groups.io
Cc: Dong, Eric, Ni, Ray, Laszlo Ersek, Kumar, Rahul1, Feng, Roger
Thank you very much!
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> -----Original Message-----
> From: Sheng, W <w.sheng@intel.com>
> Sent: Friday, March 26, 2021 2:33 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Feng, Roger
> <roger.feng@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@intel.com>
> > Sent: 2021年3月26日 14:14
> > To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> > Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>;
> > Feng, Roger <roger.feng@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@intel.com>
> > > Sent: Friday, March 26, 2021 2:04 PM
> > > To: devel@edk2.groups.io
> > > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> > > Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
> > > <rahul1.kumar@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Feng,
> > > Roger <roger.feng@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@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Rahul Kumar <rahul1.kumar@intel.com>
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Roger Feng <roger.feng@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow
2021-03-29 5:13 ` Yao, Jiewen
@ 2021-04-06 14:26 ` Laszlo Ersek
2021-04-09 5:05 ` Ni, Ray
0 siblings, 1 reply; 6+ messages in thread
From: Laszlo Ersek @ 2021-04-06 14:26 UTC (permalink / raw)
To: devel, jiewen.yao, Sheng, W, Ni, Ray
Cc: Dong, Eric, Kumar, Rahul1, Feng, Roger
Ray,
On 03/29/21 07:13, Yao, Jiewen wrote:
> Thank you very much!
>
> Reviewed-by: Jiewen Yao <Jiewen.yao@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.sheng@intel.com>
>> Sent: Friday, March 26, 2021 2:33 PM
>> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
>> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Feng, Roger
>> <roger.feng@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@intel.com>
>>> Sent: 2021年3月26日 14:14
>>> To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
>>> Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>;
>>> Feng, Roger <roger.feng@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@intel.com>
>>>> Sent: Friday, March 26, 2021 2:04 PM
>>>> To: devel@edk2.groups.io
>>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
>>>> Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
>>>> <rahul1.kumar@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Feng,
>>>> Roger <roger.feng@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@intel.com>
>>>> Cc: Eric Dong <eric.dong@intel.com>
>>>> Cc: Ray Ni <ray.ni@intel.com>
>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
>>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>>>> Cc: Roger Feng <roger.feng@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
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow
2021-04-06 14:26 ` [edk2-devel] " Laszlo Ersek
@ 2021-04-09 5:05 ` Ni, Ray
0 siblings, 0 replies; 6+ messages in thread
From: Ni, Ray @ 2021-04-09 5:05 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, Yao, Jiewen, Sheng, W
Cc: Dong, Eric, Kumar, Rahul1, Feng, Roger
Reviewed-by: Ray Ni <ray.ni@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.yao@intel.com>; Sheng, W <w.sheng@intel.com>; Ni, Ray <ray.ni@intel.com>
> Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Feng, Roger <roger.feng@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.yao@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.sheng@intel.com>
> >> Sent: Friday, March 26, 2021 2:33 PM
> >> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> >> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek
> >> <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Feng, Roger
> >> <roger.feng@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@intel.com>
> >>> Sent: 2021年3月26日 14:14
> >>> To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> >>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> >>> Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>;
> >>> Feng, Roger <roger.feng@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@intel.com>
> >>>> Sent: Friday, March 26, 2021 2:04 PM
> >>>> To: devel@edk2.groups.io
> >>>> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> >>>> Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1
> >>>> <rahul1.kumar@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Feng,
> >>>> Roger <roger.feng@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@intel.com>
> >>>> Cc: Eric Dong <eric.dong@intel.com>
> >>>> Cc: Ray Ni <ray.ni@intel.com>
> >>>> Cc: Laszlo Ersek <lersek@redhat.com>
> >>>> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> >>>> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >>>> Cc: Roger Feng <roger.feng@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
> >
> >
> >
> >
> >
> >
>
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-04-09 5:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-26 6:04 [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow Sheng Wei
2021-03-26 6:13 ` Yao, Jiewen
2021-03-26 6:33 ` Sheng Wei
2021-03-29 5:13 ` Yao, Jiewen
2021-04-06 14:26 ` [edk2-devel] " Laszlo Ersek
2021-04-09 5:05 ` Ni, Ray
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox