linux-kernel - Re: [PATCH RFC 3/8] KVM: SVM: write back corrected CPUID page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 27 Jan 2023 16:46:31 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Jarkko Sakkinen <jarkko@...fian.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>
Cc:     Harald Hoyer <harald@...fian.com>, Tom Dohrmann <erbse.13@....de>,
        Ashish Kalra <ashish.kalra@....com>,
        Michael Roth <michael.roth@....com>,
        "open list:KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86)" 
        <kvm@...r.kernel.org>,
        "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" 
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC 3/8] KVM: SVM: write back corrected CPUID page

On 1/26/23 20:52, Jarkko Sakkinen wrote:
> From: Tom Dohrmann <erbse.13@....de>
> 
> When doing a launch update for a CPUID page the firmware checks that the values
> conform to the policy laid out in the processor programming manual. If the
> values don't conform, the firmware will return SEV_RET_INVALID_PARAM.
> In addition to returning an error the firmware will choose some acceptable
> values and write them back to the page that was used for the launch update, so
> that the VMM can inspect the changes and try again with the corrected values.
> This is specified in section 8.17.2.6 in the SEV-SNP Firmware ABI spec.
> Because launch updates are always done on the private UPM mappings, the pages
> are first copied from the shared mappings to the private mappings. When the
> firmware corrects the values, the corrected values are in the private mappings,
> inaccessible to userspace. In order to make the corrected values accessible to
> userspace, the page containing them must be copied from the private mappings
> back to the shared mappings.
> 
> [jarkko@...fian.com: fixed checkpatch.pl errors]
> Link: https://lore.kernel.org/lkml/Y76%2FI6Nrh7xEAAwv@notebook/
> Signed-off-by: Tom Dohrmann <erbse.13@....de>
> Signed-off-by: Jarkko Sakkinen <jarkko@...fian.com>
> ---
>   arch/x86/kvm/svm/sev.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 6d3162853c33..4a8e552d8cfe 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2230,6 +2230,23 @@ static int snp_launch_update_gfn_handler(struct kvm *kvm,
>   			pr_err("SEV-SNP launch update failed, ret: 0x%x, fw_error: 0x%x\n",
>   			       ret, *error);
>   			snp_page_reclaim(pfns[i]);
> +
> +			/*
> +			 * When invalid CPUID function entries are detected, the firmware
> +			 * corrects these entries.  In that case write the page back to
> +			 * userspace.

I would additionally add that the firmware does not encrypt the page, 
which allows the hypervisor to copy the page back to userspace.

Thanks,
Tom

> +			 */
> +			if (params.page_type == SNP_PAGE_TYPE_CPUID &&
> +			    *error == SEV_RET_INVALID_PARAM) {
> +				int ret;
> +
> +				host_rmp_make_shared(pfns[i], PG_LEVEL_4K, true);
> +
> +				ret = kvm_write_guest_page(kvm, gfn, kvaddr, 0, PAGE_SIZE);
> +				if (ret)
> +					pr_err("Guest write failed, ret: 0x%x\n", ret);
> +			}
> +
>   			goto e_release;
>   		}
>   	}