Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Missing Out-of-Bounds CPUID Leaf Check in xen_tsc_khz()
--

commit 7f8bc6380ca7c0fd31cb0c2bf54c9ef13654964c
Author: David Woodhouse <[email protected]>

x86/xen: Obtain TSC frequency from CPUID if present

This commit updates the Xen timekeeping code to try reading the guest
TSC frequency directly from Xen CPUID leaf 3, sub-leaf 0, ECX. If the
value is present, it uses it instead of reverse-calculating the frequency
from the pvclock multiplier and shift.

> diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
> index 487ad838c4418..36d66abf53796 100644
> --- a/arch/x86/xen/time.c
> +++ b/arch/x86/xen/time.c
> @@ -42,6 +42,17 @@ static unsigned int __init xen_tsc_khz(void)
>  {
>       struct pvclock_vcpu_time_info *info =
>               &HYPERVISOR_shared_info->vcpu_info[0].time;
> +     u32 base = xen_cpuid_base();
> +     u32 eax, ebx, ecx, edx;
> +
> +     /*
> +      * If Xen provides the guest TSC frequency directly in CPUID
> +      * (leaf 3, sub-leaf 0, ECX), use that instead of reverse-
> +      * calculating from the pvclock mul/shift.
> +      */
> +     cpuid_count(base + 3, 0, &eax, &ebx, &ecx, &edx);
> +     if (ecx)
> +             return ecx;

[Severity: High]
Is it possible for the hypervisor to support a maximum leaf of base + 2,
resulting in an out-of-bounds CPUID query here?

Looking at xen_cpuid_base(), it calls cpuid_base_hypervisor(XEN_SIGNATURE, 2)
which only guarantees that the hypervisor supports up to leaf base + 2.

If the hypervisor does not support base + 3, this CPUID query could be out
of bounds. Processors handling out-of-bounds CPUID requests typically return
data from the maximum basic leaf.

Since the ECX register of the maximum basic leaf frequently contains non-zero
data (such as a bus or crystal clock frequency), would the if (ecx) check
blindly accept this unintended data as the guest's TSC frequency?

Using a drastically incorrect TSC frequency could break timekeeping, timers,
and the scheduler during boot.

>  
>       return pvclock_tsc_khz(info);
>  }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=21

Reply via email to