linux-kernel - Re: [PATCH] KVM: vmx: fix underflow in TSC deadline calculation

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <640874638.2724253.1467136598812.JavaMail.zimbra@redhat.com>
Date:	Tue, 28 Jun 2016 13:56:38 -0400 (EDT)
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	yunhong jiang <yunhong.jiang@...ux.intel.com>
Cc:	Wanpeng Li <kernellwp@...il.com>, linux-kernel@...r.kernel.org,
	kvm@...r.kernel.org, Wanpeng Li <wanpeng.li@...mail.com>,
	Radim Krčmář <rkrcmar@...hat.com>,
	Yunhong Jiang <yunhong.jiang@...el.com>
Subject: Re: [PATCH] KVM: vmx: fix underflow in TSC deadline calculation



> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index fdc05ae..b15e32a 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -1454,11 +1454,18 @@ static void start_apic_timer(struct kvm_lapic
> > *apic) /* lapic timer in tsc deadline mode */
> >  		u64 tscdeadline = apic->lapic_timer.tscdeadline;
> >  
> > -		if (kvm_x86_ops->set_hv_timer &&
> > -		    !kvm_x86_ops->set_hv_timer(apic->vcpu,
> > tscdeadline)) {
> > -			apic->lapic_timer.hv_timer_in_use = true;
> > -			trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
> > +		if (kvm_x86_ops->set_hv_timer) {
> > +			if (kvm_x86_ops->set_hv_timer(apic->vcpu,
> 
> Would it be better that if set_hv_timer fails, we clear the vmx timer (i.e. the
> VMCS field) before return the failure? I'm not sure if it make sense to clear
> the previous setup if a new setup fails, although it seems OK for me, since
> we have to cancel the hv_timer anyway.

Good question.  I think we should abstract a little the set_hv_timer and
cancel_hv_timer calls, for example with a new function start_hv_tscdeadline.
It's also simpler to avoid the race window by disabling interrupts around the
calls to set_hv_timer and hrtimer_cancel.  I'll see what I can come up with.

Paolo