From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] kvm: use PIT channel index in hpet_legacy_start mode Date: Thu, 7 Jan 2016 13:50:16 +0100 Message-ID: <568E5F08.5030900@redhat.com> References: <1452169950-14711-1-git-send-email-ppandit@redhat.com> <568E5C0B.9010507@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: P J P To: Yang Zhang , P J P , kvm@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:51877 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753489AbcAGMuV (ORCPT ); Thu, 7 Jan 2016 07:50:21 -0500 In-Reply-To: <568E5C0B.9010507@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 07/01/2016 13:37, Yang Zhang wrote: > On 2016/1/7 20:32, P J P wrote: >> From: P J P >> >> While setting the KVM PIT counters in 'kvm_pit_load_count', if >> 'hpet_legacy_start' is set, the function disables the timer on >> channel[0], instead of the respective index 'channel'. Update it >> to use 'channel' index parameter. >> >> Signed-off-by: P J P >> --- >> arch/x86/kvm/i8254.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c >> index 08116ff..154e936 100644 >> --- a/arch/x86/kvm/i8254.c >> +++ b/arch/x86/kvm/i8254.c >> @@ -420,10 +420,11 @@ void kvm_pit_load_count(struct kvm *kvm, int >> channel, u32 val, int hpet_legacy_s >> u8 saved_mode; >> if (hpet_legacy_start) { >> /* save existing mode for later reenablement */ >> - saved_mode = kvm->arch.vpit->pit_state.channels[0].mode; >> - kvm->arch.vpit->pit_state.channels[0].mode = 0xff; /* disable >> timer */ >> + saved_mode = kvm->arch.vpit->pit_state.channels[channel].mode; >> + /* disable timer */ >> + kvm->arch.vpit->pit_state.channels[channel].mode = 0xff; >> pit_load_count(kvm, channel, val); >> - kvm->arch.vpit->pit_state.channels[0].mode = saved_mode; >> + kvm->arch.vpit->pit_state.channels[channel].mode = saved_mode; >> } else { >> pit_load_count(kvm, channel, val); >> } >> > > Will this trigger the same issue like CVE-2015-7513 ? > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0185604c2d82c560dab2f2933a18f797e74ab5a8 I am not sure (--verbose please :)) but the right fix is to change the caller like this: diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c index 08116ff227cc..b0ea42b78ccd 100644 --- a/arch/x86/kvm/i8254.c +++ b/arch/x86/kvm/i8254.c @@ -420,6 +420,7 @@ void kvm_pit_load_count(struct kvm *kvm, int channel, u32 val, int hpet_legacy_s u8 saved_mode; if (hpet_legacy_start) { /* save existing mode for later reenablement */ + WARN_ON(channel != 0); saved_mode = kvm->arch.vpit->pit_state.channels[0].mode; kvm->arch.vpit->pit_state.channels[0].mode = 0xff; /* disable timer */ pit_load_count(kvm, channel, val); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 7ffc224bbe41..97592e190413 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3606,7 +3606,8 @@ static int kvm_vm_ioctl_set_pit2(struct kvm *kvm, struct kvm_pit_state2 *ps) sizeof(kvm->arch.vpit->pit_state.channels)); kvm->arch.vpit->pit_state.flags = ps->flags; for (i = 0; i < 3; i++) - kvm_pit_load_count(kvm, i, kvm->arch.vpit->pit_state.channels[i].count, start); + kvm_pit_load_count(kvm, i, kvm->arch.vpit->pit_state.channels[i].count, + start && i == 0); mutex_unlock(&kvm->arch.vpit->pit_state.lock); return 0; } Thanks, Paolo