Re: [Patch v2 1/4] KVM: SVM: Re-queue events that were never injected

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: svm: unconditional use of non-zero EVENTINJ without VALID check clobbers EXITINTINFO
--

commit ccdc89849239002e8efbee8f8c5f359259c07989
Author: Pratik R. Sampat <prsampat@amd.com>

KVM: SVM: Re-queue events that were never injected

This commit modifies SVM to re-queue undelivered events from the EVENTINJ
field to EXITINTINFO upon #VMEXIT. It accommodates cases like Enhanced SMT
Protection where VMRUN might stall and exit without entering guest mode.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d0971685034b9..e2be2cd86c320 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c

[ ... ]

> @@ -4323,6 +4324,27 @@ static void svm_complete_interrupts(struct kvm_vcpu *vcpu)
>  	svm->nmi_l1_to_l2 = false;
>  	svm->soft_int_injected = false;
>  
> +	/*
> +	 * Hardware clears EVENTINJ field when it injects an event.
> +	 * A non-empty EVENTINJ on #VMEXIT means the vCPU never entered guest
> +	 * mode, and thus that the event was never delivered.  Migrate the event
> +	 * to EXITINTINFO so that it's requeued instead of being dropped.
> +	 *
> +	 * An undelivered event doesn't imply a fatal VMEXIT_INVALID. With
> +	 * Enhanced SMT Protection, VMRUN may exit with an ordinary #VMEXIT
> +	 * without having injected that event into the guest.
> +	 *
> +	 * Clobbering EXITINTINFO is safe precisely because the vCPU never
> +	 * entered guest mode.
> +	 */
> +	if (control->event_inj) {

[Severity: High]
Is it possible for a stale EVENTINJ to clobber a valid EXITINTINFO here in
svm_complete_interrupts() in arch/x86/kvm/svm/svm.c?

By checking for a non-zero value instead of checking the explicit VALID bit,
this path might unconditionally overwrite EXITINTINFO when EVENTINJ has a
leftover value with its VALID bit cleared.

Since initial nested parameters and valid bit clearance are permitted by
KVM's nested_svm_check_event_inj(), would a hardware exit with a valid fault
record in EXITINTINFO be overwritten by the stale EVENTINJ?

Could this lead to silent dropping of legitimate L2 hardware events such as
page faults, debug exceptions, or machine checks during event delivery,
resulting in nested guest corruption?

> +		control->exit_int_info = control->event_inj;
> +		control->exit_int_info_err = control->event_inj_err;
> +		control->event_inj = 0;
> +
> +		exitintinfo = control->exit_int_info;
> +	}
>  
>  	/*
>  	 * If we've made progress since setting awaiting_iret_completion, we've

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791302373.git.prsampat@amd.com?part=1