* [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).