From: sashiko-bot@kernel.org
To: "Sohil Mehta" <sohil.mehta@intel.com>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v10 24/28] KVM: nVMX: Validate FRED-related VMCS fields
Date: Fri, 11 Sep 2026 22:29:43 +0000 [thread overview]
Message-ID: <20260911222944.4A8361F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260911213659.2025974-25-sohil.mehta@intel.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing cross-field consistency checks for FRED VM-entry and VM-exit controls against IA-32e mode, allowing invalid VMCS state to cause hardware VM-entry failures.
- [Low] Redundant nesting of CC() macro causes duplicate tracepoints on failure.
--
commit 20abff7abf2b901c4e2f9f748875173a59380be0
Author: Xin Li (Intel) <xin@zytor.com>
KVM: nVMX: Validate FRED-related VMCS fields
Extend nested VMX field validation to include FRED-specific VMCS fields,
mirroring hardware behavior. This enables support for nested FRED by ensuring
control and guest/host state fields are properly checked.
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 83e846fb3fc8c..3e0a1ed6440d4 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
[ ... ]
> @@ -3167,8 +3158,28 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu,
> if (CC(intr_info & INTR_INFO_RESVD_BITS_MASK))
> return -EINVAL;
>
> - /* VM-entry instruction length */
> + if (CC(intr_type == INTR_TYPE_RESERVED))
> + return -EINVAL;
> +
> + /*
> + * Only for hardware exceptions and when the CPU enumerates
> + * VMX nested-exception support, bit 13 (indicating a nested
> + * exception) has value 1. Otherwise it is reserved.
> + */
> + if (CC(has_nested_exception &&
> + (intr_type != INTR_TYPE_HARD_EXCEPTION ||
> + !nested_cpu_has_nested_exception(vcpu))))
> + return -EINVAL;
> +
> switch (intr_type) {
> + case INTR_TYPE_NMI_INTR:
> + if (CC(vector != NMI_VECTOR))
> + return -EINVAL;
> + break;
> + case INTR_TYPE_HARD_EXCEPTION:
> + if (CC(vector > 31))
> + return -EINVAL;
> + break;
> case INTR_TYPE_SOFT_EXCEPTION:
> case INTR_TYPE_SOFT_INTR:
> case INTR_TYPE_PRIV_SW_EXCEPTION:
> @@ -3176,6 +3187,28 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu,
> CC(vmcs12->vm_entry_instruction_len == 0 &&
> CC(!nested_cpu_has_zero_length_injection(vcpu))))
> return -EINVAL;
[Severity: Low]
Does this double CC() macro wrapping cause duplicate tracepoints?
If both conditions evaluate to true for soft exceptions and interrupts, the
inner CC() will emit a tracepoint, which then causes the outer CC() to also
evaluate to true and emit a second tracepoint. Is this intentional?
[ ... ]
> @@ -3262,9 +3295,27 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
> if (ia32e) {
> if (CC(!(vmcs12->host_cr4 & X86_CR4_PAE)))
> return -EINVAL;
> + if (nested_cpu_load_host_fred_state(vmcs12)) {
> + if (CC(vmcs12->host_ia32_fred_config & FRED_CONFIG_RESERVED) ||
> + CC(vmcs12->host_ia32_fred_rsp1 & GENMASK_ULL(5, 0)) ||
> + CC(vmcs12->host_ia32_fred_rsp2 & GENMASK_ULL(5, 0)) ||
> + CC(vmcs12->host_ia32_fred_rsp3 & GENMASK_ULL(5, 0)) ||
> + CC(vmcs12->host_ia32_fred_ssp1 & GENMASK_ULL(2, 0)) ||
> + CC(vmcs12->host_ia32_fred_ssp2 & GENMASK_ULL(2, 0)) ||
> + CC(vmcs12->host_ia32_fred_ssp3 & GENMASK_ULL(2, 0)) ||
> + CC(is_noncanonical_msr_address(vmcs12->host_ia32_fred_config & PAGE_MASK, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->host_ia32_fred_rsp1, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->host_ia32_fred_rsp2, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->host_ia32_fred_rsp3, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->host_ia32_fred_ssp1, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->host_ia32_fred_ssp2, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->host_ia32_fred_ssp3, vcpu)))
> + return -EINVAL;
> + }
> } else {
> if (CC(vmcs12->vm_entry_controls & VM_ENTRY_IA32E_MODE) ||
> CC(vmcs12->host_cr4 & X86_CR4_PCIDE) ||
> + CC(vmcs12->host_cr4 & X86_CR4_FRED) ||
> CC((vmcs12->host_rip) >> 32))
> return -EINVAL;
[Severity: High]
Are we missing a check for FRED host state when not in IA-32e mode?
If an L1 guest configures vmcs12 with VM_ENTRY_LOAD_IA32_FRED = 1 but
VM_ENTRY_IA32E_MODE = 0 (or equivalent host state), the FRED host state checks
are completely bypassed since ia32e is false here.
Would this missing cross-field consistency check allow an invalid vmcs12 state
to be accepted, failing to validate that FRED controls must be zero in 32-bit
mode?
[ ... ]
> @@ -3447,6 +3498,48 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
> CC((vmcs12->guest_bndcfgs & MSR_IA32_BNDCFGS_RSVD))))
> return -EINVAL;
>
> + if (ia32e) {
> + if (nested_cpu_load_guest_fred_state(vmcs12)) {
> + if (CC(vmcs12->guest_ia32_fred_config & FRED_CONFIG_RESERVED) ||
> + CC(vmcs12->guest_ia32_fred_rsp1 & GENMASK_ULL(5, 0)) ||
> + CC(vmcs12->guest_ia32_fred_rsp2 & GENMASK_ULL(5, 0)) ||
> + CC(vmcs12->guest_ia32_fred_rsp3 & GENMASK_ULL(5, 0)) ||
> + CC(vmcs12->guest_ia32_fred_ssp1 & GENMASK_ULL(2, 0)) ||
> + CC(vmcs12->guest_ia32_fred_ssp2 & GENMASK_ULL(2, 0)) ||
> + CC(vmcs12->guest_ia32_fred_ssp3 & GENMASK_ULL(2, 0)) ||
> + CC(is_noncanonical_msr_address(vmcs12->guest_ia32_fred_config & PAGE_MASK, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->guest_ia32_fred_rsp1, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->guest_ia32_fred_rsp2, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->guest_ia32_fred_rsp3, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->guest_ia32_fred_ssp1, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->guest_ia32_fred_ssp2, vcpu)) ||
> + CC(is_noncanonical_msr_address(vmcs12->guest_ia32_fred_ssp3, vcpu)))
> + return -EINVAL;
> + }
> + if (vmcs12->guest_cr4 & X86_CR4_FRED) {
> + unsigned int ss_dpl = VMX_AR_DPL(vmcs12->guest_ss_ar_bytes);
> +
> + if (CC(ss_dpl == 1 || ss_dpl == 2))
> + return -EINVAL;
> +
> + switch (ss_dpl) {
> + case 0:
> + if (CC(!(vmcs12->guest_cs_ar_bytes & VMX_AR_L_MASK)))
> + return -EINVAL;
> + break;
> + case 3:
> + if (CC(vmcs12->guest_rflags & X86_EFLAGS_IOPL))
> + return -EINVAL;
> + if (CC(vmcs12->guest_interruptibility_info & GUEST_INTR_STATE_STI))
> + return -EINVAL;
> + break;
> + }
> + }
> + } else {
> + if (CC(vmcs12->guest_cr4 & X86_CR4_FRED))
> + return -EINVAL;
> + }
[Severity: High]
Similarly, does this else branch for the guest state miss failing the entry if
nested_cpu_load_guest_fred_state(vmcs12) is true?
Without this cross-field check, a malicious L1 hypervisor could provide a
vmcs12 with VM_ENTRY_LOAD_IA32_FRED = 1 but VM_ENTRY_IA32E_MODE = 0, causing
KVM to accept the invalid configuration and copy the VM_ENTRY_LOAD_IA32_FRED
bit directly into vmcs02 during prepare_vmcs02.
This would lead to an L0 hardware VM-entry failure instead of the required
software synthesis of an L1 VMFail, causing a host-side denial of service
against the VM process.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260911213659.2025974-1-sohil.mehta@intel.com?part=24
next prev parent reply other threads:[~2026-09-11 22:29 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-11 21:36 [PATCH v10 00/28] KVM: Enable FRED support with KVM VMX Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 01/28] KVM: VMX: Enable support for secondary VM exit controls Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 02/28] KVM: VMX: Initialize VM entry/exit FRED controls in vmcs_config Sohil Mehta
2026-09-11 22:00 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 03/28] KVM: VMX: Disable FRED if FRED consistency checks fail Sohil Mehta
2026-09-11 22:08 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 04/28] x86/cea: Prefix event stack names with ESTACK_ Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 05/28] x86/cea: Use array indexing to simplify exception stack access Sohil Mehta
2026-09-11 21:57 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 06/28] x86/fred: Export this_cpu_fred_rsp() for KVM usage Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 07/28] KVM: VMX: Initialize VMCS FRED fields Sohil Mehta
2026-09-11 22:10 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 08/28] KVM: VMX: Set FRED MSR intercepts Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 09/28] KVM: VMX: Save/restore guest FRED RSP0 Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 10/28] KVM: VMX: Add support for saving and restoring FRED MSRs Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 11/28] KVM: x86: Add a helper to detect if FRED is enabled for a vCPU Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 12/28] KVM: x86: Add a new save/restore flag for FRED metadata Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 13/28] KVM: VMX: Virtualize FRED nested exception tracking Sohil Mehta
2026-09-11 22:09 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 14/28] KVM: VMX: Virtualize FRED event_data Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 15/28] KVM: x86: Include CR4.FRED in the emulator CR4 write mask Sohil Mehta
2026-09-11 22:14 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 16/28] KVM: x86: Mark CR4.FRED as not reserved Sohil Mehta
2026-09-11 22:14 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 17/28] KVM: x86: Handle CR4.FRED when emulating RSM Sohil Mehta
2026-09-11 22:15 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 18/28] KVM: VMX: Dump FRED context in dump_vmcs() Sohil Mehta
2026-09-11 22:11 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 19/28] KVM: x86: Advertise support for FRED Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 20/28] KVM: nVMX: Enable support for secondary VM exit controls Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 21/28] KVM: nVMX: Handle FRED VMCS fields in nested VMX context Sohil Mehta
2026-09-11 22:34 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 22/28] KVM: nVMX: Restrict event data VMCS fields to FRED-supported hosts Sohil Mehta
2026-09-11 22:20 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 23/28] KVM: nVMX: Shadow ORIGINAL_EVENT_DATA and INJECTED_EVENT_DATA fields Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 24/28] KVM: nVMX: Validate FRED-related VMCS fields Sohil Mehta
2026-09-11 22:29 ` sashiko-bot [this message]
2026-09-11 21:36 ` [PATCH v10 25/28] KVM: nVMX: Enable VMX FRED controls Sohil Mehta
2026-09-11 22:37 ` sashiko-bot
2026-09-11 21:36 ` [PATCH v10 26/28] KVM: selftests: Add FRED MSRs to msrs_test Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 27/28] KVM: selftests: Add a new VM guest mode to run user level code Sohil Mehta
2026-09-11 21:36 ` [PATCH v10 28/28] KVM: selftests: Add fred exception tests Sohil Mehta
2026-09-11 22:32 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260911222944.4A8361F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=kvm@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=sohil.mehta@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox