linux-kernel - Re: [PATCH 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: <43f702b383fb99d435f2cdb8ef35cc1449fe6c23.camel@infradead.org>
Date: Wed, 05 Feb 2025 09:27:26 +0000
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, 
	syzbot+cdeaeec70992eca2d920@...kaller.appspotmail.com, Joao Martins
	 <joao.m.martins@...cle.com>
Subject: Re: [PATCH 1/5] KVM: x86/xen: Restrict hypercall MSR to unofficial
 synthetic range

On Fri, 2025-01-31 at 17:13 -0800, Sean Christopherson wrote:
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1324,6 +1324,14 @@ 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 < 0x40000000 || xhc->msr > 0x4fffffff))
> +		return -EINVAL;
> +
>  	mutex_lock(&kvm->arch.xen.xen_lock);
>  
>  	if (xhc->msr && !kvm->arch.xen_hvm_config.msr)

I'd prefer to see #defines for those magic values. Especially as there
is a corresponding requirement that they never be set from host context
(which is where the potential locking issues come in). Which train of
thought leads me to ponder this as an alternative (or additional)
solution:

--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3733,7 +3733,13 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
        u32 msr = msr_info->index;
        u64 data = msr_info->data;
 
-       if (msr && msr == vcpu->kvm->arch.xen_hvm_config.msr)
+       /*
+        * Do not allow host-initiated writes to trigger the Xen hypercall
+        * page setup; it could incur locking paths which are not expected
+        * if userspace sets the MSR in an unusual location.
+        */
+       if (msr && msr == vcpu->kvm->arch.xen_hvm_config.msr &&
+           !msr_info->host_initiated)
                return kvm_xen_write_hypercall_page(vcpu, data);
 
        switch (msr) {


Download attachment "smime.p7s" of type "application/pkcs7-signature" (5069 bytes)