On Wed, 2023-02-15 at 09:42 -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. Is there any doc/reference mentioning this? > > I still think returning error for cases mmap(..., enclave_fd) if enclave > is not created would be less intrusive change. But you need the first mmap() to work before IOCTL(ecreate) to get enclave's base address, correct? > > Haitao
