From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web10.5073.1670540593743540835 for ; Thu, 08 Dec 2022 15:03:14 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=bfmnnu9a; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 5E8EAB8262B for ; Thu, 8 Dec 2022 23:03:11 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 94B08C433F2 for ; Thu, 8 Dec 2022 23:03:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1670540589; bh=CZtbc8Kl62j8t9BFQVCQgb3WZ6ivGtvcqpiiYEUXjf4=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=bfmnnu9a2wJ6SiFFxTpZtWoKsNeWZrhUeDnMr5sHECCIzzf+paAb4PMAqege1ZFML rShTKT9eTbYhRC93j8x76ejOzRJUEubJBhGA+bqc6Q63jDL4V2V5yPG5wiL4Id8kEx hBpRCsxx6hJ19RhqNmJ+lcP0J+ewqcrp8lYE3YiF/BgqoeAYmnrbxjtFAaqBSrIIOo x7giU+jLPXEvaQNunrBclbYW1P+jlAJyRh/ncBtSGy0TBTi8mDXVYT9KhoJy1ov8A+ N18hHQVzPDpoiJhTJ5mk1Wg6+3xCPJDMsDwZSf5fOq3PDLrHQo3JRzYNkgRB/3wBC5 2pV9CjpotTR4Q== Received: by mail-lf1-f53.google.com with SMTP id x28so4333884lfn.6 for ; Thu, 08 Dec 2022 15:03:09 -0800 (PST) X-Gm-Message-State: ANoB5pmKB3I90lhUjLFCaQ5CseUzObQC00wdGjfkAHXvnn+jXOF/6cGH U0wEC/pucEwT3BRCxGsKiujx1O+6mHhUTile28w= X-Google-Smtp-Source: AA0mqf7gLLdnTe0b25pSPOmBZ7G1rS611PygiuhHQE+r25gjLrCl6BjrXvcfWrQD4DHEZ+BQjeDcYHxzrO35iF2G0rQ= X-Received: by 2002:a05:6512:3c89:b0:4a2:bfd2:b218 with SMTP id h9-20020a0565123c8900b004a2bfd2b218mr31682256lfv.228.1670540587457; Thu, 08 Dec 2022 15:03:07 -0800 (PST) MIME-Version: 1.0 References: <20221208080311.2025737-1-dovmurik@linux.ibm.com> In-Reply-To: <20221208080311.2025737-1-dovmurik@linux.ibm.com> From: "Ard Biesheuvel" Date: Fri, 9 Dec 2022 00:02:55 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 1/1] OvmfPkg/AmdSev/SecretDxe: Allocate CC secret location as runtime memory To: Dov Murik Cc: devel@edk2.groups.io, Tobin Feldman-Fitzthum , Ard Biesheuvel , Erdem Aktas , Gerd Hoffmann , James Bottomley , Jiewen Yao , Jordan Justen , Min Xu , Tobin Feldman-Fitzthum , Tom Lendacky Content-Type: text/plain; charset="UTF-8" On Thu, 8 Dec 2022 at 09:08, Dov Murik wrote: > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4186 > > Commit 079a58276b98 ("OvmfPkg/AmdSev/SecretPei: Mark SEV launch secret > area as reserved") marked the launch secret area itself (1 page) as > reserved so it the guest OS can use it during the lifetime of the OS. > However, the address and size of the secret area held in the > CONFIDENTIAL_COMPUTING_SECRET_LOCATION struct are declared as STATIC in > OVMF (in AmdSev/SecretDxe); therefore there's no guarantee that it will > not be written over by OS data. > > Fix this by allocating the memory for the > CONFIDENTIAL_COMPUTING_SECRET_LOCATION struct with AllocateRuntimePool > to ensure the guest OS will not reuse this memory. > This memory type is mapped into the EFI page tables, and omitted from the linear map in Linux on arm64, so it is generally not the right type for data that only has significance to the OS. In spite of the name, EfiAcpiReclaimMemory is more suitable here - the OS is free to preserve it or treat it as ordinary memory. I realise that this is not of great importance here given that the table is only 8 bytes in size, but if we can, I'd prefer it if we use ACPI reclaim memory here. > Fixes: 079a58276b98 ("OvmfPkg/AmdSev/SecretPei: Mark SEV launch secret area as reserved") > Cc: Ard Biesheuvel > Cc: Erdem Aktas > Cc: Gerd Hoffmann > Cc: James Bottomley > Cc: Jiewen Yao > Cc: Jordan Justen > Cc: Min Xu > Cc: Tobin Feldman-Fitzthum > Cc: Tom Lendacky > Signed-off-by: Dov Murik > --- > OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf | 2 ++ > OvmfPkg/AmdSev/SecretDxe/SecretDxe.c | 17 +++++++++++------ > 2 files changed, 13 insertions(+), 6 deletions(-) > > diff --git a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf > index 40bda7ff846c..67d35f19b063 100644 > --- a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf > +++ b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.inf > @@ -23,6 +23,8 @@ [Packages] > MdePkg/MdePkg.dec > > [LibraryClasses] > + DebugLib > + MemoryAllocationLib > UefiBootServicesTableLib > UefiDriverEntryPoint > > diff --git a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c > index 3d84b2545052..615dff6cbf59 100644 > --- a/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c > +++ b/OvmfPkg/AmdSev/SecretDxe/SecretDxe.c > @@ -5,14 +5,11 @@ > SPDX-License-Identifier: BSD-2-Clause-Patent > **/ > #include > +#include > #include > +#include // AllocateRuntimePool() > #include > > -STATIC CONFIDENTIAL_COMPUTING_SECRET_LOCATION mSecretDxeTable = { > - FixedPcdGet32 (PcdSevLaunchSecretBase), > - FixedPcdGet32 (PcdSevLaunchSecretSize), > -}; > - > EFI_STATUS > EFIAPI > InitializeSecretDxe ( > @@ -20,8 +17,16 @@ InitializeSecretDxe ( > IN EFI_SYSTEM_TABLE *SystemTable > ) > { > + CONFIDENTIAL_COMPUTING_SECRET_LOCATION *SecretDxeTable; > + > + SecretDxeTable = AllocateRuntimePool (sizeof (CONFIDENTIAL_COMPUTING_SECRET_LOCATION)); > + ASSERT (SecretDxeTable != NULL); > + > + SecretDxeTable->Base = FixedPcdGet32 (PcdSevLaunchSecretBase); > + SecretDxeTable->Size = FixedPcdGet32 (PcdSevLaunchSecretSize); > + > return gBS->InstallConfigurationTable ( > &gConfidentialComputingSecretGuid, > - &mSecretDxeTable > + SecretDxeTable > ); > } > -- > 2.25.1 >