linux-kernel - [RFC PATCH] KVM: x86: Disallow KVM_SET_CPUID{2} if the vCPU is in guest mode

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

Date:   Wed, 18 Dec 2019 09:42:55 -0800
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Sean Christopherson <sean.j.christopherson@...el.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Weijiang Yang <weijiang.yang@...el.com>
Subject: [RFC PATCH] KVM: x86: Disallow KVM_SET_CPUID{2} if the vCPU is in guest mode

Reject KVM_SET_CPUID{2} with -EBUSY if the vCPU is in guest mode (L2) to
avoid complications and potentially undesirable KVM behavior.  Allowing
userspace to change a guest's capabilities while L2 is active would at
best result in unexpected behavior in the guest (L1 or L2), and at worst
induce bad KVM behavior by breaking fundamental assumptions regarding
transitions between L0, L1 and L2.

Cc: Jim Mattson <jmattson@...gle.com>
Cc: Weijiang Yang <weijiang.yang@...el.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@...el.com>
---

This came up in the context of the CET series, where passing through MSRs
to L1 depends on the CPUID-based capabilities of the guest[*].  The CET
problem is solvable, but IMO unnecessarily complex.   And I'm more
concerned that userspace would be able to induce bad behavior in KVM by
changing core capabilites while L2 is active, e.g. VMX, LM, LA57, etc...

Tagged RFC as this is an ABI change, though I highly doubt it actually
affects a real world VMM.

[*] https://lkml.kernel.org/r/20191218160228.GB25201@linux.intel.com/

 arch/x86/kvm/x86.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8bb2fb1705ff..974983140e42 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4189,6 +4189,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		struct kvm_cpuid __user *cpuid_arg = argp;
 		struct kvm_cpuid cpuid;

+		r = -EBUSY;
+		if (is_guest_mode(vcpu))
+			goto out;
+
 		r = -EFAULT;
 		if (copy_from_user(&cpuid, cpuid_arg, sizeof(cpuid)))
 			goto out;
@@ -4199,6 +4203,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		struct kvm_cpuid2 __user *cpuid_arg = argp;
 		struct kvm_cpuid2 cpuid;

+		r = -EBUSY;
+		if (is_guest_mode(vcpu))
+			goto out;
+
 		r = -EFAULT;
 		if (copy_from_user(&cpuid, cpuid_arg, sizeof(cpuid)))
 			goto out;
-- 
2.24.1