Re: [PATCH v10 03/28] KVM: VMX: Disable FRED if FRED consistency checks fail

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Flawed boolean logic in `vmx_check_entry_exit_pairs` allows KVM to expose FRED on nested hypervisors or broken hardware with incomplete exit controls, bypassing the intended mitigation.
--

commit 125b825fe12cce02b0f6ac3d6d7b8bd72cd92cfb
Author: Xin Li (Intel) <xin@zytor.com>

KVM: VMX: Disable FRED if FRED consistency checks fail

This commit adds a check to disable the FRED CPU capability if FRED
consistency checks fail during VMX initialization. It clears the
X86_FEATURE_FRED capability by verifying the presence of
VM_ENTRY_LOAD_IA32_FRED in the entry controls.

> 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

[ ... ]

> @@ -8122,6 +8122,9 @@ static __init void vmx_set_cpu_caps(void)
>  		kvm_cpu_cap_check_and_set(X86_FEATURE_DTES64);
>  	}
>  
> +	if (!cpu_has_vmx_fred())
> +		kvm_cpu_cap_clear(X86_FEATURE_FRED);
> +

[Severity: High]
This isn't a bug introduced by this patch, but is it possible that this
mitigation can be bypassed due to a logical flaw in the pre-existing
vmx_check_entry_exit_pairs() macro?

This new mitigation clears X86_FEATURE_FRED if cpu_has_vmx_fred() returns
false, which checks for VM_ENTRY_LOAD_IA32_FRED. The presence of this entry
control relies on the consistency checks performed earlier.

Looking at the pre-existing vmx_check_entry_exit_pairs() macro in
arch/x86/kvm/vmx/vmx.c:

        if (!(entry_controls & n_ctrl) == !(exit_controls & x_ctrl))    \
                continue;                                               \

The x_ctrl parameter used for FRED is a multi-bit mask:

        { VM_ENTRY_LOAD_IA32_FRED,
          SECONDARY_VM_EXIT_SAVE_IA32_FRED | SECONDARY_VM_EXIT_LOAD_IA32_FRED },

If the underlying hardware (or a nested hypervisor) provides incomplete
support and only sets one of the exit bits (for example, just SAVE), the
bitwise AND (exit_controls & x_ctrl) evaluates to a non-zero value. The
logical NOT of this non-zero value evaluates to 0.

If the entry control bit is present, its logical NOT is also 0. Thus, the
equality check passes (0 == 0), the macro skips clearing the entry bit, and
VM_ENTRY_LOAD_IA32_FRED incorrectly remains set.

Because VM_ENTRY_LOAD_IA32_FRED is still set, cpu_has_vmx_fred() evaluates
to true, and this new mitigation is bypassed. This could allow a guest to
escalate privileges if the CPU fails to restore the host's FRED MSRs on
VM-exit due to the incomplete exit controls.

Does the macro need to be updated to ensure all bits in a multi-bit mask are
present before considering the controls consistent?

>  	if (!enable_pmu)
>  		kvm_cpu_cap_clear(X86_FEATURE_PDCM);
>  	kvm_caps.supported_perf_cap = vmx_get_perf_capabilities();

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