linux-kernel - Re: [PATCH v19 101/130] KVM: TDX: handle ept violation/misconfig exit

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]

Date: Mon, 6 May 2024 07:21:57 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: Reinette Chatre <reinette.chatre@...el.com>, Isaku Yamahata <isaku.yamahata@...el.com>, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, isaku.yamahata@...il.com, 
	Paolo Bonzini <pbonzini@...hat.com>, erdemaktas@...gle.com, Sagi Shahar <sagis@...gle.com>, 
	Kai Huang <kai.huang@...el.com>, chen.bo@...el.com, hang.yuan@...el.com, 
	tina.zhang@...el.com, isaku.yamahata@...ux.intel.com
Subject: Re: [PATCH v19 101/130] KVM: TDX: handle ept violation/misconfig exit

On Mon, May 06, 2024, Chao Gao wrote:
> On Wed, May 01, 2024 at 09:54:07AM -0700, Reinette Chatre wrote:
> >diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> >index 499c6cd9633f..ba81e6f68c97 100644
> >--- a/arch/x86/kvm/vmx/tdx.c
> >+++ b/arch/x86/kvm/vmx/tdx.c
> >@@ -1305,11 +1305,20 @@ static int tdx_handle_ept_violation(struct kvm_vcpu *vcpu)
> > 	} else {
> > 		exit_qual = tdexit_exit_qual(vcpu);
> > 		if (exit_qual & EPT_VIOLATION_ACC_INSTR) {
> >+			union tdx_exit_reason exit_reason = to_tdx(vcpu)->exit_reason;
> >+
> >+			/*
> >+			 * Instruction fetch in TD from shared memory
> >+			 * causes a #PF.
> >+			 */
> 
> It is better to hoist this comment above the if-statement.
> 
> 		/*
> 		 * Instruction fetch in TD from shared memory never causes EPT
> 		 * violation. Warn if such an EPT violation occurs as the CPU
> 		 * probably is buggy.
> 		 */
>  		if (exit_qual & EPT_VIOLATION_ACC_INSTR) {
> 		...
> 		}
> 
> 
> but, I am wondering why KVM needs to perform this check. I prefer to drop this
> check if KVM doesn't rely on it to handle EPT violation correctly.
> 
> > 			pr_warn("kvm: TDX instr fetch to shared GPA = 0x%lx @ RIP = 0x%lx\n",
> > 				tdexit_gpa(vcpu), kvm_rip_read(vcpu));
> 
> how about using vcpu_unimpl()? it is a wrapper for printing such a message.

This isn't an "unimplemented" scenario in KVM, this is a "hardware is broken"
scenario.  Just WARN_ON_ONCE() and move on.  Or I suppose since EPT misconfig
needs to do something as well:

		if (KVM_BUG_ON(exit_qual & EPT_VIOLATION_ACC_INSTR, vcpu->kvm))
			return -EIO;