linux-kernel - Re: [PATCH v2 2/2] KVM: x86: On emulation failure, convey the exit reason to userspace

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: <YOhvfDfqypLCRZuO@google.com>
Date:   Fri, 9 Jul 2021 15:47:08 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     David Edmondson <david.edmondson@...cle.com>
Cc:     linux-kernel@...r.kernel.org, Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Borislav Petkov <bp@...en8.de>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        David Matlack <dmatlack@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>, x86@...nel.org,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Joao Martins <joao.m.martins@...cle.com>
Subject: Re: [PATCH v2 2/2] KVM: x86: On emulation failure, convey the exit
 reason to userspace

On Tue, Jul 06, 2021, David Edmondson wrote:
> Should instruction emulation fail, include the VM exit reason in the
> emulation_failure data passed to userspace, in order that the VMM can
> report it as a debugging aid when describing the failure.

...

> @@ -7473,7 +7474,14 @@ static void prepare_emulation_failure_exit(struct kvm_vcpu *vcpu)
>  		memcpy(run->emulation_failure.insn_bytes,
>  		       ctxt->fetch.data, insn_size);
>  	}
> +
> +	run->emulation_failure.ndata = 4;
> +	run->emulation_failure.flags |=
> +		KVM_INTERNAL_ERROR_EMULATION_FLAG_EXIT_REASON;
> +	run->emulation_failure.exit_reason =
> +		static_call(kvm_x86_get_exit_reason)(vcpu);
>  }

...

> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index d9e4aabcb31a..863195371272 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -282,6 +282,7 @@ struct kvm_xen_exit {
>  
>  /* Flags that describe what fields in emulation_failure hold valid data. */
>  #define KVM_INTERNAL_ERROR_EMULATION_FLAG_INSTRUCTION_BYTES (1ULL << 0)
> +#define KVM_INTERNAL_ERROR_EMULATION_FLAG_EXIT_REASON       (1ULL << 1)
>  
>  /* for KVM_RUN, returned by mmap(vcpu_fd, offset=0) */
>  struct kvm_run {
> @@ -404,6 +405,12 @@ struct kvm_run {
>  			__u64 flags;
>  			__u8  insn_size;
>  			__u8  insn_bytes[15];
> +			/*
> +			 * The "exit reason" extracted from the
> +			 * VMCS/VMCB that was the cause of attempted
> +			 * emulation.
> +			 */
> +			__u64 exit_reason;

Rather than providing just the exit reason and adding another kvm_x86_ops hook,
I would prefer to extend kvm_x86_get_exit_info() to also provide the exit reason
and use that.  E.g. on VMX, all exceptions funnel through a single exit reason.
Dumping exit_info_{1,2} and error_code in addition to intr_info might not be all
that useful, but I can't see in harm either, and more info is generally a good
thing.

The only other user of kvm_x86_get_exit_info() is for tracepoints, those could
be modified to not pass in the exit reason.

>  		} emulation_failure;
>  		/* KVM_EXIT_OSI */
>  		struct {
> -- 
> 2.30.2
>