linux-kernel - Re: [PATCH 03/10] KVM: SVM: Terminate the VM if a SEV-ES+ guest is run with an invalid VMSA

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]

Message-ID: <76d50c20-6e00-33a4-9170-63ed7d29673b@amd.com>
Date: Tue, 25 Feb 2025 08:42:25 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
 linux-kernel@...r.kernel.org, Naveen N Rao <naveen@...nel.org>,
 Kim Phillips <kim.phillips@....com>, Alexey Kardashevskiy <aik@....com>
Subject: Re: [PATCH 03/10] KVM: SVM: Terminate the VM if a SEV-ES+ guest is
 run with an invalid VMSA

On 2/24/25 18:54, Sean Christopherson wrote:
> On Mon, Feb 24, 2025, Tom Lendacky wrote:
>> On 2/24/25 16:55, Sean Christopherson wrote:
>>> On Mon, Feb 24, 2025, Tom Lendacky wrote:
>>>> On 2/18/25 19:26, Sean Christopherson wrote:

> 
> Given that, IIUC, KVM would eventually return KVM_EXIT_FAIL_ENTRY, I like your
> idea of returning meaningful information.  And unless I'm missing something, that
> would obviate any need to terminate the VM, which would address your earlier point
> of whether terminating the VM is truly better than returning than returning a
> familiar error code.
> 
> So this? (completely untested)

This works nicely and qemu terminates quickly with:

KVM: entry failed, hardware error 0xffffffffffffffff

Thanks,
Tom

> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 7345cac6f93a..71b340cbe561 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -3463,10 +3463,8 @@ int pre_sev_run(struct vcpu_svm *svm, int cpu)
>          * invalid VMSA, e.g. if userspace forces the vCPU to be RUNNABLE after
>          * an SNP AP Destroy event.
>          */
> -       if (sev_es_guest(kvm) && !VALID_PAGE(svm->vmcb->control.vmsa_pa)) {
> -               kvm_vm_dead(kvm);
> -               return -EIO;
> -       }
> +       if (sev_es_guest(kvm) && !VALID_PAGE(svm->vmcb->control.vmsa_pa))
> +               return -EINVAL;
>  
>         /* Assign the asid allocated with this SEV guest */
>         svm->asid = asid;
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 46e0b65a9fec..f72bcf2e590e 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4233,8 +4233,12 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu,
>         if (force_immediate_exit)
>                 smp_send_reschedule(vcpu->cpu);
>  
> -       if (pre_svm_run(vcpu))
> +       if (pre_svm_run(vcpu)) {
> +               vcpu->run->exit_reason = KVM_EXIT_FAIL_ENTRY;
> +               vcpu->run->fail_entry.hardware_entry_failure_reason = SVM_EXIT_ERR;
> +               vcpu->run->fail_entry.cpu = vcpu->cpu;
>                 return EXIT_FASTPATH_EXIT_USERSPACE;
> +       }
>  
>         sync_lapic_to_cr8(vcpu);