[PATCH] MdeModulePkg/DxeCapsuleLibFmp: avoid ESRT accesses at runtime

[PATCH] MdeModulePkg/DxeCapsuleLibFmp: avoid ESRT accesses at runtime

[Ard Biesheuvel]

The DxeCapsuleLibFmp code accesses the ESRT table to decide whether
a certain capsule is an FMP capsule. Since the UEFI spec mandates
that the ESRT resides in EfiBootServicesData memory, this results
in problems at OS runtime, since the firmware implementation itself
cannot access memory that has not been virtually remapped.

Since we are only interested in the GUIDs, let's cache those at
ReadyToBoot so that we retain access to them even after the address
space has been virtually remapped.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Build tested only.

 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c          | 18 ++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c      | 81 +++++++++++++++-----
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf |  1 +
 3 files changed, 70 insertions(+), 30 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
index f38ab69e38fb..24ff6f420edb 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.c
@@ -41,8 +41,8 @@
 #include <Protocol/FirmwareManagementProgress.h>
 #include <Protocol/DevicePath.h>
 
-EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable                  = NULL;
-BOOLEAN                   mIsVirtualAddrConverted      = FALSE;
+EFI_GUID                  *mFmpGuidList;
+UINTN                     mFmpGuidCount;
 
 BOOLEAN                   mDxeCapsuleLibEndOfDxe       = FALSE;
 EFI_EVENT                 mDxeCapsuleLibEndOfDxeEvent  = NULL;
@@ -1298,7 +1298,6 @@ IsNestedFmpCapsule (
   )
 {
   EFI_STATUS                 Status;
-  EFI_SYSTEM_RESOURCE_ENTRY  *EsrtEntry;
   UINTN                      Index;
   BOOLEAN                    EsrtGuidFound;
   EFI_CAPSULE_HEADER         *NestedCapsuleHeader;
@@ -1307,14 +1306,11 @@ IsNestedFmpCapsule (
   EFI_SYSTEM_RESOURCE_ENTRY  Entry;
 
   EsrtGuidFound = FALSE;
-  if (mIsVirtualAddrConverted) {
-    if(mEsrtTable != NULL) {
-      EsrtEntry = (EFI_SYSTEM_RESOURCE_ENTRY *)(mEsrtTable + 1);
-      for (Index = 0; Index < mEsrtTable->FwResourceCount ; Index++, EsrtEntry++) {
-        if (CompareGuid(&EsrtEntry->FwClass, &CapsuleHeader->CapsuleGuid)) {
-          EsrtGuidFound = TRUE;
-          break;
-        }
+  if (mFmpGuidCount > 0) {
+    for (Index = 0; Index < mFmpGuidCount; Index++) {
+      if (CompareGuid (mFmpGuidList + Index, &CapsuleHeader->CapsuleGuid)) {
+        EsrtGuidFound = TRUE;
+        break;
       }
     }
   } else {
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
index 602921d13c06..e75e78202045 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleRuntime.c
@@ -20,9 +20,10 @@
 #include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/MemoryAllocationLib.h>
 
-extern EFI_SYSTEM_RESOURCE_TABLE *mEsrtTable;
-extern BOOLEAN                   mIsVirtualAddrConverted;
+extern EFI_GUID           *mFmpGuidList;
+extern UINTN              mFmpGuidCount;
 EFI_EVENT                 mDxeRuntimeCapsuleLibVirtualAddressChangeEvent  = NULL;
+EFI_EVENT                 mDxeRuntimeCapsuleLibReadyToBootEvent  = NULL;
 
 /**
   Convert EsrtTable physical address to virtual address.
@@ -38,37 +39,60 @@ DxeCapsuleLibVirtualAddressChangeEvent (
   IN  VOID        *Context
   )
 {
-  UINTN                    Index;
-  EFI_CONFIGURATION_TABLE  *ConfigEntry;
+  gRT->ConvertPointer (0x0, (VOID **)&mFmpGuidList);
+}
+
+/**
+  Notify function for event group EFI_EVENT_GROUP_READY_TO_BOOT. This is used to
+  install the Esrt Table into system configuration table
+
+  @param[in]  Event   The Event that is being processed.
+  @param[in]  Context The Event Context.
+
+**/
+STATIC
+VOID
+EFIAPI
+DxeCapsuleLibReadyToBootEventNotify (
+  IN EFI_EVENT        Event,
+  IN VOID             *Context
+  )
+{
+  UINTN                       Index;
+  EFI_CONFIGURATION_TABLE     *ConfigEntry;
+  EFI_SYSTEM_RESOURCE_TABLE   *EsrtTable;
+  EFI_SYSTEM_RESOURCE_ENTRY   *EsrtEntry;
 
   //
   // Get Esrt table first
   //
   ConfigEntry = gST->ConfigurationTable;
-  for (Index = 0; Index < gST->NumberOfTableEntries; Index++) {
-    if (CompareGuid(&gEfiSystemResourceTableGuid, &ConfigEntry->VendorGuid)) {
+  EsrtTable = NULL;
+  for (Index = 0; Index < gST->NumberOfTableEntries; Index++, ConfigEntry++) {
+    if (CompareGuid (&gEfiSystemResourceTableGuid, &ConfigEntry->VendorGuid)) {
+      EsrtTable = (EFI_SYSTEM_RESOURCE_TABLE *)ConfigEntry->VendorTable;
       break;
     }
-    ConfigEntry++;
   }
 
-  //
-  // If no Esrt table installed in Configure Table
-  //
-  if (Index < gST->NumberOfTableEntries) {
-    //
-    // Search Esrt to check given capsule is qualified
-    //
-    mEsrtTable = (EFI_SYSTEM_RESOURCE_TABLE *) ConfigEntry->VendorTable;
-
+  if (EsrtTable == NULL) {
     //
-    // Update protocol pointer to Esrt Table.
+    // No ESRT table was found - close the VA change event, there will
+    // be nothing to convert.
     //
-    gRT->ConvertPointer (0x00, (VOID**) &(mEsrtTable));
+    gBS->CloseEvent (mDxeRuntimeCapsuleLibVirtualAddressChangeEvent);
+    return;
   }
 
-  mIsVirtualAddrConverted = TRUE;
+  mFmpGuidCount = EsrtTable->FwResourceCount;
+  mFmpGuidList = AllocateRuntimePool (mFmpGuidCount * sizeof(EFI_GUID));
 
+  ASSERT (mFmpGuidList != NULL);
+
+  EsrtEntry = (EFI_SYSTEM_RESOURCE_ENTRY *)(EsrtTable + 1);
+  for (Index = 0; Index < mFmpGuidCount; Index++, EsrtEntry++) {
+    CopyGuid (mFmpGuidList + Index, &EsrtEntry->FwClass);
+  }
 }
 
 /**
@@ -101,6 +125,19 @@ DxeRuntimeCapsuleLibConstructor (
                   );
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // Register notify function to cache the FMP capsule GUIDs at ReadyToBoot.
+  //
+  Status = gBS->CreateEventEx (
+                  EVT_NOTIFY_SIGNAL,
+                  TPL_CALLBACK,
+                  DxeCapsuleLibReadyToBootEventNotify,
+                  NULL,
+                  &gEfiEventReadyToBootGuid,
+                  &mDxeRuntimeCapsuleLibReadyToBootEvent
+                  );
+  ASSERT_EFI_ERROR (Status);
+
   return EFI_SUCCESS;
 }
 
@@ -127,5 +164,11 @@ DxeRuntimeCapsuleLibDestructor (
   Status = gBS->CloseEvent (mDxeRuntimeCapsuleLibVirtualAddressChangeEvent);
   ASSERT_EFI_ERROR (Status);
 
+  //
+  // Close the ReadyToBoot event.
+  //
+  Status = gBS->CloseEvent (mDxeRuntimeCapsuleLibReadyToBootEvent);
+  ASSERT_EFI_ERROR (Status);
+
   return EFI_SUCCESS;
 }
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
index 700d0d5dcddd..2c93e6870023 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
@@ -66,6 +66,7 @@
   gEfiCapsuleReportGuid
   gEfiCapsuleVendorGuid                   ## SOMETIMES_CONSUMES ## Variable:L"CapsuleUpdateData"
   gEfiEndOfDxeEventGroupGuid              ## CONSUMES ## Event
+  gEfiEventReadyToBootGuid                ## CONSUMES ## Event
   gEfiEventVirtualAddressChangeGuid       ## CONSUMES ## Event
 
 [Depex]
-- 
2.20.1