Re: WARNING in kvm_inject_emulated_page_fault

[Paolo Bonzini <pbonzini@redhat.com>]

On 04/06/20 12:53, Vitaly Kuznetsov wrote:
> Exception we're trying to inject comes from
> 
>  nested_vmx_get_vmptr()
>   kvm_read_guest_virt()
>    kvm_read_guest_virt_helper()
>      vcpu->arch.walk_mmu->gva_to_gpa()
> 
> but it seems it is only set if GVA to GPA convertion fails. In case it
> doesn't, we can still fail kvm_vcpu_read_guest_page() and return
> X86EMUL_IO_NEEDED but nested_vmx_get_vmptr() doesn't case what we return
> and does kvm_inject_emulated_page_fault(). This can happen when VMXON
> parameter is MMIO, for example.
> 
> How do fix this? We can either properly exit to userspace for handling
> or, if we decide that handling such requests makes little sense, just
> inject #GP if exception is not set, e.g. 
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 9c74a732b08d..a21e2f32f59b 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -4635,7 +4635,11 @@ static int nested_vmx_get_vmptr(struct kvm_vcpu *vcpu, gpa_t *vmpointer)
>                 return 1;
>  
>         if (kvm_read_guest_virt(vcpu, gva, vmpointer, sizeof(*vmpointer), &e)) {
> -               kvm_inject_emulated_page_fault(vcpu, &e);
> +               if (e.vector == PF_VECTOR)
> +                       kvm_inject_emulated_page_fault(vcpu, &e);
> +               else
> +                       kvm_inject_gp(vcpu, 0);
> +
>                 return 1;
>         }
> 

Yes, this is a plausible fix (with a comment explaining that we are 
taking a shortcut).  Perhaps a better check would be 

	r = kvm_read_guest_virt(vcpu, gva, vmpointer, sizeof(*vmpointer), &e);
	if (r != X86EMUL_CONTINUE) {
		if (r == X86EMUL_PROPAGATE_FAULT) {
			kvm_inject_emulated_page_fault(vcpu, &e);
		} else {
			/* ... */
			kvm_inject_gp(vcpu, 0);
		}
		return 1;
	}

Are you going to send a patch?

Paolo