All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio
@ 2026-07-22  8:47 amirmohammad.eftekhar
  2026-07-22  8:47 ` [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR amirmohammad.eftekhar
  2026-07-22  9:16 ` [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio sashiko-bot
  0 siblings, 2 replies; 5+ messages in thread
From: amirmohammad.eftekhar @ 2026-07-22  8:47 UTC (permalink / raw)
  To: kvm; +Cc: Sean Christopherson, Paolo Bonzini, rossow, Amirmohammad Eftekhar

From: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>

kvm_calc_nested_tsc_multiplier() folds L1's own TSC multiplier (M1) with
the multiplier L1 requests for L2 (M2) into the single hardware ratio
programmed for L2: M_eff = (M1 * M2) >> frac.  Each input is validated
individually against the per-vendor maximum, but the composed output is
not, and it is stored into a 64-bit field:

 - Intel (frac = 48): when M1 * M2 >= 2^(64+48) the true M_eff exceeds
   2^64 and the low 64 bits are programmed silently.  L2 then runs at a
   wrong rate; the truncated value can even land on default_tsc_scaling_
   ratio (2^48), disabling scaling entirely, or on 0, which fails the
   nested VM-entry.  This is reachable with no adversary on live
   migration: M1 = (guest_khz << 48) / host_khz grows on a slower
   destination, M_eff is recomposed on KVM_SET_NESTED_STATE / the
   destination KVM_SET_TSC_KHZ with the same missing check, and a
   (M1, M2) legal on the source can recompose to M_eff == 0 on the
   destination, whose mid-flight resume fails hardware VM-entry on
   vmcs02 and tears the guest down (KVM_EXIT_SHUTDOWN).

 - AMD (frac = 32): the ratio MSR is 40 bits, but M_eff can reach ~2^48,
   setting reserved bits that architecturally #GP the physical
   MSR_AMD64_TSC_RATIO write (see the companion wrmsrq_safe() change).

KVM already rejects an input M2 == 0 at nested-entry validation; extend
that to the composed result.  Compute the full 128-bit product, detect a
value that does not fit the field (its high 64 bits reach 1 << frac) or
exceeds kvm_caps.max_tsc_scaling_ratio, and saturate to the advertised
maximum -- keeping the guest alive at a representable ratio instead of
truncating.  This runs on both the entry compose (prepare_vmcs02 /
nested_svm_update_tsc_ratio_msr) and the restore/recompute paths that
share this helper (kvm_vcpu_write_tsc_multiplier, KVM_SET_NESTED_STATE).

Signed-off-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
---
This and the follow-up wrmsrq_safe() patch (next in this thread) are
sent together to security@ rather than the public list: the AMD side
is why -- the composed value here can carry reserved bits straight into
a physical MSR write in host ring 0, and today only the WRMSR exception
table stands between that and a real #GP oops on a build that lacks it.

 arch/x86/kvm/x86.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0ce0d30e8825..5856b2358ee2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2676,11 +2676,31 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_calc_nested_tsc_offset);
 
 u64 kvm_calc_nested_tsc_multiplier(u64 l1_multiplier, u64 l2_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);
+	unsigned int frac = kvm_caps.tsc_scaling_ratio_frac_bits;
+	u64 nested_multiplier;
 
-	return l1_multiplier;
+	if (l2_multiplier == kvm_caps.default_tsc_scaling_ratio)
+		return l1_multiplier;
+
+	/*
+	 * Both inputs are individually within [1, max_tsc_scaling_ratio], but
+	 * their fixed-point product (M1 * M2) >> frac can exceed the width of
+	 * the hardware TSC multiplier field: on Intel the high bits would be
+	 * silently truncated (yielding a wrong -- or even zero -- L2 scaling
+	 * ratio), and on AMD the surplus bits are reserved bits that #GP the
+	 * physical MSR_AMD64_TSC_RATIO write.  (M1 * M2) >> frac overflows u64
+	 * iff the high 64 bits of the 128-bit product are >= (1 << frac);
+	 * detect that (and any result above the advertised maximum) and
+	 * saturate rather than program an unrepresentable ratio.
+	 */
+	if (mul_u64_u64_shr(l1_multiplier, l2_multiplier, 64) >= (1ULL << frac))
+		return kvm_caps.max_tsc_scaling_ratio;
+
+	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;
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_calc_nested_tsc_multiplier);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR
  2026-07-22  8:47 [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio amirmohammad.eftekhar
@ 2026-07-22  8:47 ` amirmohammad.eftekhar
  2026-07-22  9:14   ` sashiko-bot
  2026-07-22 13:51   ` Sean Christopherson
  2026-07-22  9:16 ` [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio sashiko-bot
  1 sibling, 2 replies; 5+ messages in thread
From: amirmohammad.eftekhar @ 2026-07-22  8:47 UTC (permalink / raw)
  To: kvm; +Cc: Sean Christopherson, Paolo Bonzini, rossow, Amirmohammad Eftekhar

From: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>

__svm_write_tsc_multiplier() writes vcpu->arch.tsc_scaling_ratio to the
physical MSR_AMD64_TSC_RATIO with a plain wrmsrq().  Under nested TSC
scaling the ratio is a composed value (see the companion x86.c change),
and a composed value can set bits above the MSR's 40-bit width; those
are reserved bits that architecturally fault (#GP) the WRMSR.  Today
only the EX_TYPE_WRMSR extable stands between a guest-influenced ratio
and a host #GP oops in ring 0 -- on a build or path without it the host
faults.

Do not depend on the extable for a guest-influenced MSR write: use
wrmsrq_safe(), and on failure keep the previous ratio (don't update the
current_tsc_ratio cache) and warn.  Defense in depth alongside bounding
the composed ratio at its source.

Signed-off-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
---
 arch/x86/kvm/svm/svm.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 4d2bacd00ec4..d7f2202b7c45 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -533,7 +533,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;
+	}
 	__this_cpu_write(current_tsc_ratio, multiplier);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR
  2026-07-22  8:47 ` [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR amirmohammad.eftekhar
@ 2026-07-22  9:14   ` sashiko-bot
  2026-07-22 13:51   ` Sean Christopherson
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-22  9:14 UTC (permalink / raw)
  To: amirmohammad.eftekhar; +Cc: kvm

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio
  2026-07-22  8:47 [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio amirmohammad.eftekhar
  2026-07-22  8:47 ` [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR amirmohammad.eftekhar
@ 2026-07-22  9:16 ` sashiko-bot
  1 sibling, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-07-22  9:16 UTC (permalink / raw)
  To: amirmohammad.eftekhar; +Cc: kvm

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR
  2026-07-22  8:47 ` [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR amirmohammad.eftekhar
  2026-07-22  9:14   ` sashiko-bot
@ 2026-07-22 13:51   ` Sean Christopherson
  1 sibling, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2026-07-22 13:51 UTC (permalink / raw)
  To: amirmohammad.eftekhar; +Cc: kvm, Paolo Bonzini, rossow

On Wed, Jul 22, 2026, amirmohammad.eftekhar@cispa.de wrote:
> From: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
> 
> __svm_write_tsc_multiplier() writes vcpu->arch.tsc_scaling_ratio to the
> physical MSR_AMD64_TSC_RATIO with a plain wrmsrq().  Under nested TSC
> scaling the ratio is a composed value (see the companion x86.c change),
> and a composed value can set bits above the MSR's 40-bit width; those
> are reserved bits that architecturally fault (#GP) the WRMSR.  Today
> only the EX_TYPE_WRMSR extable stands between a guest-influenced ratio
> and a host #GP oops in ring 0 -- on a build or path without it the host
> faults.
> 
> Do not depend on the extable for a guest-influenced MSR write: use
> wrmsrq_safe(), and on failure keep the previous ratio (don't update the
> current_tsc_ratio cache) and warn.  Defense in depth alongside bounding
> the composed ratio at its source.

NAK.  As I said off-list, the EX_TYPE_WRMSR exception fixup _is_ the defense in
depth we want.  If KVM does have a bug, I want a WARN.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-22 13:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22  8:47 [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio amirmohammad.eftekhar
2026-07-22  8:47 ` [PATCH] KVM: SVM: use wrmsrq_safe() when writing the nested TSC ratio MSR amirmohammad.eftekhar
2026-07-22  9:14   ` sashiko-bot
2026-07-22 13:51   ` Sean Christopherson
2026-07-22  9:16 ` [PATCH] KVM: x86: bound the composed nested TSC multiplier to the max ratio sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.