kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: VMX: Forbid userspace MSR filters for x2APIC
@ 2020-10-19 17:05 Paolo Bonzini
  2020-10-19 17:35 ` Peter Xu
  2020-10-19 17:45 ` Graf (AWS), Alexander
  0 siblings, 2 replies; 9+ messages in thread
From: Paolo Bonzini @ 2020-10-19 17:05 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Alexander Graf, Aaron Lewis, Peter Xu, Sean Christopherson

Allowing userspace to intercept reads to x2APIC MSRs when APICV is
fully enabled for the guest simply can't work.   But more in general,
if the LAPIC is in-kernel, allowing accessed by userspace would be very
confusing.  If userspace wants to intercept x2APIC MSRs, then it should
first disable in-kernel APIC.

We could in principle allow userspace to intercept reads and writes to TPR,
and writes to EOI and SELF_IPI, but while that could be made it work, it
would still be silly.

Cc: Alexander Graf <graf@amazon.com>
Cc: Aaron Lewis <aaronlewis@google.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c4015a43cc8a..0faf733538f4 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5246,6 +5246,13 @@ static int kvm_add_msr_filter(struct kvm *kvm, struct kvm_msr_filter_range *user
 	return r;
 }
 
+static bool range_overlaps_x2apic(struct kvm_msr_filter_range *range)
+{
+	u32 start = range->base;
+	u32 end = start + range->nmsrs;
+	return start <= 0x8ff && end > 0x800;
+}
+
 static int kvm_vm_ioctl_set_msr_filter(struct kvm *kvm, void __user *argp)
 {
 	struct kvm_msr_filter __user *user_msr_filter = argp;
@@ -5257,6 +5264,16 @@ static int kvm_vm_ioctl_set_msr_filter(struct kvm *kvm, void __user *argp)
 	if (copy_from_user(&filter, user_msr_filter, sizeof(filter)))
 		return -EFAULT;
 
+	/*
+	 * In principle it would be possible to trap x2apic ranges
+	 * if !lapic_in_kernel.  This however would be complicated
+	 * because KVM_X86_SET_MSR_FILTER can be called before
+	 * KVM_CREATE_IRQCHIP or KVM_ENABLE_CAP.
+	 */
+	for (i = 0; i < ARRAY_SIZE(filter.ranges); i++)
+		if (range_overlaps_x2apic(&filter.ranges[i]))
+			return -EINVAL;
+
 	kvm_clear_msr_filter(kvm);
 
 	default_allow = !(filter.flags & KVM_MSR_FILTER_DEFAULT_DENY);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH] KVM: VMX: Forbid userspace MSR filters for x2APIC
@ 2020-10-21 21:36 Paolo Bonzini
  0 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2020-10-21 21:36 UTC (permalink / raw)
  To: linux-kernel, kvm; +Cc: graf, Aaron Lewis, Peter Xu, Sean Christopherson

Allowing userspace to intercept reads to x2APIC MSRs when APICV is
fully enabled for the guest simply can't work.   But more in general,
the LAPIC could be set to in-kernel after the MSR filter is setup
and allowing accesses by userspace would be very confusing.

We could in principle allow userspace to intercept reads and writes to TPR,
and writes to EOI and SELF_IPI, but while that could be made it work, it
would still be silly.

Cc: Alexander Graf <graf@amazon.com>
Cc: Aaron Lewis <aaronlewis@google.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Documentation/virt/kvm/api.rst | 18 ++++++++++--------
 arch/x86/kvm/x86.c             |  9 ++++++++-
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index bd94105f2960..9ece9a827a58 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4777,20 +4777,22 @@ specify whether a certain MSR access should be explicitly filtered for or not.
 If this ioctl has never been invoked, MSR accesses are not guarded and the
 default KVM in-kernel emulation behavior is fully preserved.
 
+Calling this ioctl with an empty set of ranges (all nmsrs == 0) disables MSR
+filtering. In that mode, ``KVM_MSR_FILTER_DEFAULT_DENY`` is invalid and causes
+an error.
+
 As soon as the filtering is in place, every MSR access is processed through
 the filtering except for accesses to the x2APIC MSRs (from 0x800 to 0x8ff);
 x2APIC MSRs are always allowed, independent of the ``default_allow`` setting,
 and their behavior depends on the ``X2APIC_ENABLE`` bit of the APIC base
 register.
 
-If a bit is within one of the defined ranges, read and write
-accesses are guarded by the bitmap's value for the MSR index. If it is not
-defined in any range, whether MSR access is rejected is determined by the flags
-field in the kvm_msr_filter struct: ``KVM_MSR_FILTER_DEFAULT_ALLOW`` and
-``KVM_MSR_FILTER_DEFAULT_DENY``.
-
-Calling this ioctl with an empty set of ranges (all nmsrs == 0) disables MSR
-filtering. In that mode, KVM_MSR_FILTER_DEFAULT_DENY no longer has any effect.
+If a bit is within one of the defined ranges, read and write accesses are
+guarded by the bitmap's value for the MSR index if the kind of access
+is included in the ``struct kvm_msr_filter_range`` flags.  If no range
+cover this particular access, the behavior is determined by the flags
+field in the kvm_msr_filter struct: ``KVM_MSR_FILTER_DEFAULT_ALLOW``
+and ``KVM_MSR_FILTER_DEFAULT_DENY``.
 
 Each bitmap range specifies a range of MSRs to potentially allow access on.
 The range goes from MSR index [base .. base+nmsrs]. The flags field
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 08cfb5e4bd07..0f02d0fe3abb 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5252,14 +5252,21 @@ static int kvm_vm_ioctl_set_msr_filter(struct kvm *kvm, void __user *argp)
 	struct kvm_msr_filter filter;
 	bool default_allow;
 	int r = 0;
+	bool empty = true;
 	u32 i;
 
 	if (copy_from_user(&filter, user_msr_filter, sizeof(filter)))
 		return -EFAULT;
 
-	kvm_clear_msr_filter(kvm);
+	for (i = 0; i < ARRAY_SIZE(filter.ranges); i++)
+		empty &= !filter.ranges[i].nmsrs;
 
 	default_allow = !(filter.flags & KVM_MSR_FILTER_DEFAULT_DENY);
+	if (empty && !default_allow)
+		return -EINVAL;
+
+	kvm_clear_msr_filter(kvm);
+
 	kvm->arch.msr_filter.default_allow = default_allow;
 
 	/*
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-10-21 21:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-19 17:05 [PATCH] KVM: VMX: Forbid userspace MSR filters for x2APIC Paolo Bonzini
2020-10-19 17:35 ` Peter Xu
2020-10-19 17:45 ` Graf (AWS), Alexander
2020-10-20  9:27   ` Paolo Bonzini
2020-10-20  9:48     ` Alexander Graf
2020-10-20 10:34       ` Paolo Bonzini
2020-10-20 10:52         ` Alexander Graf
2020-10-20 11:49           ` Paolo Bonzini
2020-10-21 21:36 Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).