[PATCH v2 0/3] Fix bugs in core ResetSystem software stack

[PATCH v2 0/3] Fix bugs in core ResetSystem software stack

[Ruiyu Ni]

v2:
  Fix the comments in 2/3 to solve Laszlo's concern.
  Change DxeResetSystemLib to disallow to be linked by runtime/smm drivers.
  

Ruiyu Ni (3):
  MdePkg/UefiRuntimeLib: Do not allow to be linked by DXE driver
  MdeModulePkg/DxeResetSystemLib: Avoid depending on UefiRuntimeLib
  MdeModulePkg: Make sure ResetSystemRuntimeDxe uses ResetSystemLibNull

 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c   | 10 +++++-----
 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf |  5 ++---
 MdeModulePkg/MdeModulePkg.dsc                                |  5 ++++-
 MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf             |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

-- 
2.16.1.windows.1

[PATCH v2 1/3] MdePkg/UefiRuntimeLib: Do not allow to be linked by DXE driver

[Ruiyu Ni]

When UefiRuntimeLib links to a DXE driver, its constructor
still registers a Virtual Address Change event. The event callback
will get called when RT.SetVirtualAddressMap() is called from OS.
But when the driver is a DXE driver, the memory occupied by the
callback function might be zeroed or used by OS since the BS type
memory is free memory when entering to RT phase.

The patch reverts commit 97511979b4fdd84cf7cd51e43c22dc03e79bd4f3
"MdePkg/UefiRuntimeLib: Support more module types."
It makes sure that DXE driver cannot link to this library.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf b/MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
index d053da545a..8f46495fc5 100644
--- a/MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
+++ b/MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
@@ -5,7 +5,7 @@
 #  EVT_SIGNAL_EXIT_BOOT_SERVICES event, to provide runtime services.
 # This instance also supports SAL drivers for better performance.
 #
-# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD License
@@ -24,7 +24,7 @@ [Defines]
   FILE_GUID                      = b1ee6c28-54aa-4d17-b705-3e28ccb27b2e
   MODULE_TYPE                    = DXE_RUNTIME_DRIVER
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = UefiRuntimeLib|DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_CORE DXE_DRIVER DXE_SMM_DRIVER 
+  LIBRARY_CLASS                  = UefiRuntimeLib|DXE_RUNTIME_DRIVER DXE_SAL_DRIVER
 
   CONSTRUCTOR                    = RuntimeDriverLibConstruct
   DESTRUCTOR                     = RuntimeDriverLibDeconstruct
-- 
2.16.1.windows.1

[PATCH v2 2/3] MdeModulePkg/DxeResetSystemLib: Avoid depending on UefiRuntimeLib

[Ruiyu Ni]

Current DxeResetSystemLib depends on UefiRuntimeLib because it calls
EfiResetSystem() API exposed by UefiRuntimeLib.

Due to the commit:
"MdePkg/UefiRuntimeLib: Do not allow to be linked by DXE driver"
which reverts UefiRuntimeLib to only support DXE_RUNTIME_DRIVER,
removing UefiRuntimeLib dependency makes the DxeResetSystemLib
can be used by DXE drivers.

The patch also disallows the DxeResetSystemLib to be linked by
runtime driver, SMM drivers.
Runtime driver cannot link to this library because the gRT is not
converted when entering to RT.
SMM driver cannot link to this library because calling RT services
from SMM after EndOfDxe violates security guideline.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
---
 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c   | 10 +++++-----
 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf |  5 ++---
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
index ea452e3231..76bca223ae 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
@@ -14,7 +14,7 @@
 
 #include <PiDxe.h>
 #include <Library/ResetSystemLib.h>
-#include <Library/UefiRuntimeLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
 
 /**
   This function causes a system-wide reset (cold reset), in which
@@ -30,7 +30,7 @@ ResetCold (
   VOID
   )
 {
-  EfiResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
+  gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
 }
 
 /**
@@ -45,7 +45,7 @@ ResetWarm (
   VOID
   )
 {
-  EfiResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL);
+  gRT->ResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL);
 }
 
 /**
@@ -60,7 +60,7 @@ ResetShutdown (
   VOID
   )
 {
-  EfiResetSystem (EfiResetShutdown, EFI_SUCCESS, 0, NULL);
+  gRT->ResetSystem (EfiResetShutdown, EFI_SUCCESS, 0, NULL);
 }
 
 /**
@@ -94,5 +94,5 @@ ResetPlatformSpecific (
   IN VOID    *ResetData
   )
 {
-  EfiResetSystem (EfiResetPlatformSpecific, EFI_SUCCESS, DataSize, ResetData);
+  gRT->ResetSystem (EfiResetPlatformSpecific, EFI_SUCCESS, DataSize, ResetData);
 }
diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
index 6eb2766b93..5cd52d8859 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
@@ -19,7 +19,7 @@ [Defines]
   FILE_GUID                      = C2BDE4F6-65EE-440B-87B5-83ABF10EF45B
   MODULE_TYPE                    = DXE_DRIVER
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = ResetSystemLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS                  = ResetSystemLib|DXE_CORE DXE_DRIVER UEFI_APPLICATION UEFI_DRIVER
 
 #
 # The following information is for reference only and not required by the build tools.
@@ -35,5 +35,4 @@ [Packages]
   MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
-  UefiRuntimeLib
-
+  UefiRuntimeServicesTableLib
-- 
2.16.1.windows.1

[PATCH v2 3/3] MdeModulePkg: Make sure ResetSystemRuntimeDxe uses ResetSystemLibNull

[Ruiyu Ni]

Because the DxeResetSystemLib calls gRT->ResetSystem(), make sure
the gRT->ResetSystem() implementation doesn't call into
DxeResetSystemLib to avoid chicken-egg issue.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
---
 MdeModulePkg/MdeModulePkg.dsc | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index ec24a50c7d..734c5e5ab7 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -363,7 +363,10 @@ [Components]
     <LibraryClasses>
       ResetSystemLib|MdeModulePkg/Library/BaseResetSystemLibNull/BaseResetSystemLibNull.inf
   }
-  MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+  MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf {
+    <LibraryClasses>
+      ResetSystemLib|MdeModulePkg/Library/BaseResetSystemLibNull/BaseResetSystemLibNull.inf
+  }
   MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
   MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf
 
-- 
2.16.1.windows.1

Re: [PATCH v2 0/3] Fix bugs in core ResetSystem software stack

[Zeng, Star]

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Monday, June 4, 2018 2:10 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2 0/3] Fix bugs in core ResetSystem software stack

v2:
  Fix the comments in 2/3 to solve Laszlo's concern.
  Change DxeResetSystemLib to disallow to be linked by runtime/smm drivers.
  

Ruiyu Ni (3):
  MdePkg/UefiRuntimeLib: Do not allow to be linked by DXE driver
  MdeModulePkg/DxeResetSystemLib: Avoid depending on UefiRuntimeLib
  MdeModulePkg: Make sure ResetSystemRuntimeDxe uses ResetSystemLibNull

 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c   | 10 +++++-----
 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf |  5 ++---
 MdeModulePkg/MdeModulePkg.dsc                                |  5 ++++-
 MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf             |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

-- 
2.16.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel