On Wed, 2021-02-03 at 09:07 -0800, Sean Christopherson wrote:
> On Wed, Feb 03, 2021, Kai Huang wrote:
> > On Tue, 2 Feb 2021 17:36:12 -0800 Sean Christopherson wrote:
> > > On Wed, Feb 03, 2021, Edgecombe, Rick P wrote:
> > > > On Tue, 2021-01-26 at 22:31 +1300, Kai Huang wrote:
> > > > > + /*
> > > > > + * Verify alignment early. This conveniently avoids having
> > > > > to worry
> > > > > + * about page splits on userspace addresses.
> > > > > + */
> > > > > + if (!IS_ALIGNED(pageinfo.metadata, 64) ||
> > > > > + !IS_ALIGNED(pageinfo.contents, 4096)) {
> > > > > + kvm_inject_gp(vcpu, 0);
> > > > > + return 1;
> > > > > + }
> > > > > +
> > > > > + /*
> > > > > + * Translate the SECINFO, SOURCE and SECS pointers from GVA
> > > > > to GPA.
> > > > > + * Resume the guest on failure to inject a #PF.
> > > > > + */
> > > > > + if (sgx_gva_to_gpa(vcpu, pageinfo.metadata, false,
> > > > > &metadata_gpa) ||
> > > > > + sgx_gva_to_gpa(vcpu, pageinfo.contents, false,
> > > > > &contents_gpa) ||
> > > > > + sgx_gva_to_gpa(vcpu, secs_gva, true, &secs_gpa))
> > > > > + return 1;
> > > > > +
> > > >
> > > > Do pageinfo.metadata and pageinfo.contents need cannonical checks here?
> > >
> > > Bugger, yes. So much boilerplate needed in this code :-/
> > >
> > > Maybe add yet another helper to do alignment+canonical checks, up where the
> > > IS_ALIGNED() calls are?
> >
> > sgx_get_encls_gva() already does canonical check. Couldn't we just use it?
>
> After rereading the SDM for the bajillionth time, yes, these should indeed use
> sgx_get_encls_gva(). Originally I was thinking they were linear addresses, but
> they are effective addresses that use DS, i.e. not using the helper to avoid the
> DS.base adjustment for 32-bit mode was also wrong.
Agreed.
>
> > For instance:
> >
> > if (sgx_get_encls_gva(vcpu, pageinfo.metadata, 64, 64 &metadata_gva) ||
> > sgx_get_encls_gva(vcpu, pageinfo.contents, 4096, 4096,
> > &contents_gva))
> > return 1;
