linux-kernel - Re: [PATCH v3 09/11] KVM: x86/mmu: serialize vCPUs to zap gfn when guest MTRRs are honored

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]

Date:   Thu, 29 Jun 2023 09:51:53 +0800
From:   Yan Zhao <yan.y.zhao@...el.com>
To:     Sean Christopherson <seanjc@...gle.com>
CC:     <kvm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <pbonzini@...hat.com>, <chao.gao@...el.com>, <kai.huang@...el.com>,
        <robert.hoo.linux@...il.com>
Subject: Re: [PATCH v3 09/11] KVM: x86/mmu: serialize vCPUs to zap gfn when
 guest MTRRs are honored

On Wed, Jun 28, 2023 at 04:00:55PM -0700, Sean Christopherson wrote:
> On Fri, Jun 16, 2023, Yan Zhao wrote:
> > diff --git a/arch/x86/kvm/mtrr.c b/arch/x86/kvm/mtrr.c
> > index b35dd0bc9cad..688748e3a4d2 100644
> > --- a/arch/x86/kvm/mtrr.c
> > +++ b/arch/x86/kvm/mtrr.c
> > @@ -25,6 +25,8 @@
> >  #define IA32_MTRR_DEF_TYPE_FE		(1ULL << 10)
> >  #define IA32_MTRR_DEF_TYPE_TYPE_MASK	(0xff)
> >  
> > +static void kvm_mtrr_zap_gfn_range(struct kvm_vcpu *vcpu,
> > +				   gfn_t gfn_start, gfn_t gfn_end);
> >  static bool is_mtrr_base_msr(unsigned int msr)
> >  {
> >  	/* MTRR base MSRs use even numbers, masks use odd numbers. */
> > @@ -341,7 +343,7 @@ static void update_mtrr(struct kvm_vcpu *vcpu, u32 msr)
> >  		var_mtrr_range(var_mtrr_msr_to_range(vcpu, msr), &start, &end);
> >  	}
> >  
> > -	kvm_zap_gfn_range(vcpu->kvm, gpa_to_gfn(start), gpa_to_gfn(end));
> > +	kvm_mtrr_zap_gfn_range(vcpu, gpa_to_gfn(start), gpa_to_gfn(end));
> >  }
> >  
> >  static bool var_mtrr_range_is_valid(struct kvm_mtrr_range *range)
> > @@ -437,6 +439,11 @@ int kvm_mtrr_get_msr(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
> >  void kvm_vcpu_mtrr_init(struct kvm_vcpu *vcpu)
> >  {
> >  	INIT_LIST_HEAD(&vcpu->arch.mtrr_state.head);
> > +
> > +	if (vcpu->vcpu_id == 0) {
> 
> Eww.  This is actually unsafe, because kvm_arch_vcpu_create() is invoked without
> holding kvm->lock.  Oh, and vcpu_id is userspace controlled, so it's *very*
> unsafe.  Just initialize these in kvm_arch_init_vm().
Will do. Thanks!

> 
> > +		spin_lock_init(&vcpu->kvm->arch.mtrr_zap_list_lock);
> > +		INIT_LIST_HEAD(&vcpu->kvm->arch.mtrr_zap_list);
> > +	}
> >  }