All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Amirmohammad Eftekhar <amirmohammadeftekhar@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
Subject: Re: [PATCH] KVM: nVMX: Reject nested VM-entry in wait-for-SIPI with a pending event injection
Date: Thu, 22 Oct 2026 14:42:46 -0700	[thread overview]
Message-ID: <atqDVkHjdJ2iF9Uv@google.com> (raw)
In-Reply-To: <cfe12756-208a-4a3c-bd46-cfdfeae6cdb1@gmail.com>

On Thu, Oct 22, 2026, Amirmohammad Eftekhar wrote:
> nested_check_guest_non_reg_state() only verifies that
> guest_activity_state is one of ACTIVE, HLT or WAIT_SIPI; it performs
> none of the activity-state cross-checks that VM entry requires.  Per
> Intel's SDM, "Checks on Guest Non-Register State", when the valid bit of
> the VM-entry interruption-information field is set the interruption to
> be delivered "must not be one that would normally be blocked while a
> logical processor is in the activity state", and for the wait-for-SIPI
> state "No interruptions are allowed".  The same section also requires
> that the activity state indicate the active state when the
> interruptibility-state field blocks by STI or MOV SS.
> 
> KVM accepts a vmcs12 that violates both rules, and hardware does not
> catch the miss: prepare_vmcs02() never writes GUEST_ACTIVITY_STATE, so
> vmcs02 always carries active, and wait-for-SIPI is emulated in software
> only after a successful entry (the mp_state = INIT_RECEIVED arm).  The
> injection, in contrast, is copied into vmcs02.  The physical VMLAUNCH
> therefore sees {active, valid injection} -- a legal pair -- and
> succeeds.  Because the offending field is stripped before hardware sees
> vmcs02, the missed-exit WARN in __nested_vmx_vmexit() can never fire for
> this class either.
> 
> From L1's point of view a VMLAUNCH that bare metal rejects with a
> VM-entry failure (invalid guest state) instead succeeds, with L2 left
> parked in wait-for-SIPI holding a stale VM_ENTRY_INTR_INFO in vmcs02.
> 
> Add the two activity-state cross-checks that KVM strips before hardware
> can enforce them. 

Please split this into two patches, one for each consistency check.  The justification
for why KVM needs these checks is the same, but the changes themselves are
independent.

> The HLT state additionally restricts which
> interruption types may be injected (external interrupt / NMI / #DB / #MC
> / pending-MTF only); that matrix is left for a follow-up as it needs the
> per-type table, whereas the two checks added here are unconditional and
> cover the fields KVM elides.
> 
> Signed-off-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
> ---
>  arch/x86/kvm/vmx/nested.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 9b0bfa2f8..66c9d1ffe 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -3311,6 +3311,24 @@ static int nested_check_guest_non_reg_state(struct
> vmcs12 *vmcs12)
>  	       vmcs12->guest_activity_state != GUEST_ACTIVITY_WAIT_SIPI))
>  		return -EINVAL;
>  +	/*
> +	 * The activity state must be active if the interruptibility state
> +	 * blocks by STI or MOV SS.
> +	 */

Eh, drop this comment, it's pretty much just restating the code verbatim.

> +	if (CC(vmcs12->guest_activity_state != GUEST_ACTIVITY_ACTIVE &&
> +	       (vmcs12->guest_interruptibility_info &
> +		(GUEST_INTR_STATE_STI | GUEST_INTR_STATE_MOV_SS))))
> +		return -EINVAL;
> +
> +	/*
> +	 * An injected event must not be one that would be blocked in the
> +	 * guest's activity state; no interruptions are allowed in the
> +	 * wait-for-SIPI state.
> +	 */

I think a TODO would be helpful.  Actually, what if we structure the entire
function as a switch-statement?  Gah, never mind, the check on MOV-SS and STI
blocking makes that annoying to deal with.

Anyways, how about this?

	/* 
	 * Injected events are disallowed if they are blocked by the activity
	 * state.  All injectable events are allowed in the active state, all
	 * are blocked by Wait-For-SIPI, and a small subset are allowed for HLT.
	 *
	 * TODO: Add consistency checks for HLT with an injected event.
	 */
	if (CC(vmcs12->guest_activity_state == GUEST_ACTIVITY_WAIT_SIPI &&
	       vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK))
		return -EINVAL;


> +	if (vmcs12->guest_activity_state == GUEST_ACTIVITY_WAIT_SIPI &&
> +	    CC(vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK))

Put the whole if-statement in the CC(), otherwise the error that shows up in the
trace will just be "vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK"
without any context.

> +		return -EINVAL;
> +
>  	return 0;
>  }
>  -- 2.54.0 (Apple Git-157)
> 

  reply	other threads:[~2026-10-22 21:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-22 21:12 [PATCH] KVM: nVMX: Reject nested VM-entry in wait-for-SIPI with a pending event injection Amirmohammad Eftekhar
2026-10-22 21:42 ` Sean Christopherson [this message]
2026-10-22 21:51   ` [PATCH v3 0/2] KVM: nVMX: Add the activity-state cross-checks VM-entry requires amirmohammad.eftekhar
2026-10-22 21:51   ` [PATCH v3 1/2] KVM: nVMX: Reject nested VM-entry in a non-active state blocked by STI/MOV-SS amirmohammad.eftekhar
2026-10-22 21:51   ` [PATCH v3 2/2] KVM: nVMX: Reject nested VM-entry in wait-for-SIPI with a pending event injection amirmohammad.eftekhar
  -- strict thread matches above, loose matches on Subject: below --
2026-10-16 23:52 [PATCH] " Amirmohammad Eftekhar

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=atqDVkHjdJ2iF9Uv@google.com \
    --to=seanjc@google.com \
    --cc=amirmohammad.eftekhar@cispa.de \
    --cc=amirmohammadeftekhar@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.