linux-kernel - Re: [PATCH v2 1/2] KVM: nVMX: Properly handle kvm_read/write_guest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200605200651.GC11449@linux.intel.com>
Date:   Fri, 5 Jun 2020 13:06:51 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Vitaly Kuznetsov <vkuznets@...hat.com>
Cc:     kvm@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] KVM: nVMX: Properly handle
 kvm_read/write_guest_virt*() result

On Fri, Jun 05, 2020 at 01:59:05PM +0200, Vitaly Kuznetsov wrote:
> Introduce vmx_handle_memory_failure() as an interim solution.

Heh, "interim".  I'll take the over on that :-D.

> Note, nested_vmx_get_vmptr() now has three possible outcomes: OK, PF,
> KVM_EXIT_INTERNAL_ERROR and callers need to know if userspace exit is
> needed (for KVM_EXIT_INTERNAL_ERROR) in case of failure. We don't seem
> to have a good enum describing this tristate, just add "int *ret" to
> nested_vmx_get_vmptr() interface to pass the information.
> 
> Reported-by: syzbot+2a7156e11dc199bdbd8a@...kaller.appspotmail.com
> Suggested-by: Sean Christopherson <sean.j.christopherson@...el.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.com>
> ---

...

> +/*
> + * Handles kvm_read/write_guest_virt*() result and either injects #PF or returns
> + * KVM_EXIT_INTERNAL_ERROR for cases not currently handled by KVM. Return value
> + * indicates whether exit to userspace is needed.
> + */
> +int vmx_handle_memory_failure(struct kvm_vcpu *vcpu, int r,
> +			      struct x86_exception *e)
> +{
> +	if (r == X86EMUL_PROPAGATE_FAULT) {
> +		kvm_inject_emulated_page_fault(vcpu, e);
> +		return 1;
> +	}
> +
> +	/*
> +	 * In case kvm_read/write_guest_virt*() failed with X86EMUL_IO_NEEDED
> +	 * while handling a VMX instruction KVM could've handled the request

A nit similar to your observation on the shortlog, this isn't limited to VMX
instructions.

> +	 * correctly by exiting to userspace and performing I/O but there
> +	 * doesn't seem to be a real use-case behind such requests, just return
> +	 * KVM_EXIT_INTERNAL_ERROR for now.
> +	 */
> +	vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> +	vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
> +	vcpu->run->internal.ndata = 0;
> +
> +	return 0;
> +}