Re: [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] kvm_calc_nested_tsc_multiplier prevents overflow truncations but fails to prevent the composed multiplier from underflowing to 0, which leads to hardware VM-entry failures or unhandled host #GPs.
--

--- Patch [1]: [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio ---
commit d48c3d3ad18139611ae5e395e3b21cd0a088ff37
Author: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>

KVM: x86: bound the composed nested TSC multiplier to the max ratio

This patch limits the composed nested TSC multiplier to the hardware maximum
ratio to prevent truncation or reserved bit exceptions. It calculates the
product of L1 and L2 multipliers as a 128-bit value and saturates it to the
advertised maximum if it overflows. This ensures the guest remains alive at a
representable ratio rather than failing.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index ff8812f3a1293..ad9372e6f55df 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -2684,11 +2684,31 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_calc_nested_tsc_offset);
>  
>  u64 kvm_calc_nested_tsc_multiplier(u64 l1_multiplier, u64 l2_multiplier)
>  {
[ ... ]
> +	nested_multiplier = mul_u64_u64_shr(l1_multiplier, l2_multiplier, frac);
> +	if (nested_multiplier > kvm_caps.max_tsc_scaling_ratio)
> +		return kvm_caps.max_tsc_scaling_ratio;
> +
> +	return nested_multiplier;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does this composed multiplier
need to be clamped to prevent underflowing to 0?

While this patch successfully caps the multiplier at
kvm_caps.max_tsc_scaling_ratio, if the L0 guest runs significantly slower than
the host (M1 < 2^frac) and L1 configures a valid L2 multiplier (e.g., M2=1),
the product (M1 * 1) >> frac naturally evaluates to 0.

When the multiplier evaluates to 0, it can lead to a hardware VM-entry
failure on VMX, destroying L1.

Additionally, looking at svm_set_msr() in arch/x86/kvm/svm/svm.c:

	case MSR_AMD64_TSC_RATIO:
		if (data & SVM_TSC_RATIO_RSVD)
			return 1;

		svm->tsc_ratio_msr = data;

Since 0 & SVM_TSC_RATIO_RSVD evaluates to 0, this check fails to reject
data == 0. This allows L1 to directly trigger the zero-multiplier path on
SVM without KVM injecting the architecturally required #GP, which can result
in an unhandled host #GP.

Could the result be clamped to a valid minimum (such as 1) to prevent the
composed multiplier from evaluating to 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722084702.1028983-1-amirmohammad.eftekhar@cispa.de?part=1