On Fri, 5 Mar 2021 09:51:56 -0800 Dave Hansen wrote:
> On 2/26/21 4:15 AM, Kai Huang wrote:
> > +int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
> > + int *trapnr)
> > +{
> > + int ret;
> > +
> > + /*
> > + * @secs is userspace address, and it's not guaranteed @secs points at
> > + * an actual EPC page.
>
> There are four cases that I can think of:
> 1. @secs points to an EPC page. Good, return 0 and go on with life.
> 2. @secs points to a non-EPC page. It will fault and permanently error
> out
> 3. @secs points to a Present=0 PTE. It will fault, but we need to call
> the fault handler to get a page in here.
> 4. @secs points to a kernel address
>
> #1 and #2 are handled and described.
>
> #4 is probably impossible because the address comes out of some
> gpa_to_hva() KVM code. But, it still _looks_ wonky here. I wouldn't
> hate an access_ok() check on it.
>
> /*
> * @secs is an untrusted, userspace-provided address. It comes
> * from KVM and is assumed to point somewhere in userspace.
> * This can fault and call SGX or other fault handlers.
> */
>
> You can also spend a moment to describe the kinds of faults that are
> handled and what is fatal.
Thanks Dave for the comments. I'll refine accordingly.
>
>
> > + * to physical EPC page by resolving PFN but using __uaccess_xx() is
> > + * simpler.
> > + */
>
> I'd leave the justification for the changelog.
Will do.
>
> > + __uaccess_begin();
> > + ret = __ecreate(pageinfo, (void *)secs);
> > + __uaccess_end();
> > +
> > + if (encls_faulted(ret)) {
> > + *trapnr = ENCLS_TRAPNR(ret);
> > + return -EFAULT;
> > + }
>
