Hi Jarkko
On Fri, 17 Feb 2023 16:32:12 -0600, jarkko@xxxxxxxxxx <jarkko@xxxxxxxxxx>
wrote:
On Wed, Feb 15, 2023 at 09:42:30AM -0600, Haitao Huang wrote:
On Wed, 15 Feb 2023 02:51:23 -0600, Huang, Kai <kai.huang@xxxxxxxxx>
wrote:
> > > > >
> > >
> > > Since sgx_mmap() can happen before enclave is created,
calculating the
> > > vm_pgoff
> > > from enclave_base is conceptually wrong. Even if you really want
> > to do
> > > it, it
> > > should be:
> > >
> > > if (enclave_has_initialized())
> > > vma->vm_pgoff = ...;
> >
> > I got your point now. I can add a condition to test the
SGX_ENCL_CREATED
> > bit. However, we still have a hole if we must handle the sequence
> > mmap(..., enclave_fd) being called before ECREATE ioctl. We can't
leave
> > vm_pgoff not set for those cases.
> >
> > Since no one does that so far, can we explicitly return an error
from
> > sgx_mmap when that happens?
> > Other suggestions?
>
> As I replied to patch 4/4, I believe userspace should pass the correct
> pgoff in
> mmap(). It's wrong to always pass 0 or any random value.
>
> If userspace follow the mmap() rule, you won't need to manually set
> vm_pgoff
> here (which is hacky IMHO). Everything works fine.
>
SGX driver was following MAP_ANONYMOUS semantics. If we change that,
it'd
break current usage/ABI.
I still think returning error for cases mmap(..., enclave_fd) if
enclave is
not created would be less intrusive change.
Is this something you care in SGX SDK?
It is not categorically forbidden so that's why I'm asking.
SDK does not care as we would never do this and don't think anyone is
doing that either. Suggesting returning error to cover all cases so user
space would not accidentally cause incorrect vm_pgoff set.
Thanks
Haitao
