On Fri Jun 30, 2023 at 2:29 AM EEST, Huang, Kai wrote: > On Thu, 2023-06-29 at 07:23 -0700, Sean Christopherson wrote: > > On Thu, Jun 29, 2023, Kai Huang wrote: > > > On Wed, 2023-06-28 at 07:57 -0700, Sean Christopherson wrote: > > > > On Wed, Jun 28, 2023, Kai Huang wrote: > > > > > (but requires MAP_FIXED). > > > > > > > > No, SGX doesn't require MAP_FIXED. The requirements of ELRANGE make it extremely > > > > unlikely that mmap() will succeed, but it's not a strict requirement. > > > > > > Looks w/o MAP_FIXED, the driver just uses the generic mm->get_unmapped_area() to > > > return the address, which doesn't guarantee the right address will be returned > > > at all. Especially when ELRANGE is reserved via mmap(NULL), the > > > mmap(/dev/sgx_enclave) will not return the correct address no matter what pgoff > > > is used IIUC. > > > > > > static unsigned long sgx_get_unmapped_area(struct file *file, > > > unsigned long addr, > > > unsigned long len, > > > unsigned long pgoff, > > > unsigned long flags) > > > { > > > if ((flags & MAP_TYPE) == MAP_PRIVATE) > > > return -EINVAL; > > > > > > if (flags & MAP_FIXED) > > > return addr; > > > > > > return current->mm->get_unmapped_area(file, addr, len, pgoff, flags); > > > } > > > > > > So to me userspace has to use MAP_FIXED to get the correct address. > > > > No. As called out below, @addr is still used as a fairly strong hint: > > > > unsigned long > > arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0, > > const unsigned long len, const unsigned long pgoff, > > const unsigned long flags) > > { > > struct vm_area_struct *vma; > > struct mm_struct *mm = current->mm; > > unsigned long addr = addr0; > > struct vm_unmapped_area_info info; > > > > /* requested length too big for entire address space */ > > if (len > TASK_SIZE) > > return -ENOMEM; > > > > /* No address checking. See comment at mmap_address_hint_valid() */ > > if (flags & MAP_FIXED) > > return addr; > > > > /* for MAP_32BIT mappings we force the legacy mmap base */ > > if (!in_32bit_syscall() && (flags & MAP_32BIT)) > > goto bottomup; > > > > /* requesting a specific address */ <================================== > > if (addr) { > > addr &= PAGE_MASK; > > if (!mmap_address_hint_valid(addr, len)) > > goto get_unmapped_area; > > > > vma = find_vma(mm, addr); > > if (!vma || addr + len <= vm_start_gap(vma)) > > return addr; > > } > > > > ... > > } > > > > > > In practice, I expect most/all userspace implementations do use MAP_FIXED, but > > it's not strictly required. > > > > Yeah I agree it's a strong hint, but from ABI's perspective, I think even a > strong hint isn't good enough. If there's no guarantee userspace can 100% get > the correct enclave address, then userspace will always need to verify the > return address and if not do mmap() again. > > Btw, my reading of above function is if @addr hint doesn't work if the ELRANGE > is reserved using mmap(NULL), because below code will always NOT return addr: > > vma = find_vma(mm, addr); <--- A valid VMA will be found > if (!vma || addr + len <= vm_start_gap(vma)) <-- This check > will be false > return addr; > > This is kinda reasonable because ELRANGE via mmap(NULL) doesn't have a file > backed, so the mmap(/dev/sgx_enclave) should never return an overlapping address > even we pass a addr within ELRANGE. > > But my true argument is from ABI's perspective, although @addr is a hint, but > it's not guaranteed the *exact* addr will be returned (see man page below): > > " > If addr is not NULL, then the kernel takes it as a hint about where to place the > mapping; ...... If another mapping already exists there, the kernel picks a new > address that may or may not depend on the hint. > " > > But SGX usrespace needs a *exact* address. MAP_FIXED is the only ABI can > guarantee this. A practical point of view: I don't see much any other point with Intel SDK than provide environment to compile and run attestation shenanigans. Is there something people *actually* use it in the cloud? I'm starting to miss the perspective on why waste so much energy on this? Feels fuzzy. BR, Jarkko