linux-kernel - Re: [PATCH v2 07/25] KVM: VMX: Set intercept for FRED 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] [day] [month] [year] [list]

Message-ID: <d65e62d2-ca64-4b29-8656-bb8411fe837d@zytor.com>
Date: Wed, 18 Sep 2024 01:35:51 -0700
From: Xin Li <xin@...or.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: Chao Gao <chao.gao@...el.com>, Xin Li <xin3.li@...el.com>,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kselftest@...r.kernel.org,
        pbonzini@...hat.com, corbet@....net, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
        x86@...nel.org, hpa@...or.com, shuah@...nel.org, vkuznets@...hat.com,
        peterz@...radead.org, ravi.v.shankar@...el.com
Subject: Re: [PATCH v2 07/25] KVM: VMX: Set intercept for FRED MSRs

>> MSR_IA32_FRED_SSP0 is an alias of the CET MSR_IA32_PL0_SSP and likely to
>> be used in the same way as FRED RSP0, i.e., host FRED SSP0 _should_ be
>> restored in arch_exit_to_user_mode_prepare().  However as of today Linux
>> has no plan to utilize kernel shadow stack thus no one cares host FRED
>> SSP0 (no?).  But lets say anyway it is host's responsibility to manage
>> host FRED SSP0, then KVM only needs to take care of guest FRED SSP0
>> (just like how KVM should handle guest FRED RSP0) even before the
>> supervisor shadow stack feature is advertised to guest.
> 
> Heh, I'm not sure what your question is, or if there even is a question.  KVM
> needs to context switch FRED SSP0 if FRED is exposed to the guest, but presumably
> that will be done through XSAVE state?  If that's the long term plan, I would
> prefer to focus on merging CET virtualization first, and then land FRED virtualization
> on top so that KVM doesn't have to carry intermediate code to deal with the aliased
> MSR.

You mean the following patch set, right?

https://lore.kernel.org/kvm/20240531090331.13713-1-weijiang.yang@intel.com/

I think it's probably better to do so.  Otherwise a patch to explicitly
save/restore FRED SSP0 would be needed before the CET patch set lands
and then reverted immediately after:

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f828f03d48ab..c790cb7a7d6b 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1343,8 +1343,10 @@ void vmx_prepare_switch_to_guest(struct kvm_vcpu 
*vcpu)

         wrmsrl(MSR_KERNEL_GS_BASE, vmx->msr_guest_kernel_gs_base);

-       if (cpu_feature_enabled(X86_FEATURE_FRED) && guest_can_use(vcpu, 
X86_FEATURE_FRED))
+       if (cpu_feature_enabled(X86_FEATURE_FRED) && guest_can_use(vcpu, 
X86_FEATURE_FRED)) {
                 wrmsrns(MSR_IA32_FRED_RSP0, vmx->msr_guest_fred_rsp0);
+               wrmsrns(MSR_IA32_FRED_SSP0, vmx->msr_guest_fred_ssp0);
+       }
  #else
         savesegment(fs, fs_sel);
         savesegment(gs, gs_sel);
@@ -1393,6 +1395,8 @@ static void vmx_prepare_switch_to_host(struct 
vcpu_vmx *vmx)
         if (cpu_feature_enabled(X86_FEATURE_FRED) && 
guest_can_use(&vmx->vcpu, X86_FEATURE_FRED)) {
                 vmx->msr_guest_fred_rsp0 = read_msr(MSR_IA32_FRED_RSP0);
                 fred_sync_rsp0(vmx->msr_guest_fred_rsp0);
+
+               vmx->msr_guest_fred_ssp0 = read_msr(MSR_IA32_FRED_SSP0);
         }
  #endif
         load_fixmap_gdt(raw_smp_processor_id());
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 9012428984de..d1b9352f56c7 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -282,6 +282,7 @@ struct vcpu_vmx {
         u64                   msr_host_kernel_gs_base;
         u64                   msr_guest_kernel_gs_base;
         u64                   msr_guest_fred_rsp0;
+       u64                   msr_guest_fred_ssp0;
  #endif

         u64                   spec_ctrl;

> 
> Ugh, but what happens if a CPU (or the host kernel) supports FRED but not CET SS?
> Or is that effectively an illegal combination?

The FRED Spec says:

IA32_FRED_SSP1, IA32_FRED_SSP2, and IA32_FRED_SSP3 (MSR indices 1D1H–
1D3H). Together with the existing MSR IA32_PL0_SSP (MSR index 6A4H), 
these are the FRED SSP MSRs.

The FRED SSP MSRs are supported by any processor that enumerates
CPUID.(EAX=7,ECX=1):EAX.FRED[bit 17] as 1. If such a processor does not 
support CET, FRED transitions will not use the MSRs (because shadow 
stacks are not enabled), but the MSRs would still be accessible using 
RDMSR and WRMSR.


So they are independent, just that FRED SSP MSRs are NOT used if
supervisor shadow stacks are not enabled (obviously Qemu can be
configured to not advertise CET but FRED).

When FRED is advertised to a guest, KVM should allow FRED SSP MSRs
accesses through disabling FRED SSP MSRs interception no matter whether
supervisor shadow stacks are enabled or not.

Thanks!
     Xin