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>
This commit is contained in:
Laszlo Ersek 2016-04-27 17:37:42 +02:00
parent 058196bbb3
commit 70017e4461
1 changed files with 39 additions and 25 deletions

View File

@ -88,6 +88,20 @@ InstallDevicePathCallback (
VOID VOID
); );
EFI_STATUS
EFIAPI
ConnectRootBridge (
IN EFI_HANDLE RootBridgeHandle,
IN VOID *Instance,
IN VOID *Context
);
STATIC
VOID
SaveS3BootScript (
VOID
);
// //
// BDS Platform Functions // BDS Platform Functions
// //
@ -113,6 +127,31 @@ Returns:
{ {
DEBUG ((EFI_D_INFO, "PlatformBdsInit\n")); DEBUG ((EFI_D_INFO, "PlatformBdsInit\n"));
InstallDevicePathCallback (); 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")); 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)) { if (PcdGetBool (PcdOvmfFlashVariablesEnable)) {
DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior: not restoring NvVars " DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior: not restoring NvVars "
"from disk since flash variables appear to be supported.\n")); "from disk since flash variables appear to be supported.\n"));