[PATCH] KVM: x86: Guard against division by zero in adjust_lapic_timer_advance()

[PATCH] KVM: x86: Guard against division by zero in adjust_lapic_timer_advance()

[Peng Hao]

When TSC calibration fails and userspace never programs a tsc khz,
vcpu->arch.virtual_tsc_khz stays zero, but the lapic timer's
software-mode expiry path still calls adjust_lapic_timer_advance()
unconditionally on every timer expiration (unlike
__wait_lapic_expire(), this path is not gated on tsc_scaling_ratio).
Both the "too early" and "too late" branches do_div() by
virtual_tsc_khz, so a zero khz crashes the host with #DE as soon as
the timer advance drifts outside the adjust window.

Bail out before either division.

Signed-off-by: Peng Hao <flyingpeng@tencent.com>
---
 arch/x86/kvm/lapic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 13e32c4915e2..4dcbc66c8e07 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1991,6 +1991,9 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
 	    abs(advance_expire_delta) < LAPIC_TIMER_ADVANCE_ADJUST_MIN)
 		return;
 
+	if (unlikely(!vcpu->arch.virtual_tsc_khz))
+		return;
+
 	/* too early */
 	if (advance_expire_delta < 0) {
 		ns = -advance_expire_delta * 1000000ULL;
-- 
2.43.7

Re: [PATCH] KVM: x86: Guard against division by zero in adjust_lapic_timer_advance()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: x86: divide by zero in __wait_lapic_expire() with nested TSC scaling
--

commit 615a870f84e9397c4bfba8db0f187034794e3d9e
Author: Peng Hao <flyingpenghao@gmail.com>

KVM: x86: Guard against division by zero in adjust_lapic_timer_advance()

When TSC calibration fails, vcpu->arch.virtual_tsc_khz is zero. This patch
adds a check to avoid division by zero in adjust_lapic_timer_advance().

>     When TSC calibration fails and userspace never programs a tsc khz,
>     vcpu->arch.virtual_tsc_khz stays zero, but the lapic timer's
>     software-mode expiry path still calls adjust_lapic_timer_advance()
>     unconditionally on every timer expiration (unlike
>     __wait_lapic_expire(), this path is not gated on tsc_scaling_ratio).

[ ... ]

> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 48b019114c196..dc4a669d1d460 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1978,6 +1978,9 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
>  	    abs(advance_expire_delta) < LAPIC_TIMER_ADVANCE_ADJUST_MIN)
>  		return;
>  
> +	if (unlikely(!vcpu->arch.virtual_tsc_khz))
> +		return;
> +

[Severity: High]
Does __wait_lapic_expire() need this same check?

The commit message mentions __wait_lapic_expire() is safely gated by
tsc_scaling_ratio, but is that description accurate when nested
virtualization is enabled?

An L1 guest can enable nested virtualization and set a custom tsc_multiplier
in VMCS12 for its L2 guest. KVM computes vcpu->arch.tsc_scaling_ratio to this
non-default value:

arch/x86/kvm/vmx/nested.c:prepare_vmcs02_early() {
    ...
	vcpu->arch.tsc_scaling_ratio = kvm_calc_nested_tsc_multiplier(
			vcpu->arch.l1_tsc_scaling_ratio,
			vmx_get_l2_tsc_multiplier(vcpu));
    ...
}

If host TSC calibration failed and virtual_tsc_khz is 0, when the LAPIC timer
expires during L2 execution, __wait_lapic_expire() takes the else branch
because tsc_scaling_ratio != default_tsc_scaling_ratio:

arch/x86/kvm/lapic.c:__wait_lapic_expire() {
    ...
	if (vcpu->arch.tsc_scaling_ratio == kvm_caps.default_tsc_scaling_ratio) {
		__delay(min(guest_cycles,
			nsec_to_cycles(vcpu, timer_advance_ns)));
	} else {
		u64 delay_ns = guest_cycles * 1000000ULL;
		do_div(delay_ns, vcpu->arch.virtual_tsc_khz);
		ndelay(min_t(u32, delay_ns, timer_advance_ns));
	}
    ...
}

Can an untrusted L1 guest use this path to reliably crash the host with a
divide-by-zero panic?

>  	/* too early */
>  	if (advance_expire_delta < 0) {
>  		ns = -advance_expire_delta * 1000000ULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008090037.86931-1-flyingpeng@tencent.com?part=1

Re: [PATCH] KVM: x86: Guard against division by zero in adjust_lapic_timer_advance()

[Sean Christopherson]

On Thu, Oct 08, 2026, Peng Hao wrote:
> When TSC calibration fails and userspace never programs a tsc khz,
> vcpu->arch.virtual_tsc_khz stays zero, but the lapic timer's
> software-mode expiry path still calls adjust_lapic_timer_advance()
> unconditionally on every timer expiration (unlike
> __wait_lapic_expire(), this path is not gated on tsc_scaling_ratio).
> Both the "too early" and "too late" branches do_div() by
> virtual_tsc_khz, so a zero khz crashes the host with #DE as soon as
> the timer advance drifts outside the adjust window.
> 
> Bail out before either division.
> 
> Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> ---
>  arch/x86/kvm/lapic.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 13e32c4915e2..4dcbc66c8e07 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1991,6 +1991,9 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
>  	    abs(advance_expire_delta) < LAPIC_TIMER_ADVANCE_ADJUST_MIN)
>  		return;
>  
> +	if (unlikely(!vcpu->arch.virtual_tsc_khz))
> +		return;

Ugh.  I'd rather reject KVM_CREATE_VCPU, even though there is a rather surprising
amount of code in KVM that does indeed play nice with tsc_khz == 0.  I don't see
how the guest can possibly function with virtual_tsc_khz==0.

vcpu->arch.virtual_tsc_{shift,mult} will also be left as zero (to avoid #DE there
as well):

	/* tsc_khz can be zero if TSC calibration fails */
	if (user_tsc_khz == 0) {
		/* set tsc_scaling_ratio to a safe value */
		kvm_vcpu_write_tsc_multiplier(vcpu, kvm_caps.default_tsc_scaling_ratio);
		return -1;
	}

	/* Compute a scale to convert nanoseconds in TSC cycles */
	kvm_get_time_scale(user_tsc_khz * 1000LL, NSEC_PER_SEC,
			   &vcpu->arch.virtual_tsc_shift,
			   &vcpu->arch.virtual_tsc_mult);

and so nsec_to_cycles() will always return zero due to multiplying by zero.  That
means things like the APIC timer will always fire immediately:

	apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
		nsec_to_cycles(apic->vcpu, deadline);

The first instance of this goes back to 03ba32cae66e ("VMX: x86: handle host TSC
calibration failure"), and every "fix" since then has been to avoid #DE.

> +
>  	/* too early */
>  	if (advance_expire_delta < 0) {
>  		ns = -advance_expire_delta * 1000000ULL;
> -- 
> 2.43.7
> 

Re: [PATCH] KVM: x86: Guard against division by zero in adjust_lapic_timer_advance()

[Hao Peng]

On Sat, Oct 10, 2026 at 4:35 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Thu, Oct 08, 2026, Peng Hao wrote:
> > When TSC calibration fails and userspace never programs a tsc khz,
> > vcpu->arch.virtual_tsc_khz stays zero, but the lapic timer's
> > software-mode expiry path still calls adjust_lapic_timer_advance()
> > unconditionally on every timer expiration (unlike
> > __wait_lapic_expire(), this path is not gated on tsc_scaling_ratio).
> > Both the "too early" and "too late" branches do_div() by
> > virtual_tsc_khz, so a zero khz crashes the host with #DE as soon as
> > the timer advance drifts outside the adjust window.
> >
> > Bail out before either division.
> >
> > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > ---
> >  arch/x86/kvm/lapic.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > index 13e32c4915e2..4dcbc66c8e07 100644
> > --- a/arch/x86/kvm/lapic.c
> > +++ b/arch/x86/kvm/lapic.c
> > @@ -1991,6 +1991,9 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
> >           abs(advance_expire_delta) < LAPIC_TIMER_ADVANCE_ADJUST_MIN)
> >               return;
> >
> > +     if (unlikely(!vcpu->arch.virtual_tsc_khz))
> > +             return;
>
> Ugh.  I'd rather reject KVM_CREATE_VCPU, even though there is a rather surprising
> amount of code in KVM that does indeed play nice with tsc_khz == 0.  I don't see
> how the guest can possibly function with virtual_tsc_khz==0.
>
Agreed the guest is broken in that state -- virtual_tsc_mult is zero too, so
nsec_to_cycles() returns 0 and the deadline timer fires immediately. But I don't
think KVM_CREATE_VCPU is where to enforce it, because letting vCPU creation
 succeed is what makes the state recoverable:
  - kvm_arch_vcpu_create() deliberately ignores kvm_set_tsc_khz()'s -1
  - KVM_SET_TSC_KHZ is only reachable once the vCPU exists. With
      kvm_caps.has_tsc_control and a khz below kvm_caps.max_guest_tsc_khz it
      recomputes virtual_tsc_{shift,mult} and the guest is fully
functional again.
 - max_tsc_khz does the same thing ahead of time, via default_tsc_khz.

 Rejecting KVM_CREATE_VCPU removes both, the guard is the kind of small,
obviously-correct fix that is.

> vcpu->arch.virtual_tsc_{shift,mult} will also be left as zero (to avoid #DE there
> as well):
>
>         /* tsc_khz can be zero if TSC calibration fails */
>         if (user_tsc_khz == 0) {
>                 /* set tsc_scaling_ratio to a safe value */
>                 kvm_vcpu_write_tsc_multiplier(vcpu, kvm_caps.default_tsc_scaling_ratio);
>                 return -1;
>         }
>
>         /* Compute a scale to convert nanoseconds in TSC cycles */
>         kvm_get_time_scale(user_tsc_khz * 1000LL, NSEC_PER_SEC,
>                            &vcpu->arch.virtual_tsc_shift,
>                            &vcpu->arch.virtual_tsc_mult);
>
> and so nsec_to_cycles() will always return zero due to multiplying by zero.  That
> means things like the APIC timer will always fire immediately:
>
>         apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
>                 nsec_to_cycles(apic->vcpu, deadline);
>
> The first instance of this goes back to 03ba32cae66e ("VMX: x86: handle host TSC
> calibration failure"), and every "fix" since then has been to avoid #DE.
>
> > +
> >       /* too early */
> >       if (advance_expire_delta < 0) {
> >               ns = -advance_expire_delta * 1000000ULL;
> > --
> > 2.43.7
> >

Re: [PATCH] KVM: x86: Guard against division by zero in adjust_lapic_timer_advance()

[Sean Christopherson]

On Sat, Oct 10, 2026, Hao Peng wrote:
> On Sat, Oct 10, 2026 at 4:35 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Thu, Oct 08, 2026, Peng Hao wrote:
> > > When TSC calibration fails and userspace never programs a tsc khz,
> > > vcpu->arch.virtual_tsc_khz stays zero, but the lapic timer's
> > > software-mode expiry path still calls adjust_lapic_timer_advance()
> > > unconditionally on every timer expiration (unlike
> > > __wait_lapic_expire(), this path is not gated on tsc_scaling_ratio).
> > > Both the "too early" and "too late" branches do_div() by
> > > virtual_tsc_khz, so a zero khz crashes the host with #DE as soon as
> > > the timer advance drifts outside the adjust window.
> > >
> > > Bail out before either division.
> > >
> > > Signed-off-by: Peng Hao <flyingpeng@tencent.com>
> > > ---
> > >  arch/x86/kvm/lapic.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> > > index 13e32c4915e2..4dcbc66c8e07 100644
> > > --- a/arch/x86/kvm/lapic.c
> > > +++ b/arch/x86/kvm/lapic.c
> > > @@ -1991,6 +1991,9 @@ static inline void adjust_lapic_timer_advance(struct kvm_vcpu *vcpu,
> > >           abs(advance_expire_delta) < LAPIC_TIMER_ADVANCE_ADJUST_MIN)
> > >               return;
> > >
> > > +     if (unlikely(!vcpu->arch.virtual_tsc_khz))
> > > +             return;
> >
> > Ugh.  I'd rather reject KVM_CREATE_VCPU, even though there is a rather surprising
> > amount of code in KVM that does indeed play nice with tsc_khz == 0.  I don't see
> > how the guest can possibly function with virtual_tsc_khz==0.
> >
> Agreed the guest is broken in that state -- virtual_tsc_mult is zero too, so
> nsec_to_cycles() returns 0 and the deadline timer fires immediately. But I don't
> think KVM_CREATE_VCPU is where to enforce it, because letting vCPU creation
>  succeed is what makes the state recoverable:
>   - kvm_arch_vcpu_create() deliberately ignores kvm_set_tsc_khz()'s -1
>   - KVM_SET_TSC_KHZ is only reachable once the vCPU exists. With
>       kvm_caps.has_tsc_control and a khz below kvm_caps.max_guest_tsc_khz it
>       recomputes virtual_tsc_{shift,mult} and the guest is fully
> functional again.
>  - max_tsc_khz does the same thing ahead of time, via default_tsc_khz.

I don't see how it's recoverable.  kvm_set_tsc_khz() will compute thresh_{lo,hi}=0,
and so use_scaling will always be true.  Which means this:

	ratio = mul_u64_u32_div(1ULL << kvm_caps.tsc_scaling_ratio_frac_bits,
				user_tsc_khz, tsc_khz);

will again hit a #DE, no?

Even if KVM somehow makes it past that, kvm_guest_time_update() will put the vCPU
into an infinite request loop as get_cpu_tsc_khz() will always return 0 for modern
CPUs with a constant TSC:

  static unsigned long get_cpu_tsc_khz(void)
  {
	if (cpu_feature_enabled(X86_FEATURE_CONSTANT_TSC))
		return tsc_khz;    <======================== always zero
	else
		return __this_cpu_read(cpu_tsc_khz);
  }

And so this code will re-request KVM_REQ_CLOCK_UPDATE and the vCPU will hang.

	tgt_tsc_hz = (u64)get_cpu_tsc_khz() * HZ_PER_KHZ;
	if (unlikely(tgt_tsc_hz == 0)) {
		preempt_enable();
		kvm_make_request(KVM_REQ_CLOCK_UPDATE, v);
		return 1;
	}