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: <CB09D020-703C-41D2-8F1F-32BF776BE1DB@infradead.org>
Date: Tue, 18 Feb 2025 19:47:27 +0100
From: David Woodhouse <dwmw2@...radead.org>
To: Sean Christopherson <seanjc@...gle.com>
CC: Paolo Bonzini <pbonzini@...hat.com>, Paul Durrant <paul@....org>,
 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 18 February 2025 17:33:14 CET, Sean Christopherson <seanjc@...gle.com> wrote:
>On Sat, Feb 15, 2025, David Woodhouse wrote:
>> On 15 February 2025 02:14:33 CET, Sean Christopherson <seanjc@...gle.com> wrote:
>> >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.
>
>I think that has to go in userspace.  If KVM adds on-by-default, i.e. unguarded,
>conflicting MSR emulation, then KVM will have broken userspace regardless of
>whether or not KVM explicitly rejects KVM_XEN_HVM_CONFIG based on emulated MSRs.
>
>If we assume future us are somewhat competent and guard new MSR emulation with a
>feature bit, capability, etc., then rejecting KVM_XEN_HVM_CONFIG isn't obviously
>better, or even feasible in some cases.  E.g. if the opt-in is done via guest
>CPUID, then KVM is stuck because KVM_XEN_HVM_CONFIG can (and generally should?)
>be called before vCPUs are even created.  Even if the opt-in is VM-scoped, e.g.
>a capabilitiy, there are still ordering issues as userpace would see different
>behavior depending on the order between KVM_XEN_HVM_CONFIG and the capability.

Well, I just changed QEMU to do KVM_XEN_HVM_CONFIG from the first vCPU init because QEMU doesn't know if it needs to avoid the Hyper-V MSR space at kvm_xen_init() time. But yes, we don't want to depend on ordering either way.