* [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON
@ 2020-03-19 17:43 Paolo Bonzini
2020-03-19 17:52 ` Sean Christopherson
2020-03-20 15:22 ` Thomas Gleixner
0 siblings, 2 replies; 6+ messages in thread
From: Paolo Bonzini @ 2020-03-19 17:43 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: syzbot+00be5da1d75f1cc95f6b, Sean Christopherson
The WARN_ON is essentially comparing a user-provided value with 0. It is
trivial to trigger it just by passing garbage to KVM_SET_CLOCK. Guests
can break if you do so, but if it hurts when you do like this just do not
do it.
Reported-by: syzbot+00be5da1d75f1cc95f6b@syzkaller.appspotmail.com
Fixes: 9446e6fce0ab ("KVM: x86: fix WARN_ON check of an unsigned less than zero")
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/x86.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3156e25b0774..d65ff2008cf1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2444,7 +2444,6 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
vcpu->hv_clock.tsc_timestamp = tsc_timestamp;
vcpu->hv_clock.system_time = kernel_ns + v->kvm->arch.kvmclock_offset;
vcpu->last_guest_tsc = tsc_timestamp;
- WARN_ON((s64)vcpu->hv_clock.system_time < 0);
/* If the host uses TSC clocksource, then it is stable */
pvclock_flags = 0;
--
2.18.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON
2020-03-19 17:43 [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON Paolo Bonzini
@ 2020-03-19 17:52 ` Sean Christopherson
2020-03-20 15:22 ` Thomas Gleixner
1 sibling, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2020-03-19 17:52 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm, syzbot+00be5da1d75f1cc95f6b
On Thu, Mar 19, 2020 at 01:43:18PM -0400, Paolo Bonzini wrote:
> The WARN_ON is essentially comparing a user-provided value with 0. It is
> trivial to trigger it just by passing garbage to KVM_SET_CLOCK. Guests
> can break if you do so, but if it hurts when you do like this just do not
> do it.
>
> Reported-by: syzbot+00be5da1d75f1cc95f6b@syzkaller.appspotmail.com
> Fixes: 9446e6fce0ab ("KVM: x86: fix WARN_ON check of an unsigned less than zero")
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON
2020-03-19 17:43 [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON Paolo Bonzini
2020-03-19 17:52 ` Sean Christopherson
@ 2020-03-20 15:22 ` Thomas Gleixner
2020-03-20 15:23 ` Thomas Gleixner
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Gleixner @ 2020-03-20 15:22 UTC (permalink / raw)
To: Paolo Bonzini, linux-kernel, kvm
Cc: syzbot+00be5da1d75f1cc95f6b, Sean Christopherson
Paolo Bonzini <pbonzini@redhat.com> writes:
> The WARN_ON is essentially comparing a user-provided value with 0. It is
> trivial to trigger it just by passing garbage to KVM_SET_CLOCK. Guests
> can break if you do so, but if it hurts when you do like this just do not
> do it.
Yes, it's a user provided value and it's completely unchecked. If that
value is bogus then the guest will go sideways because timekeeping is
completely busted. At least you should explain WHY you don't care.
Thanks,
tglx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON
2020-03-20 15:22 ` Thomas Gleixner
@ 2020-03-20 15:23 ` Thomas Gleixner
2020-03-20 17:47 ` Paolo Bonzini
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Gleixner @ 2020-03-20 15:23 UTC (permalink / raw)
To: Paolo Bonzini, linux-kernel, kvm
Cc: syzbot+00be5da1d75f1cc95f6b, Sean Christopherson
Thomas Gleixner <tglx@linutronix.de> writes:
> Paolo Bonzini <pbonzini@redhat.com> writes:
>> The WARN_ON is essentially comparing a user-provided value with 0. It is
>> trivial to trigger it just by passing garbage to KVM_SET_CLOCK. Guests
>> can break if you do so, but if it hurts when you do like this just do not
>> do it.
>
> Yes, it's a user provided value and it's completely unchecked. If that
> value is bogus then the guest will go sideways because timekeeping is
> completely busted. At least you should explain WHY you don't care.
Or why it does not matter....
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON
2020-03-20 15:23 ` Thomas Gleixner
@ 2020-03-20 17:47 ` Paolo Bonzini
2020-03-20 18:18 ` Thomas Gleixner
0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2020-03-20 17:47 UTC (permalink / raw)
To: Thomas Gleixner, linux-kernel, kvm
Cc: syzbot+00be5da1d75f1cc95f6b, Sean Christopherson
On 20/03/20 16:23, Thomas Gleixner wrote:
> Thomas Gleixner <tglx@linutronix.de> writes:
>
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>> The WARN_ON is essentially comparing a user-provided value with 0. It is
>>> trivial to trigger it just by passing garbage to KVM_SET_CLOCK. Guests
>>> can break if you do so, but if it hurts when you do like this just do not
>>> do it.
>>
>> Yes, it's a user provided value and it's completely unchecked. If that
>> value is bogus then the guest will go sideways because timekeeping is
>> completely busted. At least you should explain WHY you don't care.
>
> Or why it does not matter....
I can change the commit message to "Guests can break if you do so, but
the same applies to every KVM_SET_* ioctl". It's impossible to be sure
that userspace doesn't ever send a bogus KVM_SET_CLOCK and later
rectifies it with the right value.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON
2020-03-20 17:47 ` Paolo Bonzini
@ 2020-03-20 18:18 ` Thomas Gleixner
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Gleixner @ 2020-03-20 18:18 UTC (permalink / raw)
To: Paolo Bonzini, linux-kernel, kvm
Cc: syzbot+00be5da1d75f1cc95f6b, Sean Christopherson
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 20/03/20 16:23, Thomas Gleixner wrote:
>> Thomas Gleixner <tglx@linutronix.de> writes:
>>
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>> The WARN_ON is essentially comparing a user-provided value with 0. It is
>>>> trivial to trigger it just by passing garbage to KVM_SET_CLOCK. Guests
>>>> can break if you do so, but if it hurts when you do like this just do not
>>>> do it.
>>>
>>> Yes, it's a user provided value and it's completely unchecked. If that
>>> value is bogus then the guest will go sideways because timekeeping is
>>> completely busted. At least you should explain WHY you don't care.
>>
>> Or why it does not matter....
>
> I can change the commit message to "Guests can break if you do so, but
> the same applies to every KVM_SET_* ioctl". It's impossible to be sure
> that userspace doesn't ever send a bogus KVM_SET_CLOCK and later
> rectifies it with the right value.
Yes please.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-20 18:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19 17:43 [PATCH] KVM: x86: remove bogus user-triggerable WARN_ON Paolo Bonzini
2020-03-19 17:52 ` Sean Christopherson
2020-03-20 15:22 ` Thomas Gleixner
2020-03-20 15:23 ` Thomas Gleixner
2020-03-20 17:47 ` Paolo Bonzini
2020-03-20 18:18 ` Thomas Gleixner
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).