* [PATCH] kvm: use PIT channel index in hpet_legacy_start mode @ 2016-01-07 12:32 P J P 2016-01-07 12:37 ` Yang Zhang 0 siblings, 1 reply; 5+ messages in thread From: P J P @ 2016-01-07 12:32 UTC (permalink / raw) To: kvm; +Cc: Paolo Bonzini, P J P From: P J P <pjp@fedoraproject.org> 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 <pjp@fedoraproject.org> --- 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); } -- 2.4.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: use PIT channel index in hpet_legacy_start mode 2016-01-07 12:32 [PATCH] kvm: use PIT channel index in hpet_legacy_start mode P J P @ 2016-01-07 12:37 ` Yang Zhang 2016-01-07 12:50 ` Paolo Bonzini 0 siblings, 1 reply; 5+ messages in thread From: Yang Zhang @ 2016-01-07 12:37 UTC (permalink / raw) To: P J P, kvm; +Cc: Paolo Bonzini, P J P On 2016/1/7 20:32, P J P wrote: > From: P J P <pjp@fedoraproject.org> > > 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 <pjp@fedoraproject.org> > --- > 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 -- best regards yang ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: use PIT channel index in hpet_legacy_start mode 2016-01-07 12:37 ` Yang Zhang @ 2016-01-07 12:50 ` Paolo Bonzini 2016-01-07 13:28 ` P J P 0 siblings, 1 reply; 5+ messages in thread From: Paolo Bonzini @ 2016-01-07 12:50 UTC (permalink / raw) To: Yang Zhang, P J P, kvm; +Cc: P J P On 07/01/2016 13:37, Yang Zhang wrote: > On 2016/1/7 20:32, P J P wrote: >> From: P J P <pjp@fedoraproject.org> >> >> 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 <pjp@fedoraproject.org> >> --- >> 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: use PIT channel index in hpet_legacy_start mode 2016-01-07 12:50 ` Paolo Bonzini @ 2016-01-07 13:28 ` P J P 2016-01-07 13:50 ` Paolo Bonzini 0 siblings, 1 reply; 5+ messages in thread From: P J P @ 2016-01-07 13:28 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Yang Zhang, kvm +-- On Thu, 7 Jan 2016, Paolo Bonzini wrote --+ | > 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 :)) IIUC, it shouldn't, because pit_load_count() does /* * The largest possible initial count is 0; this is equivalent * to 216 for binary counting and 104 for BCD counting. */ if (val == 0) val = 0x10000; | 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 | @@ -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); In that case I guess, 'pit_load_count' could be called as + pit_load_count(kvm, 0, val); Thank you. -- - P J P 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm: use PIT channel index in hpet_legacy_start mode 2016-01-07 13:28 ` P J P @ 2016-01-07 13:50 ` Paolo Bonzini 0 siblings, 0 replies; 5+ messages in thread From: Paolo Bonzini @ 2016-01-07 13:50 UTC (permalink / raw) To: P J P; +Cc: Yang Zhang, kvm On 07/01/2016 14:28, P J P wrote: > +-- On Thu, 7 Jan 2016, Paolo Bonzini wrote --+ > | > 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 :)) > > IIUC, it shouldn't, because pit_load_count() does > > /* > * The largest possible initial count is 0; this is equivalent > * to 216 for binary counting and 104 for BCD counting. > */ > if (val == 0) > val = 0x10000; > > > | 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 > | @@ -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); > > In that case I guess, 'pit_load_count' could be called as > > + pit_load_count(kvm, 0, val); Good idea. Paolo > > Thank you. > -- > - P J P > 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-01-07 13:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-07 12:32 [PATCH] kvm: use PIT channel index in hpet_legacy_start mode P J P 2016-01-07 12:37 ` Yang Zhang 2016-01-07 12:50 ` Paolo Bonzini 2016-01-07 13:28 ` P J P 2016-01-07 13:50 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).