linux-kernel - Re: [PATCH v3 12/13] KVM: x86: SVM: don't save SVM state to SMRAM when VM is not long mode capable

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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 24 Aug 2022 22:58:14 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>
Cc:     kvm@...r.kernel.org, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        linux-kernel@...r.kernel.org, Wanpeng Li <wanpengli@...cent.com>,
        Ingo Molnar <mingo@...hat.com>, x86@...nel.org,
        Jim Mattson <jmattson@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>, Joerg Roedel <joro@...tes.org>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v3 12/13] KVM: x86: SVM: don't save SVM state to SMRAM
 when VM is not long mode capable

On Wed, Aug 03, 2022, Maxim Levitsky wrote:
> When the guest CPUID doesn't have support for long mode, 32 bit SMRAM
> layout is used and it has no support for preserving EFER and/or SVM
> state.
> 
> Note that this isn't relevant to running 32 bit guests on VM which is
> long mode capable - such VM can still run 32 bit guests in compatibility
> mode.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
> ---
>  arch/x86/kvm/svm/svm.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 7ca5e06878e19a..64cfd26bc5e7a6 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4442,6 +4442,15 @@ static int svm_enter_smm(struct kvm_vcpu *vcpu, union kvm_smram *smram)
>  	if (!is_guest_mode(vcpu))
>  		return 0;
>  
> +	/*
> +	 * 32 bit SMRAM format doesn't preserve EFER and SVM state.
> +	 * SVM should not be enabled by the userspace without marking
> +	 * the CPU as at least long mode capable.

Hmm, or userspace can ensure SMIs never get delivered.  Maybe?

	/*
	 * 32-bit SMRAM format doesn't preserve EFER and SVM state.  Userspace is
	 * responsible for ensuring nested SVM and SMIs are mutually exclusive.
	 */

> +	 */
> +

Unnecessary newline.

> +	if (!guest_cpuid_has(vcpu, X86_FEATURE_LM))
> +		return 1;

This doesn't actually fix anything,  RSM will still jump to L2 state but in L1
context.  I think we first need to actually handle errors from
static_call(kvm_x86_enter_smm).

Given that SVM can't even guarantee nested_svm_simple_vmexit() succeeds, i.e. KVM
can't force the vCPU out of L2 to ensure triple fault would hit L1, killing the VM
seems like the least awful solution (and it's still quite awful).

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 54fa0aa95785..38a6f4089296 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9985,7 +9985,10 @@ static void enter_smm(struct kvm_vcpu *vcpu)
         * state (e.g. leave guest mode) after we've saved the state into the
         * SMM state-save area.
         */
-       static_call(kvm_x86_enter_smm)(vcpu, &smram);
+       if (static_call(kvm_x86_enter_smm)(vcpu, &smram)) {
+               kvm_vm_dead(vcpu->vm);
+               return;
+       }

        kvm_smm_changed(vcpu, true);
        kvm_vcpu_write_guest(vcpu, vcpu->arch.smbase + 0xfe00, &smram, sizeof(smram));

> +
>  	smram->smram64.svm_guest_flag = 1;
>  	smram->smram64.svm_guest_vmcb_gpa = svm->nested.vmcb12_gpa;
>  
> -- 
> 2.26.3
>