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
