linux-kernel - Re: [PATCH v1 10/23] KVM: VMX: Add support for FRED context save/restore

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]

Date:   Mon, 13 Nov 2023 13:24:27 +0800
From:   Chao Gao <chao.gao@...el.com>
To:     Xin Li <xin3.li@...el.com>
CC:     <kvm@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-hyperv@...r.kernel.org>,
        <linux-kselftest@...r.kernel.org>, <seanjc@...gle.com>,
        <pbonzini@...hat.com>, <corbet@....net>, <kys@...rosoft.com>,
        <haiyangz@...rosoft.com>, <wei.liu@...nel.org>,
        <decui@...rosoft.com>, <tglx@...utronix.de>, <mingo@...hat.com>,
        <bp@...en8.de>, <dave.hansen@...ux.intel.com>, <x86@...nel.org>,
        <hpa@...or.com>, <vkuznets@...hat.com>, <peterz@...radead.org>,
        <ravi.v.shankar@...el.com>
Subject: Re: [PATCH v1 10/23] KVM: VMX: Add support for FRED context
 save/restore

>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 2c924075f6f1..c5a55810647f 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -1451,6 +1451,9 @@ static const u32 msrs_to_save_base[] = {
> 	MSR_STAR,
> #ifdef CONFIG_X86_64
> 	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
>+	MSR_IA32_FRED_RSP0, MSR_IA32_FRED_RSP1, MSR_IA32_FRED_RSP2,
>+	MSR_IA32_FRED_RSP3, MSR_IA32_FRED_STKLVLS, MSR_IA32_FRED_SSP1,
>+	MSR_IA32_FRED_SSP2, MSR_IA32_FRED_SSP3, MSR_IA32_FRED_CONFIG,

Need to handle the case where FRED MSRs are valid but KVM cannot virtualize
FRED, see kvm_probe_msr_to_save().

> #endif
> 	MSR_IA32_TSC, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA,
> 	MSR_IA32_FEAT_CTL, MSR_IA32_BNDCFGS, MSR_TSC_AUX,
>@@ -1890,6 +1893,16 @@ static int __kvm_set_msr(struct kvm_vcpu *vcpu, u32 index, u64 data,
> 
> 		data = (u32)data;
> 		break;
>+	case MSR_IA32_FRED_RSP0 ... MSR_IA32_FRED_CONFIG:
>+		if (host_initiated || guest_cpuid_has(vcpu, X86_FEATURE_FRED))
>+			break;

Nothing guarantees FRED MSRs/VMCS fields exist on the hardware here. Probably use
guest_cpu_cap_has()*.

*: https://lore.kernel.org/kvm/20231110235528.1561679-1-seanjc@google.com

>+
>+		/*
>+		 * Inject #GP upon FRED MSRs accesses from a non-FRED guest to
>+		 * make sure no malicious guest can write to FRED MSRs thus to
>+		 * corrupt host FRED MSRs.
>+		 */

I think injecting #GP here is simply because KVM should emulate hardware
behavior. To me, preventing guest from corrupting FRED MSRs is at most a
byproduct. I prefer to drop the comment.

>+		return 1;
> 	}
> 
> 	msr.data = data;
>@@ -1933,6 +1946,16 @@ int __kvm_get_msr(struct kvm_vcpu *vcpu, u32 index, u64 *data,
> 		    !guest_cpuid_has(vcpu, X86_FEATURE_RDPID))
> 			return 1;
> 		break;
>+	case MSR_IA32_FRED_RSP0 ... MSR_IA32_FRED_CONFIG:
>+		if (host_initiated || guest_cpuid_has(vcpu, X86_FEATURE_FRED))
>+			break;
>+
>+		/*
>+		 * Inject #GP upon FRED MSRs accesses from a non-FRED guest to
>+		 * make sure no malicious guest can write to FRED MSRs thus to
>+		 * corrupt host FRED MSRs.
>+		 */
>+		return 1;
> 	}
> 
> 	msr.index = index;
>-- 
>2.42.0
>
>