Re: [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Failing to write the MSR on an invalid TSC scaling ratio leaves the hardware MSR unmodified, allowing the guest to run with and leak the previous tenant's TSC ratio.
--

--- Patch [1]: [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR ---
commit 1572db0bc125650e334549b4e8c1388d585aa8c9
Author: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>

KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR

This commit updates __svm_write_tsc_multiplier() to use wrmsrq_safe() when
writing the MSR_AMD64_TSC_RATIO. This change aims to prevent a host #GP
oops if a guest configures a composed nested TSC multiplier that exceeds the
40-bit hardware limit, falling back to a warning instead of a fault.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -467,7 +467,18 @@ static void __svm_write_tsc_multiplier(u64 multiplier)
>  	if (multiplier == __this_cpu_read(current_tsc_ratio))
>  		return;
>  
> -	wrmsrq(MSR_AMD64_TSC_RATIO, multiplier);
> +	/*
> +	 * multiplier is derived from the guest-influenced nested TSC scaling
> +	 * ratio and can, on a composed nested value, set bits above the 40-bit
> +	 * MSR_AMD64_TSC_RATIO field.  A plain wrmsrq() would #GP in host ring 0
> +	 * (caught only by the WRMSR extable); never let a guest-influenced
> +	 * value fault the host MSR write regardless of the compose bound.
> +	 */
> +	if (wrmsrq_safe(MSR_AMD64_TSC_RATIO, multiplier)) {
> +		pr_warn_ratelimited("SVM: rejected out-of-range TSC ratio %#llx\n",
> +				    multiplier);
> +		return;
> +	}

[Severity: High]
This is a pre-existing issue, but does returning early here leave the
hardware MSR unmodified?

If wrmsrq_safe() fails, the physical MSR_AMD64_TSC_RATIO retains its
previous value from the last VCPU scheduled on that physical core. Since the
error is caught but not propagated to abort the VCPU run, the guest could
enter with the unmodified, cross-tenant MSR value.

Because kvm_calc_nested_tsc_multiplier() in arch/x86/kvm/x86.c lacks bounds
checking (as the companion patch mentioned in the commit message is missing
from this tree), a malicious L1 guest could configure a large nested TSC
multiplier that exceeds the 40-bit hardware limit:

arch/x86/kvm/x86.c:kvm_calc_nested_tsc_multiplier() {
	if (l2_multiplier != kvm_caps.default_tsc_scaling_ratio)
		return mul_u64_u64_shr(l1_multiplier, l2_multiplier,
				       kvm_caps.tsc_scaling_ratio_frac_bits);

	return l1_multiplier;
}

Could this allow an L1 guest to run with and deduce the previous tenant's
TSC ratio?

>  	__this_cpu_write(current_tsc_ratio, multiplier);
>  }

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