From patchwork Wed Apr 27 19:20:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laszlo Ersek X-Patchwork-Id: 66830 Delivered-To: patch@linaro.org Received: by 10.140.93.198 with SMTP id d64csp2366743qge; Wed, 27 Apr 2016 12:21:05 -0700 (PDT) X-Received: by 10.66.100.197 with SMTP id fa5mr14221526pab.25.1461784861747; Wed, 27 Apr 2016 12:21:01 -0700 (PDT) Return-Path: Received: from ml01.01.org (ml01.01.org. [2001:19d0:306:5::1]) by mx.google.com with ESMTPS id wg1si6767881pab.52.2016.04.27.12.21.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 27 Apr 2016 12:21:01 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) client-ip=2001:19d0:306:5::1; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 439981A1F25; Wed, 27 Apr 2016 12:21:01 -0700 (PDT) X-Original-To: edk2-devel@ml01.01.org Delivered-To: edk2-devel@ml01.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2C0E31A1E27 for ; Wed, 27 Apr 2016 12:21:00 -0700 (PDT) Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B831B7F6A6; Wed, 27 Apr 2016 19:20:59 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-86.phx2.redhat.com [10.3.113.86]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u3RJKqTd023137; Wed, 27 Apr 2016 15:20:58 -0400 From: Laszlo Ersek To: edk2-devel-01 Date: Wed, 27 Apr 2016 21:20:48 +0200 Message-Id: <1461784849-30809-3-git-send-email-lersek@redhat.com> In-Reply-To: <1461784849-30809-1-git-send-email-lersek@redhat.com> References: <1461784849-30809-1-git-send-email-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 Subject: [edk2] [PATCH 2/3] OvmfPkg: PlatformBdsLib: lock down SMM in PlatformBdsInit() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Ruiyu Ni , Jordan Justen , Feng Tian , Jiewen Yao , Star Zeng MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" 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 Cc: Jiewen Yao Cc: Jordan Justen Cc: Ruiyu Ni Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- 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 --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