From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 03D5E21DF8098 for ; Mon, 28 Aug 2017 05:36:55 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F0EA046269; Mon, 28 Aug 2017 12:39:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com F0EA046269 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-67.phx2.redhat.com [10.3.116.67]) by smtp.corp.redhat.com (Postfix) with ESMTP id CA1E36F9D5; Mon, 28 Aug 2017 12:39:32 +0000 (UTC) From: Laszlo Ersek To: edk2-devel-01 Cc: Brijesh Singh , Jordan Justen , Tom Lendacky Date: Mon, 28 Aug 2017 14:39:28 +0200 Message-Id: <20170828123928.14627-2-lersek@redhat.com> In-Reply-To: <20170828123928.14627-1-lersek@redhat.com> References: <20170828123928.14627-1-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 28 Aug 2017 12:39:34 +0000 (UTC) Subject: [PATCH 1/1] OvmfPkg/QemuFwCfgDxeLib: SEV: zero FW_CFG_DMA_ACCESS before decrypting it X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Aug 2017 12:36:55 -0000 There's a small window between - AllocFwCfgDmaAccessBuffer() mapping the new FW_CFG_DMA_ACCESS object for common buffer operation (i.e., decrypting it), and - InternalQemuFwCfgDmaBytes() setting the fields of the object. In this window, earlier garbage in the object is "leaked" to the hypervisor. So zero the object before we decrypt it. (This commit message references AMD SEV directly, because QemuFwCfgDxeLib is not *generally* enabled for IOMMU operation just yet, unlike our goal for the virtio infrastructure. Instead, QemuFwCfgDxeLib uses MemEncryptSevLib explicitly to detect SEV, and then relies on IOMMU protocol behavior that is specific to SEV. At this point, this is by design.) Cc: Brijesh Singh Cc: Jordan Justen Cc: Tom Lendacky Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c index 8b98208591e6..22077851a40c 100644 --- a/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c +++ b/OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c @@ -1,29 +1,30 @@ /** @file Stateful and implicitly initialized fw_cfg library implementation. Copyright (C) 2013, Red Hat, Inc. Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.
Copyright (c) 2017, Advanced Micro Devices. All rights reserved.
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at http://opensource.org/licenses/bsd-license.php THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. **/ #include #include #include +#include #include #include #include #include #include #include "QemuFwCfgLibInternal.h" @@ -155,75 +156,81 @@ VOID AllocFwCfgDmaAccessBuffer ( OUT VOID **Access, OUT VOID **MapInfo ) { UINTN Size; UINTN NumPages; EFI_STATUS Status; VOID *HostAddress; EFI_PHYSICAL_ADDRESS DmaAddress; VOID *Mapping; Size = sizeof (FW_CFG_DMA_ACCESS); NumPages = EFI_SIZE_TO_PAGES (Size); // // As per UEFI spec, in order to map a host address with // BusMasterCommomBuffer64, the buffer must be allocated using the IOMMU // AllocateBuffer() // Status = mIoMmuProtocol->AllocateBuffer ( mIoMmuProtocol, AllocateAnyPages, EfiBootServicesData, NumPages, &HostAddress, EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE ); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "%a:%a failed to allocate FW_CFG_DMA_ACCESS\n", gEfiCallerBaseName, __FUNCTION__)); ASSERT (FALSE); CpuDeadLoop (); } + // + // Avoid exposing stale data even temporarily: zero the area before mapping + // it. + // + ZeroMem (HostAddress, Size); + // // Map the host buffer with BusMasterCommonBuffer64 // Status = mIoMmuProtocol->Map ( mIoMmuProtocol, EdkiiIoMmuOperationBusMasterCommonBuffer64, HostAddress, &Size, &DmaAddress, &Mapping ); if (EFI_ERROR (Status)) { mIoMmuProtocol->FreeBuffer (mIoMmuProtocol, NumPages, HostAddress); DEBUG ((DEBUG_ERROR, "%a:%a failed to Map() FW_CFG_DMA_ACCESS\n", gEfiCallerBaseName, __FUNCTION__)); ASSERT (FALSE); CpuDeadLoop (); } if (Size < sizeof (FW_CFG_DMA_ACCESS)) { mIoMmuProtocol->Unmap (mIoMmuProtocol, Mapping); mIoMmuProtocol->FreeBuffer (mIoMmuProtocol, NumPages, HostAddress); DEBUG ((DEBUG_ERROR, "%a:%a failed to Map() - requested 0x%Lx got 0x%Lx\n", gEfiCallerBaseName, __FUNCTION__, (UINT64)sizeof (FW_CFG_DMA_ACCESS), (UINT64)Size)); ASSERT (FALSE); CpuDeadLoop (); } *Access = HostAddress; *MapInfo = Mapping; } /** Function is to used for freeing the Access buffer allocated using AllocFwCfgDmaAccessBuffer() **/ -- 2.14.1.3.gb7cf6e02401b