linux-kernel - Re: [PATCH 05/21] KVM: VMX: Teach EPT violation helper about private mem

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: <ZuLzl6reeDH_1fFh@google.com>
Date: Thu, 12 Sep 2024 06:58:47 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Kai Huang <kai.huang@...el.com>
Cc: Rick Edgecombe <rick.p.edgecombe@...el.com>, pbonzini@...hat.com, kvm@...r.kernel.org, 
	dmatlack@...gle.com, isaku.yamahata@...il.com, yan.y.zhao@...el.com, 
	nik.borisov@...e.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/21] KVM: VMX: Teach EPT violation helper about private mem

On Thu, Sep 12, 2024, Kai Huang wrote:
> > +static inline bool kvm_is_private_gpa(struct kvm *kvm, gpa_t gpa)
> > +{
> > +	/* For TDX the direct mask is the shared mask. */
> > +	return !kvm_is_addr_direct(kvm, gpa);
> > +}
> 
> Does this get used in any other places?  If no I think we can open code this
> in the __vmx_handle_ept_violation().
> 
> The reason is I think the name kvm_is_private_gpa() is too generic and this
> is in the header file.

+1, kvm_is_private_gpa() is much too generic.  I knew what the code was *supposed*
to do, but had to look at the implementation to verify that's actually what it did.

> E.g., one can come up with another kvm_is_private_gpa() checking the memory
> attributes to tell whether a GPA is private.
> 
> Or we rename it to something like
> 
> 	__vmx_is_faulting_gpa_private()
> ?
> 
> Which clearly says it is checking the *faulting* GPA.

I don't think that necessarily solves the problem either, because the reader has
to know that the KVM looks at the shared bit.

If open coding is undesirable, maybe a very literal name, e.g. vmx_is_shared_bit_set()?