Re: [PATCH v2] KVM: x86: Mark nested RSM-created VM-Enter as untrusted

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Leaked `nested_run_pending` flag in VMX permanently blocks event injection to L1 after recovering from an invalid untrusted nested state.
--

--- Patch [1]: [PATCH v2] KVM: x86: Mark nested RSM-created VM-Enter as untrusted ---
commit 096beaf30ae7e6e858dddc7bb2b2a980624ad7d1
Author: Hao Zhang <zhanghao1@kylinos.cn>

KVM: x86: Mark nested RSM-created VM-Enter as untrusted

RSM can restore state that indicates the vCPU was running L2 when SMI
arrived.  KVM then re-enters nested guest mode and marks nested_run_pending
so that the next KVM_RUN completes the pending nested transition.

Link to v1: https://lore.kernel.org/all/al3Qbq-jUYE-_72N@192.168.1.215/

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 4d2bacd00ec4f..f54757bd83821 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c

[ ... ]

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index cc75feec05daa..333072cc9bce4 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -6720,7 +6720,8 @@ static int __vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
>  	 * invalid guest state should never happen as that means KVM knowingly
>  	 * allowed a nested VM-Enter with an invalid vmcs12.  More below.
>  	 */
> -	if (KVM_BUG_ON(vcpu->arch.nested_run_pending, vcpu->kvm))
> +	if (KVM_BUG_ON(vcpu->arch.nested_run_pending == KVM_NESTED_RUN_PENDING,
> +		       vcpu->kvm))
>  		return -EIO;

[Severity: High]
Does this allow the KVM_NESTED_RUN_PENDING_UNTRUSTED flag to leak and lock up
the vCPU?

If a malicious L2 guest modifies SMRAM to create an invalid nested state, then
executes RSM, vmx_leave_smm() restores this state and sets nested_run_pending
to KVM_NESTED_RUN_PENDING_UNTRUSTED.

During vmx_vcpu_run(), the invalid state triggers an early return because of
vmx->vt.emulation_required:

	if (unlikely(vmx->vt.emulation_required)) {
		vmx->fail = 0;
		vmx->vt.exit_reason.full = EXIT_REASON_INVALID_STATE;
		...
		return EXIT_FASTPATH_NONE;
	}

This skips the code block at the end of vmx_vcpu_run() that normally clears
vcpu->arch.nested_run_pending = 0.

When __vmx_handle_exit() is subsequently called, the modified KVM_BUG_ON
allows this UNTRUSTED state to fall through without returning -EIO.  KVM then
reaches this block:

		if (vmx->vt.emulation_required) {
			nested_vmx_vmexit(vcpu, EXIT_REASON_TRIPLE_FAULT, 0, 0);
			return 1;
		}

This synthesizes a TRIPLE_FAULT and transitions the vCPU back to L1, but it
doesn't clear the nested_run_pending flag before doing so.

Once back in L1, vmx_interrupt_allowed() unconditionally returns -EBUSY
because the flag is still set:

	if (vcpu->arch.nested_run_pending)
		return -EBUSY;

Will this cause kvm_check_and_inject_events() to constantly request an
immediate exit, permanently livelocking the host vCPU thread in an infinite
VM-Enter/VM-Exit loop?

>  
>  	if (is_guest_mode(vcpu)) {

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/al8M4gCwsWXS_jMs@192.168.1.215?part=1