KVM: x86: avoid underflow when scaling TSC frequency

[PATCH] KVM: x86: avoid underflow when scaling TSC frequency

Posted by Paolo Bonzini 2 months, 4 weeks ago

In function kvm_guest_time_update(), __scale_tsc() is used to calculate
a TSC *frequency* rather than a TSC value.  With low-enough ratios,
a TSC value that is less than 1 would underflow to 0 and to an infinite
while loop in kvm_get_time_scale():

  kvm_guest_time_update(struct kvm_vcpu *v)
    if (kvm_caps.has_tsc_control)
      tgt_tsc_khz = kvm_scale_tsc(tgt_tsc_khz,
                                  v->arch.l1_tsc_scaling_ratio);
        __scale_tsc(u64 ratio, u64 tsc)
          ratio=122380531, tsc=2299998, N=48
          ratio*tsc >> N = 0.999... -> 0

Later in the function:

  Call Trace:
   <TASK>
   kvm_get_time_scale arch/x86/kvm/x86.c:2458 [inline]
   kvm_guest_time_update+0x926/0xb00 arch/x86/kvm/x86.c:3268
   vcpu_enter_guest.constprop.0+0x1e70/0x3cf0 arch/x86/kvm/x86.c:10678
   vcpu_run+0x129/0x8d0 arch/x86/kvm/x86.c:11126
   kvm_arch_vcpu_ioctl_run+0x37a/0x13d0 arch/x86/kvm/x86.c:11352
   kvm_vcpu_ioctl+0x56b/0xe60 virt/kvm/kvm_main.c:4188
   vfs_ioctl fs/ioctl.c:51 [inline]
   __do_sys_ioctl fs/ioctl.c:871 [inline]
   __se_sys_ioctl+0x12d/0x190 fs/ioctl.c:857
   do_syscall_x64 arch/x86/entry/common.c:51 [inline]
   do_syscall_64+0x59/0x110 arch/x86/entry/common.c:81
   entry_SYSCALL_64_after_hwframe+0x78/0xe2

This can really happen only when fuzzing, since the TSC frequency
would have to be nonsensically low.

Fixes: 35181e86df97 ("KVM: x86: Add a common TSC scaling function")
Reported-by: Yuntao Liu <liuyuntao12@huawei.com>
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index b58a74c1722d..de51dbd85a58 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3258,9 +3258,11 @@ int kvm_guest_time_update(struct kvm_vcpu *v)
 
 	/* With all the info we got, fill in the values */
 
-	if (kvm_caps.has_tsc_control)
+	if (kvm_caps.has_tsc_control) {
 		tgt_tsc_khz = kvm_scale_tsc(tgt_tsc_khz,
 					    v->arch.l1_tsc_scaling_ratio);
+		tgt_tsc_khz = tgt_tsc_khz ? : 1;
+	}
 
 	if (unlikely(vcpu->hw_tsc_khz != tgt_tsc_khz)) {
 		kvm_get_time_scale(NSEC_PER_SEC, tgt_tsc_khz * 1000LL,
-- 
2.43.5

Re: [PATCH] KVM: x86: avoid underflow when scaling TSC frequency

Posted by mlevitsk@redhat.com 2 months, 4 weeks ago

On Wed, 2025-07-09 at 13:53 -0400, Paolo Bonzini wrote:
> In function kvm_guest_time_update(), __scale_tsc() is used to calculate
> a TSC *frequency* rather than a TSC value.  With low-enough ratios,
> a TSC value that is less than 1 would underflow to 0 and to an infinite
> while loop in kvm_get_time_scale():
> 
>   kvm_guest_time_update(struct kvm_vcpu *v)
>     if (kvm_caps.has_tsc_control)
>       tgt_tsc_khz = kvm_scale_tsc(tgt_tsc_khz,
>                                   v->arch.l1_tsc_scaling_ratio);
>         __scale_tsc(u64 ratio, u64 tsc)
>           ratio=122380531, tsc=2299998, N=48
>           ratio*tsc >> N = 0.999... -> 0
> 
> Later in the function:
> 
>   Call Trace:
>    <TASK>
>    kvm_get_time_scale arch/x86/kvm/x86.c:2458 [inline]
>    kvm_guest_time_update+0x926/0xb00 arch/x86/kvm/x86.c:3268
>    vcpu_enter_guest.constprop.0+0x1e70/0x3cf0 arch/x86/kvm/x86.c:10678
>    vcpu_run+0x129/0x8d0 arch/x86/kvm/x86.c:11126
>    kvm_arch_vcpu_ioctl_run+0x37a/0x13d0 arch/x86/kvm/x86.c:11352
>    kvm_vcpu_ioctl+0x56b/0xe60 virt/kvm/kvm_main.c:4188
>    vfs_ioctl fs/ioctl.c:51 [inline]
>    __do_sys_ioctl fs/ioctl.c:871 [inline]
>    __se_sys_ioctl+0x12d/0x190 fs/ioctl.c:857
>    do_syscall_x64 arch/x86/entry/common.c:51 [inline]
>    do_syscall_64+0x59/0x110 arch/x86/entry/common.c:81
>    entry_SYSCALL_64_after_hwframe+0x78/0xe2
> 
> This can really happen only when fuzzing, since the TSC frequency
> would have to be nonsensically low.
> 
> Fixes: 35181e86df97 ("KVM: x86: Add a common TSC scaling function")
> Reported-by: Yuntao Liu <liuyuntao12@huawei.com>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/x86.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index b58a74c1722d..de51dbd85a58 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3258,9 +3258,11 @@ int kvm_guest_time_update(struct kvm_vcpu *v)
>  
>  	/* With all the info we got, fill in the values */
>  
> -	if (kvm_caps.has_tsc_control)
> +	if (kvm_caps.has_tsc_control) {
>  		tgt_tsc_khz = kvm_scale_tsc(tgt_tsc_khz,
>  					    v->arch.l1_tsc_scaling_ratio);
> +		tgt_tsc_khz = tgt_tsc_khz ? : 1;
> +	}
>  
>  	if (unlikely(vcpu->hw_tsc_khz != tgt_tsc_khz)) {
>  		kvm_get_time_scale(NSEC_PER_SEC, tgt_tsc_khz * 1000LL,

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky