Re: [PATCH v10 07/28] KVM: VMX: Initialize VMCS FRED fields

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Host FRED MSRs are left uninitialized/corrupted on VM-Exit if KVM skips explicit initialization while the hardware capabilities dictate loading them.
- [Critical] VM-Exit unconditionally corrupts the host's IA32_FRED_RSP0 MSR (alias for IA32_SYSENTER_ESP), causing host ring-3 to ring-0 transitions to use the wrong stack in FRED-enabled hosts when non-FRED guests run.
- [Critical] Guest FRED state is not explicitly initialized when the host has disabled FRED in software, leaving VMCS fields undefined.
--

commit 81aa8f5538dba9476f74b76b06da72ada534dd41
Author: Xin Li (Intel) <xin@zytor.com>
Subject: KVM: VMX: Initialize VMCS FRED fields

This commit initializes host and guest VMCS fields for FRED (Flexible Return
and Event Delivery). It populates the host fields with host MSR values and
sets the guest fields to zero. The initialization is gated by checking if
the X86_FEATURE_FRED CPU feature is enabled.

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1562,6 +1562,12 @@ void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu)
>  				    (unsigned long)(cpu_entry_stack(cpu) + 1));
>  		}

[Severity: Critical]
Could unconditionally writing to HOST_IA32_SYSENTER_ESP corrupt the host's 
IA32_FRED_RSP0 MSR?

Because MSR_IA32_FRED_RSP0 is architecturally aliased to
MSR_IA32_SYSENTER_ESP, every VM-Exit will load this small trampoline stack
into MSR_IA32_FRED_RSP0.

If a guest does not have FRED enabled, KVM's vmx_prepare_switch_to_host()
skips syncing the RSP0 state. The per-CPU cache __this_cpu_read(fred_rsp0)
is never updated to reflect the hardware change, meaning
fred_update_rsp0() assumes the MSR is already correct.

When the host returns to userspace and takes a syscall or interrupt, won't
FRED deliver the event to ring 0 using this tiny trampoline stack instead
of the proper task stack?

> +		if (cpu_feature_enabled(X86_FEATURE_FRED) && kvm_cpu_cap_has(X86_FEATURE_FRED)) {
> +			vmcs_write64(HOST_IA32_FRED_RSP1, this_cpu_fred_rsp(FRED_STACK_LEVEL_1));
> +			vmcs_write64(HOST_IA32_FRED_RSP2, this_cpu_fred_rsp(FRED_STACK_LEVEL_2));
> +			vmcs_write64(HOST_IA32_FRED_RSP3, this_cpu_fred_rsp(FRED_STACK_LEVEL_3));
> +		}

[Severity: Critical]
Does checking cpu_feature_enabled(X86_FEATURE_FRED) here leave host VMCS 
fields uninitialized if the host booted with FRED disabled in software 
(e.g., via clearcpuid=fred)?

The SECONDARY_VM_EXIT_LOAD_IA32_FRED control is set statically in 
setup_vmcs_config() based purely on hardware capability MSRs. If KVM 
enables the exit control but skips initializing these fields because 
of this software check, the CPU will validate uninitialized memory 
(left over from VMCLEAR) on VM-Exit.

Can this cause a fatal VM-Exit failure due to non-canonical addresses 
or reserved bits in the uninitialized Host-State area?

[ ... ]

> @@ -4500,6 +4506,17 @@ void vmx_set_constant_host_state(struct vcpu_vmx *vmx)
>  	 */
>  	vmcs_write16(HOST_DS_SELECTOR, 0);
>  	vmcs_write16(HOST_ES_SELECTOR, 0);
> +
> +	if (cpu_feature_enabled(X86_FEATURE_FRED) && kvm_cpu_cap_has(X86_FEATURE_FRED)) {

[Severity: Critical]
Does this condition share the same risk as vmx_vcpu_load_vmcs(), where a
software-disabled FRED still forces a hardware VM-Exit load of these
uninitialized host fields?

> +		/* FRED CONFIG and STKLVLS are the same on all CPUs */
> +		vmcs_write64(HOST_IA32_FRED_CONFIG, kvm_host.fred_config);
> +		vmcs_write64(HOST_IA32_FRED_STKLVLS, kvm_host.fred_stklvls);
> +
> +		/* Linux doesn't support kernel shadow stacks, thus SSPs are 0s */
> +		vmcs_write64(HOST_IA32_FRED_SSP1, 0);
> +		vmcs_write64(HOST_IA32_FRED_SSP2, 0);
> +		vmcs_write64(HOST_IA32_FRED_SSP3, 0);
> +	}
>  #else

[ ... ]

> @@ -5030,6 +5047,17 @@ static void init_vmcs(struct vcpu_vmx *vmx)
>  	}
>  
>  	vmx_setup_uret_msrs(vmx);
> +
> +	if (cpu_feature_enabled(X86_FEATURE_FRED) && kvm_cpu_cap_has(X86_FEATURE_FRED)) {

[Severity: Critical]
Similarly to the host state, does this condition skip initializing the
guest FRED fields if FRED is disabled in software?

If the hardware supports it, setup_vmcs_config() statically sets
VM_ENTRY_LOAD_IA32_FRED. Skipping vmcs_write64() here leaves these fields
architecturally undefined after a VMCLEAR operation.

Will this trigger an immediate VM-Entry failure when the CPU attempts
to load the uninitialized guest FRED fields from the VMCS?

> +		vmcs_write64(GUEST_IA32_FRED_CONFIG, 0);
> +		vmcs_write64(GUEST_IA32_FRED_RSP1, 0);
> +		vmcs_write64(GUEST_IA32_FRED_RSP2, 0);
> +		vmcs_write64(GUEST_IA32_FRED_RSP3, 0);
> +		vmcs_write64(GUEST_IA32_FRED_STKLVLS, 0);
> +		vmcs_write64(GUEST_IA32_FRED_SSP1, 0);
> +		vmcs_write64(GUEST_IA32_FRED_SSP2, 0);
> +		vmcs_write64(GUEST_IA32_FRED_SSP3, 0);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911213659.2025974-1-sohil.mehta@intel.com?part=7