> On May 17, 2019, at 10:29 AM, Sean Christopherson <sean.j.christopherson@xxxxxxxxx> wrote: > >> On Fri, May 17, 2019 at 12:37:40PM -0400, Stephen Smalley wrote: >>> On 5/17/19 12:20 PM, Stephen Smalley wrote: >>>> On 5/17/19 11:09 AM, Sean Christopherson wrote: >>>> I think we may want to change the SGX API to alloc an anon inode for each >>>> enclave instead of hanging every enclave off of the /dev/sgx/enclave >>>> inode. >>>> Because /dev/sgx/enclave is NOT private, SELinux's file_map_prot_check() >>>> will only require FILE__WRITE and FILE__EXECUTE to mprotect() enclave >>>> VMAs >>>> to RWX. Backing each enclave with an anon inode will make SELinux treat >>>> EPC memory like anonymous mappings, which is what we want (I think), e.g. >>>> making *any* EPC page executable will require PROCESS__EXECMEM (SGX is >>>> 64-bit only at this point, so SELinux will always have default_noexec). >>> >>> I don't think we want to require EXECMEM (or equivalently both FILE__WRITE >>> and FILE__EXECUTE to /dev/sgx/enclave) for making any EPC page executable, >>> only if the page is also writable or previously modified. The intent is >>> to prevent arbitrary code execution without EXECMEM (or >>> FILE__WRITE|FILE__EXECUTE), while still allowing enclaves to be created >>> without EXECMEM as long as the EPC page mapping is only ever mapped RX and >>> its initial contents came from an unmodified file mapping that was >>> PROT_EXEC (and hence already checked via FILE__EXECUTE). > > The idea is that by providing an SGX ioctl() to propagate VMA permissions > from a source VMA, EXECMEM wouldn't be required to make an EPC page > executable. E.g. userspace establishes an enclave in non-EPC memory from > an unmodified file (with FILE__EXECUTE perms), and the uses the SGX ioctl() > to copy the contents and permissions into EPC memory. > >> Also, just to be clear, there is nothing inherently better about checking >> EXECMEM instead of checking both FILE__WRITE and FILE__EXECUTE to the >> /dev/sgx/enclave inode, so I wouldn't switch to using anon inodes for that >> reason. Using anon inodes also unfortunately disables SELinux inode-based >> checking since we no longer have any useful inode information, so you'd lose >> out on SELinux ioctl whitelisting on those enclave inodes if that matters. > > The problem is that all enclaves are associated with a single inode, i.e. > /dev/sgx/enclave. /dev/sgx/enclave is a char device whose purpose is to > provide ioctls() and to allow mmap()'ing EPC memory. In no way is it > associated with the content that actually gets loaded into EPC memory. > > The actual file that contains the enclave's contents (assuming the enclave > came from a file) is a separate regular file that the SGX subsystem never > sees. > > AIUI, having FILE__WRITE and FILE__EXECUTE on /dev/sgx/enclave would allow > *any* enclave/process to map EPC as RWX. Moving to anon inodes and thus > PROCESS__EXECMEM achieves per-process granularity. How does anon_inode make any difference? Anon_inode is not the same thing as anon_vma.
