Re: [PATCH] KVM: nVMX: Fix bug of injecting L2 exception into L1

[Paolo Bonzini <pbonzini@redhat.com>]

On 19/11/2017 17:25, Liran Alon wrote:
> kvm_clear_exception_queue() should clear pending exception.
> This also includes exceptions which were only marked pending but not
> yet injected. This is because exception.pending is used for both L1
> and L2 to determine if an exception should be raised to guest.
> Note that an exception which is pending but not yet injected will
> be raised again once the guest will be resumed.
> 
> Consider the following scenario:
> 1) L0 KVM with ignore_msrs=false.
> 2) L1 prepare vmcs12 with the following:
>     a) No intercepts on MSR (MSR_BITMAP exist and is filled with 0).
>     b) No intercept for #GP.
>     c) vmx-preemption-timer is configured.
> 3) L1 enters into L2.
> 4) L2 reads an unhandled MSR that exists in MSR_BITMAP
> (such as 0x1fff).
> 
> L2 RDMSR could be handled as described below:
> 1) L2 exits to L0 on RDMSR and calls handle_rdmsr().
> 2) handle_rdmsr() calls kvm_inject_gp() which sets
> KVM_REQ_EVENT, exception.pending=true and exception.injected=false.
> 3) vcpu_enter_guest() consumes KVM_REQ_EVENT and calls
> inject_pending_event() which calls vmx_check_nested_events()
> which sees that exception.pending=true but
> nested_vmx_check_exception() returns 0 and therefore does nothing at
> this point. However let's assume it later sees vmx-preemption-timer
> expired and therefore exits from L2 to L1 by calling
> nested_vmx_vmexit().
> 4) nested_vmx_vmexit() calls prepare_vmcs12()
> which calls vmcs12_save_pending_event() but it does nothing as
> exception.injected is false. Also prepare_vmcs12() calls
> kvm_clear_exception_queue() which does nothing as
> exception.injected is already false.
> 5) We now return from vmx_check_nested_events() with 0 while still
> having exception.pending=true!
> 6) Therefore inject_pending_event() continues
> and we inject L2 exception to L1!...
> 
> This commit will fix above issue by changing step (4) to
> clear exception.pending in kvm_clear_exception_queue().
> 
> Fixes: 664f8e26b00c ("KVM: X86: Fix loss of exception which has not
> yet been injected")
> 
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
>  arch/x86/kvm/vmx.c | 1 -
>  arch/x86/kvm/x86.h | 1 +
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 7c3522a989d0..bee08782c781 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -11035,7 +11035,6 @@ static int vmx_check_nested_events(struct kvm_vcpu *vcpu, bool external_intr)
>  		if (vmx->nested.nested_run_pending)
>  			return -EBUSY;
>  		nested_vmx_inject_exception_vmexit(vcpu, exit_qual);
> -		vcpu->arch.exception.pending = false;
>  		return 0;
>  	}
>  
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index d0b95b7a90b4..6d112d8f799c 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -12,6 +12,7 @@
>  
>  static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu)
>  {
> +	vcpu->arch.exception.pending = false;
>  	vcpu->arch.exception.injected = false;
>  }
>  
> 

Finally queued, thanks.

Paolo