Re: [PATCH v10 22/28] KVM: nVMX: Restrict event data VMCS fields to FRED-supported hosts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete restrictions for new VMCS fields exposing unsupported fields to L1 guests.
--

commit 54389ae57bfff5e56dff4d41b4c42f872fb1693f
Author: Xin Li <xin@zytor.com>

KVM: nVMX: Restrict event data VMCS fields to FRED-supported hosts

This commit restricts L1 guest access to the ORIGINAL_EVENT_DATA and
INJECTED_EVENT_DATA VMCS fields via VMREAD or VMWRITE when FRED support
is disabled. It updates cpu_has_vmcs12_field in arch/x86/kvm/vmx/vmcs12.c
to check cpu_has_vmx_fred for these specific fields.

> diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
> --- a/arch/x86/kvm/vmx/vmcs12.c
> +++ b/arch/x86/kvm/vmx/vmcs12.c
[ ... ]
> @@ -222,6 +222,9 @@ static __init bool cpu_has_vmcs12_field(unsigned int idx)
>  	case HOST_SSP:
>  	case HOST_INTR_SSP_TABLE:
>  		return cpu_has_load_cet_ctrl();
> +	VMCS12_CASE64(ORIGINAL_EVENT_DATA):
> +	VMCS12_CASE64(INJECTED_EVENT_DATA):
> +		return cpu_has_vmx_fred();
>  
>  	/* KVM always emulates PML and the VMX preemption timer in software. */
>  	case GUEST_PML_INDEX:
>  	case VMX_PREEMPTION_TIMER_VALUE:
>  	default:
>  		return true;

[Severity: Medium]
Does this code leave other FRED-related fields exposed to L1?

While this restricts ORIGINAL_EVENT_DATA and INJECTED_EVENT_DATA, it
appears to omit all GUEST_IA32_FRED_*, HOST_IA32_FRED_*, and
SECONDARY_VM_EXIT_CONTROLS fields.

Because cpu_has_vmcs12_field falls through to the default case and
returns true for unhandled fields, wouldn't these remaining FRED MSR
fields and SECONDARY_VM_EXIT_CONTROLS be unconditionally exposed to an
L1 guest?

If a nested hypervisor (L1) executes VMREAD or VMWRITE on these specific
fields (for example, GUEST_IA32_FRED_CONFIG) on a host without FRED or
secondary VM-exit controls, KVM intercepts the instruction and calls
vmcs12_read_any or vmcs12_write_any. 

Because they are incorrectly populated in the vmcs12_field_offsets array 
by default, it seems they would be allowed instead of properly generating 
a VMfail (VMXERR_UNSUPPORTED_VMCS_COMPONENT), violating the CPUID-based 
capability masking.

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