linux-kernel - [RFC] Revert "kvm: nVMX: Restrict VMX capability MSR changes"

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-next>] [day] [month] [year] [list]

Date:   Mon, 20 Jan 2020 16:11:41 +0100
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>,
        Jim Mattson <jmattson@...gle.com>
Cc:     Sean Christopherson <sean.j.christopherson@...el.com>,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Liran Alon <liran.alon@...cle.com>
Subject: [RFC] Revert "kvm: nVMX: Restrict VMX capability MSR changes"

This reverts commit a943ac50d10aac96dca63d0460365a699d41fdd0.

Fine-grained VMX feature enablement in QEMU broke live migration with
nested guest:

 (qemu) qemu-kvm: error: failed to set MSR 0x48e to 0xfff9fffe04006172

The problem is that QEMU does KVM_SET_NESTED_STATE before KVM_SET_MSRS,
although it can probably be changed.

RFC. I think the check for vmx->nested.vmxon is legitimate for everything
but restore so removing it (what I do with the revert) is likely a no-go.
I'd like to gather opinions on the proper fix: should we somehow check
that the vCPU is in 'restore' start (has never being run) and make
KVM_SET_MSRS pass or should we actually mandate that KVM_SET_NESTED_STATE
is run after KVM_SET_MSRS by userspace?

Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
---
 arch/x86/kvm/vmx/nested.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 4aea7d304beb..bb8afe0c5e7f 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -1321,13 +1321,6 @@ int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);

-	/*
-	 * Don't allow changes to the VMX capability MSRs while the vCPU
-	 * is in VMX operation.
-	 */
-	if (vmx->nested.vmxon)
-		return -EBUSY;
-
 	switch (msr_index) {
 	case MSR_IA32_VMX_BASIC:
 		return vmx_restore_vmx_basic(vmx, data);
-- 
2.24.1