Re: [PATCH v2] KVM: x86: nSVM/nVMX: Fix handling triple fault on RSM instruction

[Wilczynski, Michal]

On 1/23/2024 1:15 AM, Michal Wilczynski wrote:
> Syzkaller found a warning triggered in nested_vmx_vmexit().
> vmx->nested.nested_run_pending is non-zero, even though we're in
> nested_vmx_vmexit(). Generally, trying  to cancel a pending entry is
> considered a bug. However in this particular scenario, the kernel
> behavior seems correct.
> 
> Syzkaller scenario:
> 1) Set up VCPU's
> 2) Run some code with KVM_RUN in L2 as a nested guest
> 3) Return from KVM_RUN
> 4) Inject KVM_SMI into the VCPU
> 5) Change the EFER register with KVM_SET_SREGS to value 0x2501
> 6) Run some code on the VCPU using KVM_RUN
> 7) Observe following behavior:
> 
> kvm_smm_transition: vcpu 0: entering SMM, smbase 0x30000
> kvm_entry: vcpu 0, rip 0x8000
> kvm_entry: vcpu 0, rip 0x8000
> kvm_entry: vcpu 0, rip 0x8002
> kvm_smm_transition: vcpu 0: leaving SMM, smbase 0x30000
> kvm_nested_vmenter: rip: 0x0000000000008002 vmcs: 0x0000000000007000
>                     nested_rip: 0x0000000000000000 int_ctl: 0x00000000
> 		    event_inj: 0x00000000 nested_ept=n guest
> 		    cr3: 0x0000000000002000
> kvm_nested_vmexit_inject: reason: TRIPLE_FAULT ext_inf1: 0x0000000000000000
>                           ext_inf2: 0x0000000000000000 ext_int: 0x00000000
> 			  ext_int_err: 0x00000000
> 
> What happened here is an SMI was injected immediately and the handler was
> called at address 0x8000; all is good. Later, an RSM instruction is
> executed in an emulator to return to the nested VM. em_rsm() is called,
> which leads to emulator_leave_smm(). A part of this function calls VMX/SVM
> callback, in this case vmx_leave_smm(). It attempts to set up a pending
> reentry to guest VM by calling nested_vmx_enter_non_root_mode() and sets
> vmx->nested.nested_run_pending to one. Unfortunately, later in
> emulator_leave_smm(), rsm_load_state_64() fails to write invalid EFER to
> the MSR. This results in em_rsm() calling triple_fault callback. At this
> point it's clear that the KVM should call the vmexit, but
> vmx->nested.nested_run_pending is left set to 1.
> 
> Similar flow goes for SVM, as the bug also reproduces on AMD platforms.
> 
> To address this issue, reset the nested_run_pending flag in the
> triple_fault handler. However, it's crucial to note that
> nested_pending_run cannot be cleared in all cases. It should only be
> cleared for the specific instruction requiring hardware VM-Enter to
> complete the emulation, such as RSM. Previously, there were instances
> where KVM prematurely synthesized a triple fault on nested VM-Enter. In
> these cases, it is not appropriate to zero the nested_pending_run.
> 
> To resolve this, introduce a new emulator flag indicating the need for
> HW VM-Enter to complete emulating RSM. Based on this flag, a decision can
> be made in vendor-specific triple fault handlers about whether
> nested_pending_run needs to be cleared.
> 
> Fixes: 759cbd59674a ("KVM: x86: nSVM/nVMX: set nested_run_pending on VM entry which is a result of RSM")
> Reported-by: Zheyu Ma <zheyuma97@xxxxxxxxx>
> Closes: https://lore.kernel.org/all/CAMhUBjmXMYsEoVYw_M8hSZjBMHh24i88QYm-RY6HDta5YZ7Wgw@xxxxxxxxxxxxxx
> Signed-off-by: Michal Wilczynski <michal.wilczynski@xxxxxxxxx>
> ---
> v2:
>  - added new emulator flags indicating whether an instruction needs a
>    VM-Enter to complete emulation (Sean)
>  - fix in SVM nested triple_fault handler (Sean)
>  - only clear nested_run_pending on RSM instruction (Sean)
> 
>  arch/x86/kvm/emulate.c     |  4 +++-
>  arch/x86/kvm/kvm_emulate.h |  2 ++
>  arch/x86/kvm/svm/nested.c  |  9 +++++++++
>  arch/x86/kvm/vmx/nested.c  | 12 ++++++++++++
>  4 files changed, 26 insertions(+), 1 deletion(-)
> 

Hi Sean,
Friendly ping,

Regards,
Michał