Re: [PATCH 2/2] KVM: nVMX: avoid losing the posted notification interrupt when exiting L2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] kvm: x86: out-of-bounds write in kvm_lapic_set_irr due to u16 promotion bypass
- [High] kvm: x86: guest-triggerable WARN_ON_ONCE and pi_pending state leak
--

commit 00e061d66b6985b4612f371acc0c5f41a6c597b6
Author: Maxim Levitsky <mlevitsk@redhat.com>

KVM: nVMX: avoid losing the posted notification interrupt when exiting L2

This commit detects if a posted notification interrupt is pending during
a nested VM exit. If the target vCPU is about to exit to L1, the
interrupt is injected into L1's APIC to avoid losing it.

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 11362aad2bc8a..66c4cf7f6e00f 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -2445,7 +2445,7 @@ static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct loaded_vmcs *vmcs0
>  			 ~PIN_BASED_VMX_PREEMPTION_TIMER);
>  
>  	/* Posted interrupts setting is only taken from vmcs12.  */
> -	vmx->nested.pi_pending = false;
> +	WARN_ON_ONCE(vmx->nested.pi_pending);

[Severity: High]
Is it possible for an L1 guest to maliciously trigger this warning and crash
the host if panic_on_warn is set?

Since the patch no longer clears pi_pending, could this state leak occur if a
nested VM-entry fails early after enter_guest_mode() is called? For example,
in nested_vmx_enter_non_root_mode(), the failure path
vmentry_fail_vmexit_guest_mode calls leave_guest_mode() and returns to L1
without executing __nested_vmx_vmexit(). This would leave pi_pending
uncleared.

Additionally, could there be a store-load race here? During normal VM exit,
leave_guest_mode() clears HF_GUEST_MASK, but there is no memory barrier
enforcing visibility before pi_pending is checked further down in
__nested_vmx_vmexit(). A concurrent posted interrupt delivery might read
guest_mode == true and set pi_pending = true, while the exit path reads
pi_pending == false.

In both cases, pi_pending remains true when returning to L1, triggering this
warning on the next nested VM-entry.

[ ... ]

> @@ -5151,6 +5151,14 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
>  	kvm_clear_exception_queue(vcpu);
>  	kvm_clear_interrupt_queue(vcpu);
>  
> +	if (lapic_in_kernel(vcpu) && vmx->nested.pi_pending) {
> +		vmx->nested.pi_pending = 0;
> +		if (!WARN_ON_ONCE(vmx->nested.posted_intr_nv == -1)) {
> +			kvm_lapic_set_irr(vmx->nested.posted_intr_nv, vcpu->arch.apic);

[Severity: Critical]
Does this guard actually protect against injecting an invalid vector?

Looking at struct nested_vmx in arch/x86/kvm/vmx/vmx.h, posted_intr_nv is
defined as a u16. When it is assigned -1 in prepare_vmcs02_early(), it
truncates to 65535.

Due to C integer promotion rules, the expression
vmx->nested.posted_intr_nv == -1 compares the u16 value 65535 to the int
literal -1, which evaluates to false. This bypasses the safety guard because
!WARN_ON_ONCE(false) is true.

If pi_pending was leaked as described above and PI is subsequently disabled by
L1, wouldn't kvm_lapic_set_irr() be called with vector 65535? Inside
kvm_lapic_set_irr(), calculating the bit offset for 65535 falls far outside
the 4KB APIC register page, which could result in a severe out-of-bounds write
into adjacent host memory.

> +			kvm_make_request(KVM_REQ_EVENT, vcpu);
> +		}
> +	}
> +
>  	vmx_switch_vmcs(vcpu, &vmx->vmcs01);
>  
>  	kvm_nested_vmexit_handle_ibrs(vcpu);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009205036.672523-2-mlevitsk@redhat.com?part=2