On 2019-10-17 19:57, Sean Christopherson wrote: > +Cc Andy > > On Mon, Oct 14, 2019 at 08:43:09AM +0000, Jethro Beekman wrote: >> On 2019-10-11 20:15, Sean Christopherson wrote: >>> On Fri, Oct 11, 2019 at 04:37:25PM +0000, Jethro Beekman wrote: >>>> UAPI: >>>> >>>> This got a whole lot more complex for userspace compared to the out-of-tree >>>> driver. >>>> >>>> 1. Manually needing to mmap a naturally-aligned memory region by allocating >>>> too much memory and then unmapping parts is quite annoying. Why was the >>>> auto-aligning removed? I think this will need to be handled the same for >>>> every consumer of SGX, so I don't see why this is not handled in the kernel. >>>> It never seems wrong to align if NULL is passed as the requested address. >>>> Alternatively, is there room in the flags for a MAP_ALIGNED bit? >>> >>> I'm pretty sure everyone agrees it's annoying. The short of it is that >>> the SGX driver is the wrong place to do the alignment. The driver could >>> key off addr=0, but we don't want to take on that implicit behavior. >> >> Why not? > > Because it's a hack. If a MAP_ALIGNED flag is added then SGX is stuck > with kludgy code that serves no purpose. And userspace needs to manually > align the result if it provides an actual hint. Regardless of whether > there are use cases for providing a hint for ELRANGE, having divergent > logic is ugly. > >>> A MAP_ALIGNED flag to have the allocation be naturally aligned is the >>> ideal solution. It's definitely something we should pursue, but that can >>> and probably should be done in parallel to the SGX series. >>> >>>> 2. Having to re-open the device for every enclave is also annoying. This >>>> means you need a filesystem available throughout the process lifetime. I >>>> tried dup, but that doesn't work. Can we make dup work? >>> >>> The semantics of dup() won't get you what want, as dup() just creates a >>> new descriptor pointing at the same file. >>> >>> An alternative solution that was proposed was to have an ioctl() for >>> creating an enclave. But that means using an anonymous inode, which runs >>> afoul of SELinux permissions, e.g. every _process_ that runs enclaves >>> would require EXECMEM. Linus was quite clear that SGX wouldn't be merged >>> if using it required users to degrade existing security. >> >> It's ok if it's the same inode, it just needs to be a different struct file. >> >>> I'm open to other ideas. I wasn't aware this was a pain point and file >>> stuff isn't exactly my area of expertise, so I haven't put much/any >>> thought into alternatives. >> >> The default permissions for /dev/sgx/enclave are root-only. This means you >> want to be able to do the same thing as network servers: initialize some >> resources as root, then drop privileges. This used to mean opening /dev/sgx >> and keeping the fd around which meant you could launch enclaves at will. With >> the new API, this is no longer possible, you can only launch one enclave per >> fd. Is there a different type of operation that doesn't just duplicate the fd >> but also the struct file? If not, can we add an ioctl for that? > > My approach to this would be to chown /dev/sgx/enclave so that it's owned > by root but accessible to users belonging to an sgx-specific group, e.g. > via a udev rule. > >> There are other scenarios where it's not just the permissions on >> /dev/sgx/enclave that are the problem but using the filesystem in general >> that is. Maybe you've used seccomp to disable file operations, etc. > > Andy and Jarkko, thoughts? Folks, any more thoughts on how to resolve the issue that you need to call open() for every enclave? -- Jethro Beekman | Fortanix
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
