linux-kernel - Re: [PATCH 1/3] KVM: x86, vmx: Add function for event delivery error generation

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: <ZxBIRBB0Ibx9J5TN@google.com>
Date: Wed, 16 Oct 2024 16:12:04 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Ivan Orlov <ivan.orlov0322@...il.com>
Cc: Ivan Orlov <iorlov@...zon.com>, bp@...en8.de, dave.hansen@...ux.intel.com, 
	mingo@...hat.com, pbonzini@...hat.com, shuah@...nel.org, tglx@...utronix.de, 
	hpa@...or.com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, x86@...nel.org, jalliste@...zon.com, 
	nh-open-source@...zon.com, pdurrant@...zon.co.uk
Subject: Re: [PATCH 1/3] KVM: x86, vmx: Add function for event delivery error generation

On Wed, Oct 16, 2024, Ivan Orlov wrote:
> On 10/16/24 22:05, Sean Christopherson wrote:
> > On Tue, Oct 15, 2024, Ivan Orlov wrote:
> > > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > > > > index c67e448c6ebd..afd785e7f3a3 100644
> > > > > --- a/arch/x86/kvm/vmx/vmx.c
> > > > > +++ b/arch/x86/kvm/vmx/vmx.c
> > > > > @@ -6550,19 +6550,10 @@ static int __vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
> > > > >   	     exit_reason.basic != EXIT_REASON_APIC_ACCESS &&
> > > > >   	     exit_reason.basic != EXIT_REASON_TASK_SWITCH &&
> > > > >   	     exit_reason.basic != EXIT_REASON_NOTIFY)) {
> > > > > -		int ndata = 3;
> > > > > +		gpa_t gpa = vmcs_read64(GUEST_PHYSICAL_ADDRESS);
> > > > > +		bool is_mmio = exit_reason.basic == EXIT_REASON_EPT_MISCONFIG;
> > > > 
> > > > There's no need for is_mmio, just pass INVALID_GPA when the GPA isn't known.
> > > 
> > > Ah alright, then we definitely don't need an is_mmio field. I assume we
> > > can't do MMIO at GPA=0, right?
> > 
> > Wrong :-)
> > 
> 
> Then getting rid of `is_mmio` will make distinguishing between vectoring
> error due to MMIO with GPA=0 and non-mmio vectoring error quite hard for the
> error reporti
> 
> Passing INVALID_GPA into the userspace due to non-mmio vectoring error will
> change the existing internal.data order, but I can do it if it's fine. Sorry
> for nitpicking :)

KVM's existing ABI is rather awful, though arguably the intent was that there is
no ABI, i.e. that KVM is dumping info to try to be helpful.  E.g. the existing
behavior is that data[3] contains a GPA only for EPT_MISCONFIG, but for everything
else, data[3] contains last_vmentry_cpu.

And because it's so awful, I doubt any userspace actually has code that acts on
the layout of data[].  So, I suspect we can do the simple and sane thing, and
fill data[3] with -1ull if the GPA is invalid, and then document that that's the
behavior (if we're feeling generous).