1. Patchew
  2. EDK2
  3. View series

[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix infinite loop issue in SMM profile

Jian J Wang posted 1 patch 6 years, 10 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix infinite loop issue in SMM profile
Posted by Jian J Wang 6 years, 10 months ago 
The infinite loop is caused by the memory instruction, such as
"rep mov", operating on memory block crossing boundary of NON-PRESENT
pages. Because the address triggering page fault set in CR2 will be in
the first page, SmmProfilePFHandler() will only change the first page
into PRESENT. The page following will be still in NON-PRESENT status.

Since SmmProfilePFHandler() will setup single-step trap for the
instruction causing #PF, when the handler returns back to the
instruction and re-execute it, both #DB and #PF will be triggered
because the instruction wants to access both first and second page
but only first page is PRESENT.

Normally #DB exception will be handled first and its handler will
change first page back to NON-PRESENT status. Then #PF is handled
and its handler will change first page to PRESENT status again and
setup another single-step for the instruction triggering #PF. Then
the whole system falls into an infinite loop and the memory operation
will never move on.

This patch fix above situation by always changing 3 pages to PRESENT
status instead of just 1 page. Those 3 pages include the page before
and after the page causing #PF because memory operation instruction
may have direction flag (DF) set or cleared.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index 9588eaf029..8cdfc82a92 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -1302,6 +1302,8 @@ SmmProfilePFHandler (
 {
   UINT64                *PageTable;
   UINT64                PFAddress;
+  UINT64                RestoreAddress;
+  UINTN                 RestorePageNumber;
   UINTN                 CpuIndex;
   UINTN                 Index;
   UINT64                InstructionAddress;
@@ -1331,10 +1333,22 @@ SmmProfilePFHandler (
   PFAddress         = AsmReadCr2 ();
   CpuIndex          = GetCpuIndex ();
 
-  if (PFAddress <= 0xFFFFFFFF) {
-    RestorePageTableBelow4G (PageTable, PFAddress, CpuIndex, ErrorCode);
-  } else {
-    RestorePageTableAbove4G (PageTable, PFAddress, CpuIndex, ErrorCode, &IsValidPFAddress);
+  //
+  // Memory operation cross pages, like "rep mov" instruction, will cause
+  // infinite loop between this and Debug Trap handler. Since the direction
+  // of memory operation is unknown, the pages before and after current page
+  // should be taken into account as well.
+  //
+  RestorePageNumber = 3;
+  RestoreAddress = PFAddress - EFI_PAGE_SIZE;
+  while (RestorePageNumber > 0) {
+    if (RestoreAddress <= 0xFFFFFFFF) {
+      RestorePageTableBelow4G (PageTable, RestoreAddress, CpuIndex, ErrorCode);
+    } else {
+      RestorePageTableAbove4G (PageTable, RestoreAddress, CpuIndex, ErrorCode, &IsValidPFAddress);
+    }
+    RestoreAddress += EFI_PAGE_SIZE;
+    RestorePageNumber--;
   }
 
   if (!IsValidPFAddress) {
-- 
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix infinite loop issue in SMM profile
Posted by Yao, Jiewen 6 years, 10 months ago 
Hi
May I know how "3" is calculated?

+  RestorePageNumber = 3;

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Tuesday, February 6, 2018 11:02 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix infinite loop issue in
> SMM profile
> 
> The infinite loop is caused by the memory instruction, such as
> "rep mov", operating on memory block crossing boundary of NON-PRESENT
> pages. Because the address triggering page fault set in CR2 will be in
> the first page, SmmProfilePFHandler() will only change the first page
> into PRESENT. The page following will be still in NON-PRESENT status.
> 
> Since SmmProfilePFHandler() will setup single-step trap for the
> instruction causing #PF, when the handler returns back to the
> instruction and re-execute it, both #DB and #PF will be triggered
> because the instruction wants to access both first and second page
> but only first page is PRESENT.
> 
> Normally #DB exception will be handled first and its handler will
> change first page back to NON-PRESENT status. Then #PF is handled
> and its handler will change first page to PRESENT status again and
> setup another single-step for the instruction triggering #PF. Then
> the whole system falls into an infinite loop and the memory operation
> will never move on.
> 
> This patch fix above situation by always changing 3 pages to PRESENT
> status instead of just 1 page. Those 3 pages include the page before
> and after the page causing #PF because memory operation instruction
> may have direction flag (DF) set or cleared.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index 9588eaf029..8cdfc82a92 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -1302,6 +1302,8 @@ SmmProfilePFHandler (
>  {
>    UINT64                *PageTable;
>    UINT64                PFAddress;
> +  UINT64                RestoreAddress;
> +  UINTN                 RestorePageNumber;
>    UINTN                 CpuIndex;
>    UINTN                 Index;
>    UINT64                InstructionAddress;
> @@ -1331,10 +1333,22 @@ SmmProfilePFHandler (
>    PFAddress         = AsmReadCr2 ();
>    CpuIndex          = GetCpuIndex ();
> 
> -  if (PFAddress <= 0xFFFFFFFF) {
> -    RestorePageTableBelow4G (PageTable, PFAddress, CpuIndex, ErrorCode);
> -  } else {
> -    RestorePageTableAbove4G (PageTable, PFAddress, CpuIndex, ErrorCode,
> &IsValidPFAddress);
> +  //
> +  // Memory operation cross pages, like "rep mov" instruction, will cause
> +  // infinite loop between this and Debug Trap handler. Since the direction
> +  // of memory operation is unknown, the pages before and after current page
> +  // should be taken into account as well.
> +  //
> +  RestorePageNumber = 3;
> +  RestoreAddress = PFAddress - EFI_PAGE_SIZE;
> +  while (RestorePageNumber > 0) {
> +    if (RestoreAddress <= 0xFFFFFFFF) {
> +      RestorePageTableBelow4G (PageTable, RestoreAddress, CpuIndex,
> ErrorCode);
> +    } else {
> +      RestorePageTableAbove4G (PageTable, RestoreAddress, CpuIndex,
> ErrorCode, &IsValidPFAddress);
> +    }
> +    RestoreAddress += EFI_PAGE_SIZE;
> +    RestorePageNumber--;
>    }
> 
>    if (!IsValidPFAddress) {
> --
> 2.14.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix infinite loop issue in SMM profile
Posted by Wang, Jian J 6 years, 10 months ago 
They're following 3 pages:

   | Page before | Page triggering #PF | Page after |


because all I need to do is to make sure that there's no non-present pages during
memory operation cross pages.

Regards,
Jian


> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, February 06, 2018 2:23 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Dong,
> Eric <eric.dong@intel.com>
> Subject: RE: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix infinite loop
> issue in SMM profile
> 
> Hi
> May I know how "3" is calculated?
> 
> +  RestorePageNumber = 3;
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian
> J
> > Wang
> > Sent: Tuesday, February 6, 2018 11:02 AM
> > To: edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Laszlo
> > Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>
> > Subject: [edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: fix infinite loop issue
> in
> > SMM profile
> >
> > The infinite loop is caused by the memory instruction, such as
> > "rep mov", operating on memory block crossing boundary of NON-PRESENT
> > pages. Because the address triggering page fault set in CR2 will be in
> > the first page, SmmProfilePFHandler() will only change the first page
> > into PRESENT. The page following will be still in NON-PRESENT status.
> >
> > Since SmmProfilePFHandler() will setup single-step trap for the
> > instruction causing #PF, when the handler returns back to the
> > instruction and re-execute it, both #DB and #PF will be triggered
> > because the instruction wants to access both first and second page
> > but only first page is PRESENT.
> >
> > Normally #DB exception will be handled first and its handler will
> > change first page back to NON-PRESENT status. Then #PF is handled
> > and its handler will change first page to PRESENT status again and
> > setup another single-step for the instruction triggering #PF. Then
> > the whole system falls into an infinite loop and the memory operation
> > will never move on.
> >
> > This patch fix above situation by always changing 3 pages to PRESENT
> > status instead of just 1 page. Those 3 pages include the page before
> > and after the page causing #PF because memory operation instruction
> > may have direction flag (DF) set or cleared.
> >
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 22 ++++++++++++++++++----
> >  1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> > b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> > index 9588eaf029..8cdfc82a92 100644
> > --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> > +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> > @@ -1302,6 +1302,8 @@ SmmProfilePFHandler (
> >  {
> >    UINT64                *PageTable;
> >    UINT64                PFAddress;
> > +  UINT64                RestoreAddress;
> > +  UINTN                 RestorePageNumber;
> >    UINTN                 CpuIndex;
> >    UINTN                 Index;
> >    UINT64                InstructionAddress;
> > @@ -1331,10 +1333,22 @@ SmmProfilePFHandler (
> >    PFAddress         = AsmReadCr2 ();
> >    CpuIndex          = GetCpuIndex ();
> >
> > -  if (PFAddress <= 0xFFFFFFFF) {
> > -    RestorePageTableBelow4G (PageTable, PFAddress, CpuIndex, ErrorCode);
> > -  } else {
> > -    RestorePageTableAbove4G (PageTable, PFAddress, CpuIndex, ErrorCode,
> > &IsValidPFAddress);
> > +  //
> > +  // Memory operation cross pages, like "rep mov" instruction, will cause
> > +  // infinite loop between this and Debug Trap handler. Since the direction
> > +  // of memory operation is unknown, the pages before and after current
> page
> > +  // should be taken into account as well.
> > +  //
> > +  RestorePageNumber = 3;
> > +  RestoreAddress = PFAddress - EFI_PAGE_SIZE;
> > +  while (RestorePageNumber > 0) {
> > +    if (RestoreAddress <= 0xFFFFFFFF) {
> > +      RestorePageTableBelow4G (PageTable, RestoreAddress, CpuIndex,
> > ErrorCode);
> > +    } else {
> > +      RestorePageTableAbove4G (PageTable, RestoreAddress, CpuIndex,
> > ErrorCode, &IsValidPFAddress);
> > +    }
> > +    RestoreAddress += EFI_PAGE_SIZE;
> > +    RestorePageNumber--;
> >    }
> >
> >    if (!IsValidPFAddress) {
> > --
> > 2.14.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.