On Thu, May 23, 2019 at 07:17:52AM -0700, Sean Christopherson wrote: > On Thu, May 23, 2019 at 01:26:28PM +0300, Jarkko Sakkinen wrote: > > On Wed, May 22, 2019 at 07:35:17PM -0700, Sean Christopherson wrote: > > > But actually, there's no need to disallow mmap() after ECREATE since the > > > LSM checks also apply to mmap(), e.g. FILE__EXECUTE would be needed to > > > mmap() any enclave pages PROT_EXEC. I guess my past self thought mmap() > > > bypassed LSM checks? The real problem is that mmap()'ng an existing > > > enclave would require FILE__WRITE and FILE__EXECUTE, which puts us back > > > at square one. > > > > I'm lost with the constraints we want to set. > > As is today, SELinux policies would require enclave loaders to have > FILE__WRITE and FILE__EXECUTE permissions on /dev/sgx/enclave. Presumably > other LSMs have similar requirements. Requiring all processes to have > FILE__{WRITE,EXECUTE} permissions means the permissions don't add much > value, e.g. they can't be used to distinguish between an enclave that is > being loaded from an unmodified file and an enclave that is being > generated on the fly, e.g. Graphene. > > Looking back at Andy's mail, he was talking about requiring FILE__EXECUTE > to run an enclave, so perhaps it's only FILE__WRITE that we're trying to > special case. Argh, as I was working through Andy's latest proposal I realized that I was subconciously making FILE__READ imply FILE__EXECUTE. The idea behind inheriting permissions from the source VMA is to exempt "standard" enclaves from needing FILE__WRITE. But if we don't add an exemption for FILE__EXECUTE as well, then all enclaves need FILE__EXECUTE, which means FILE__EXECUTE can't be used to identify the case where userspace is mapping an inherited PROT_WRITE page as PROT_EXEC. And if the SGX magic exempts FILE__EXECUTE, then FILE__READ implies FILE__EXECUTE. Yuck.