Re: [PATCH v5 2/6] KVM: nVMX: Synthesize SHUTDOWN on RSM if L2 requires emulation

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The early return for `leave_smm` failure in `emulator_leave_smm` bypasses `kvm_leave_nested()`, leaving the vCPU in an inconsistent nested guest mode state.
--

--- Patch [2]: [PATCH v5 2/6] KVM: nVMX: Synthesize SHUTDOWN on RSM if L2 requires emulation ---
commit 4200f9610608af5b1ad7221b2b5cf164faad2906
Author: Sean Christopherson <[email protected]>

KVM: nVMX: Synthesize SHUTDOWN on RSM if L2 requires emulation

This commit synthesizes SHUTDOWN for L1 if L2 requires unhandleable emulation
after loading guest state from SMRAM during RSM. This prevents a misbehaving
L1 from triggering a KVM warning and bugging the VM.

> diff --git a/arch/x86/kvm/smm.c b/arch/x86/kvm/smm.c
> index a446487bdd5cf..656a38dad7e7a 100644
> --- a/arch/x86/kvm/smm.c
> +++ b/arch/x86/kvm/smm.c
> @@ -649,6 +649,10 @@ int emulator_leave_smm(struct x86_emulate_ctxt *ctxt)
>  #endif
>  		ret = rsm_load_state_32(ctxt, &smram.smram32);
>  
> +	if (ret == X86EMUL_CONTINUE &&
> +	    kvm_x86_call(unhandleable_emulation_required)(vcpu))
> +		ret = X86EMUL_UNHANDLEABLE;
> +
>  	/*
>  	 * If RSM fails and triggers shutdown, architecturally the shutdown
>  	 * occurs *before* the transition to guest mode.  But due to KVM's

[Severity: High]
This isn't a bug introduced by this patch, but looking at the earlier call
to kvm_x86_call(leave_smm)() in emulator_leave_smm() in arch/x86/kvm/smm.c:

    if (kvm_x86_call(leave_smm)(vcpu, &smram))
        return X86EMUL_UNHANDLEABLE;

If this fails, it returns directly and bypasses the kvm_leave_nested()
cleanup at the bottom of the function.

Can this leave the vCPU in an inconsistent nested guest mode state?

For example, if an unprivileged L1 hypervisor maliciously modifies the SMRAM
state (like an invalid CR3) for an active nested VM, could
emulator_leave_smm() call svm_leave_smm() -> enter_svm_guest_mode() ->
enter_guest_mode(vcpu) and fail in nested_svm_load_cr3()?

If so, would it return an error without leaving guest mode, bypass the
kvm_leave_nested() cleanup, and synthesize a nested VM-Exit to L1 instead
of properly transitioning L1 to SHUTDOWN?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2