OvmfPkg: PlatformBdsLib: lock down SMM in PlatformBdsInit()

OVMF's PlatformBdsLib currently makes SMM vulnerable to the following
attack:

(1) a malicious guest OS copies a UEFI driver module to the EFI system
    partition,

(2) the OS adds the driver as a Driver#### option, and references it from
    DriverOrder,

(3) at next boot, the BdsEntry() function in
    "IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c" processes
    Driver#### and DriverOrder between the calls to PlatformBdsInit() and
    PlatformBdsPolicyBehavior(),

(4) OVMF locks down SMM only in PlatformBdsPolicyBehavior(), hence the
    driver runs with SMM unlocked.

The BdsEntry() function of the MdeModulePkg BDS driver (in file
"MdeModulePkg/Universal/BdsDxe/BdsEntry.c") recommends to "Signal
ReadyToLock event" in PlatformBootManagerBeforeConsole() -- which
corresponds to PlatformBdsInit() --, not in
PlatformBootManagerAfterConsole() -- which corresponds to
PlatformBdsPolicyBehavior().

Albeit an independent question, but it's worth mentioning: this patch also
brings OvmfPkg's PlatformBdsInit() closer to ArmVirtPkg's. Namely, the
latter signals End-of-Dxe in PlatformBdsInit() already.

Cc: Feng Tian <feng.tian@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Acked-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

1 files changed, 39 insertions, 25 deletions

diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
index 0bc02baf03..b22f2a74a9 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -88,6 +88,20 @@ InstallDevicePathCallback (
   VOID

   );

 

+EFI_STATUS

+EFIAPI

+ConnectRootBridge (

+  IN EFI_HANDLE  RootBridgeHandle,

+  IN VOID        *Instance,

+  IN VOID        *Context

+  );

+

+STATIC

+VOID

+SaveS3BootScript (

+  VOID

+  );

+

 //

 // BDS Platform Functions

 //

@@ -113,6 +127,31 @@ Returns:
 {

   DEBUG ((EFI_D_INFO, "PlatformBdsInit\n"));

   InstallDevicePathCallback ();

+

+  VisitAllInstancesOfProtocol (&gEfiPciRootBridgeIoProtocolGuid,

+    ConnectRootBridge, NULL);

+

+  //

+  // Signal the ACPI platform driver that it can download QEMU ACPI tables.

+  //

+  EfiEventGroupSignal (&gRootBridgesConnectedEventGroupGuid);

+

+  //

+  // We can't signal End-of-Dxe earlier than this. Namely, End-of-Dxe triggers

+  // the preparation of S3 system information. That logic has a hard dependency

+  // on the presence of the FACS ACPI table. Since our ACPI tables are only

+  // installed after PCI enumeration completes, we must not trigger the S3 save

+  // earlier, hence we can't signal End-of-Dxe earlier.

+  //

+  EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);

+

+  if (QemuFwCfgS3Enabled ()) {

+    //

+    // Save the boot script too. Note that this requires/includes emitting the

+    // DxeSmmReadyToLock event, which in turn locks down SMM.

+    //

+    SaveS3BootScript ();

+  }

 }

 

 

@@ -1242,31 +1281,6 @@ Returns:
 

   DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior\n"));

 

-  VisitAllInstancesOfProtocol (&gEfiPciRootBridgeIoProtocolGuid,

-    ConnectRootBridge, NULL);

-

-  //

-  // Signal the ACPI platform driver that it can download QEMU ACPI tables.

-  //

-  EfiEventGroupSignal (&gRootBridgesConnectedEventGroupGuid);

-

-  //

-  // We can't signal End-of-Dxe earlier than this. Namely, End-of-Dxe triggers

-  // the preparation of S3 system information. That logic has a hard dependency

-  // on the presence of the FACS ACPI table. Since our ACPI tables are only

-  // installed after PCI enumeration completes, we must not trigger the S3 save

-  // earlier, hence we can't signal End-of-Dxe earlier.

-  //

-  EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);

-

-  if (QemuFwCfgS3Enabled ()) {

-    //

-    // Save the boot script too. Note that this requires/includes emitting the

-    // DxeSmmReadyToLock event, which in turn locks down SMM.

-    //

-    SaveS3BootScript ();

-  }

-

   if (PcdGetBool (PcdOvmfFlashVariablesEnable)) {

     DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior: not restoring NvVars "

       "from disk since flash variables appear to be supported.\n"));