linux-kernel - [PATCH v3 1/4] KVM: x86: relax canonical check for some x86 architectural msrs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240815123349.729017-2-mlevitsk@redhat.com>
Date: Thu, 15 Aug 2024 15:33:46 +0300
From: Maxim Levitsky <mlevitsk@...hat.com>
To: kvm@...r.kernel.org
Cc: Ingo Molnar <mingo@...hat.com>,
	x86@...nel.org,
	Paolo Bonzini <pbonzini@...hat.com>,
	Sean Christopherson <seanjc@...gle.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Borislav Petkov <bp@...en8.de>,
	linux-kernel@...r.kernel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	Maxim Levitsky <mlevitsk@...hat.com>,
	Chao Gao <chao.gao@...el.com>
Subject: [PATCH v3 1/4] KVM: x86: relax canonical check for some x86 architectural msrs

Several x86's arch msrs contain a linear base, and thus must
contain a canonical address.

This includes FS/GS base, addresses used for SYSENTER and SYSCALL
instructions and probably more.

As it turns out, when x86 architecture was updated to 5 level paging /
57 bit virtual addresses, these MSRs were allowed to contain a full
57 bit address regardless of the state of CR4.LA57.

The main reason behind this decision is that 5 level paging, and even
paging itself can be temporarily disabled (e.g by SMM entry) leaving non
canonical values in these fields.
Another reason is that OS might prepare these fields before it switches to
5 level paging.

Experemental tests on a Sapphire Rapids CPU and on a Zen4 CPU confirm this
behavior.

In addition to that, the Intel ISA extension manual mentions that this
might be the architectural behavior:

Architecture Instruction Set Extensions and Future Features Programming
Reference [1], Chapter 6.4:

"CANONICALITY CHECKING FOR DATA ADDRESSES WRITTEN TO CONTROL REGISTERS AND
MSRS"

"In Processors that support LAM continue to require the addresses written
tocontrol registers or MSRs to be 57-bit canonical if the processor
supports 5-level paging or 48-bit canonical if it supports only 4-level
paging"

[1]: https://cdrdv2.intel.com/v1/dl/getContent/671368

Suggested-by: Chao Gao <chao.gao@...el.com>
Signed-off-by: Maxim Levitsky <mlevitsk@...hat.com>
---
 arch/x86/kvm/x86.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ce7c00894f32..2e83f7d74591 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -302,6 +302,31 @@ const struct kvm_stats_header kvm_vcpu_stats_header = {
 		       sizeof(kvm_vcpu_stats_desc),
 };
 
+
+/*
+ * Most x86 arch MSR values which contain linear addresses like
+ * segment bases, addresses that are used in instructions (e.g SYSENTER),
+ * have static canonicality checks,
+ * size of whose depends only on CPU's support for 5-level
+ * paging, rather than state of CR4.LA57.
+ *
+ * In addition to that, some of these MSRS are directly passed
+ * to the guest (e.g MSR_KERNEL_GS_BASE) thus even if the guest
+ * doen't have LA57 enabled in its CPUID, for consistency with
+ * CPUs' ucode, it is better to pivot the check around host
+ * support for 5 level paging.
+ */
+
+static u8  max_host_supported_virt_addr_bits(void)
+{
+	return kvm_cpu_cap_has(X86_FEATURE_LA57) ? 57 : 48;
+}
+
+static bool is_host_noncanonical_msr_value(u64 la)
+{
+	return !__is_canonical_address(la, max_host_supported_virt_addr_bits());
+}
+
 static struct kmem_cache *x86_emulator_cache;
 
 /*
@@ -1829,7 +1854,7 @@ static int __kvm_set_msr(struct kvm_vcpu *vcpu, u32 index, u64 data,
 	case MSR_KERNEL_GS_BASE:
 	case MSR_CSTAR:
 	case MSR_LSTAR:
-		if (is_noncanonical_address(data, vcpu))
+		if (is_host_noncanonical_msr_value(data))
 			return 1;
 		break;
 	case MSR_IA32_SYSENTER_EIP:
@@ -1846,7 +1871,7 @@ static int __kvm_set_msr(struct kvm_vcpu *vcpu, u32 index, u64 data,
 		 * value, and that something deterministic happens if the guest
 		 * invokes 64-bit SYSENTER.
 		 */
-		data = __canonical_address(data, vcpu_virt_addr_bits(vcpu));
+		data = __canonical_address(data, max_host_supported_virt_addr_bits());
 		break;
 	case MSR_TSC_AUX:
 		if (!kvm_is_supported_user_return_msr(MSR_TSC_AUX))
-- 
2.40.1