linux-kernel - Re: [PATCH v2 1/5] KVM: x86/xen: Restrict hypercall MSR to unofficial synthetic range

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

Message-ID: <DC438DC0-CC4B-4EE2-ABA8-8E0F9D15DD46@infradead.org>
Date: Sat, 15 Feb 2025 12:00:54 +0100
From: David Woodhouse <dwmw2@...radead.org>
To: Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini <pbonzini@...hat.com>,
 Paul Durrant <paul@....org>
CC: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 Joao Martins <joao.m.martins@...cle.com>,
 David Woodhouse <dwmw@...zon.co.uk>
Subject: Re: [PATCH v2 1/5] KVM: x86/xen: Restrict hypercall MSR to unofficial synthetic range

On 15 February 2025 02:14:33 CET, Sean Christopherson <seanjc@...gle.com> wrote:
>Reject userspace attempts to set the Xen hypercall page MSR to an index
>outside of the "standard" virtualization range [0x40000000, 0x4fffffff],
>as KVM is not equipped to handle collisions with real MSRs, e.g. KVM
>doesn't update MSR interception, conflicts with VMCS/VMCB fields, special
>case writes in KVM, etc.
>
>While the MSR index isn't strictly ABI, i.e. can theoretically float to
>any value, in practice no known VMM sets the MSR index to anything other
>than 0x40000000 or 0x40000200.
>
>Cc: Joao Martins <joao.m.martins@...cle.com>
>Reviewed-by: David Woodhouse <dwmw@...zon.co.uk>
>Reviewed-by: Paul Durrant <paul@....org>
>Signed-off-by: Sean Christopherson <seanjc@...gle.com>
>---
> arch/x86/include/uapi/asm/kvm.h | 3 +++
> arch/x86/kvm/xen.c              | 9 +++++++++
> 2 files changed, 12 insertions(+)
>
>diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
>index 9e75da97bce0..460306b35a4b 100644
>--- a/arch/x86/include/uapi/asm/kvm.h
>+++ b/arch/x86/include/uapi/asm/kvm.h
>@@ -559,6 +559,9 @@ struct kvm_x86_mce {
> #define KVM_XEN_HVM_CONFIG_PVCLOCK_TSC_UNSTABLE	(1 << 7)
> #define KVM_XEN_HVM_CONFIG_SHARED_INFO_HVA	(1 << 8)
> 
>+#define KVM_XEN_MSR_MIN_INDEX			0x40000000u
>+#define KVM_XEN_MSR_MAX_INDEX			0x4fffffffu
>+
> struct kvm_xen_hvm_config {
> 	__u32 flags;
> 	__u32 msr;
>diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
>index a909b817b9c0..5b94825001a7 100644
>--- a/arch/x86/kvm/xen.c
>+++ b/arch/x86/kvm/xen.c
>@@ -1324,6 +1324,15 @@ int kvm_xen_hvm_config(struct kvm *kvm, struct kvm_xen_hvm_config *xhc)
> 	     xhc->blob_size_32 || xhc->blob_size_64))
> 		return -EINVAL;
> 
>+	/*
>+	 * Restrict the MSR to the range that is unofficially reserved for
>+	 * synthetic, virtualization-defined MSRs, e.g. to prevent confusing
>+	 * KVM by colliding with a real MSR that requires special handling.
>+	 */
>+	if (xhc->msr &&
>+	    (xhc->msr < KVM_XEN_MSR_MIN_INDEX || xhc->msr > KVM_XEN_MSR_MAX_INDEX))
>+		return -EINVAL;
>+
> 	mutex_lock(&kvm->arch.xen.xen_lock);
> 
> 	if (xhc->msr && !kvm->arch.xen_hvm_config.msr)

I'd still like to restrict this to ensure it doesn't collide with MSRs that KVM expects to emulate. But that can be a separate patch, as discussed.

This patch should probably have a docs update too.