diff mbox

[edk2,2/3] OvmfPkg: PlatformBdsLib: lock down SMM in PlatformBdsInit()

Message ID 1461784849-30809-3-git-send-email-lersek@redhat.com
State Accepted
Commit 70017e446125a454b6dc8f8fe6e4cfe5ff35b38e
Headers show

Commit Message

Laszlo Ersek April 27, 2016, 7:20 p.m. UTC 
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>

---
 OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 64 ++++++++++++--------
 1 file changed, 39 insertions(+), 25 deletions(-)

-- 
1.8.3.1


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

Patch

diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
index 0bc02baf0371..b22f2a74a9d8 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -85,12 +85,26 @@  VisitAllPciInstancesOfProtocol (
 
 VOID
 InstallDevicePathCallback (
   VOID
   );
 
+EFI_STATUS
+EFIAPI
+ConnectRootBridge (
+  IN EFI_HANDLE  RootBridgeHandle,
+  IN VOID        *Instance,
+  IN VOID        *Context
+  );
+
+STATIC
+VOID
+SaveS3BootScript (
+  VOID
+  );
+
 //
 // BDS Platform Functions
 //
 VOID
 EFIAPI
 PlatformBdsInit (
@@ -110,12 +124,37 @@  Returns:
   None.
 
 --*/
 {
   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 ();
+  }
 }
 
 
 EFI_STATUS
 EFIAPI
 ConnectRootBridge (
@@ -1239,37 +1278,12 @@  Returns:
 {
   EFI_STATUS                         Status;
   EFI_BOOT_MODE                      BootMode;
 
   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"));
   } else {
     //
     // Try to restore variables from the hard disk early so