On Fri, Sep 18, 2020 at 08:09:04AM -0700, Andy Lutomirski wrote: > On Tue, Sep 15, 2020 at 4:28 AM Jarkko Sakkinen > <jarkko.sakkinen@xxxxxxxxxxxxxxx> wrote: > > > > From: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> > > > > Add vm_ops()->mprotect() for additional constraints for a VMA. > > > > Intel Software Guard eXtensions (SGX) will use this callback to add two > > constraints: > > > > 1. Verify that the address range does not have holes: each page address > > must be filled with an enclave page. > > 2. Verify that VMA permissions won't surpass the permissions of any enclave > > page within the address range. Enclave cryptographically sealed > > permissions for each page address that set the upper limit for possible > > VMA permissions. Not respecting this can cause #GP's to be emitted. Side note, #GP is wrong. EPCM violations are #PFs. Skylake CPUs #GP, but that's technically an errata. But this isn't the real motivation, e.g. userspace can already trigger #GP/#PF by reading/writing a bad address, SGX simply adds another flavor. > It's been awhile since I looked at this. Can you remind us: is this > just preventing userspace from shooting itself in the foot or is this > something more important? Something more important, it's used to prevent userspace from circumventing a noexec filesystem by loading code into an enclave, and to give the kernel the option of adding enclave specific LSM policies in the future. The source file (if one exists) for the enclave is long gone when the enclave is actually mmap()'d and mprotect()'d. To enforce noexec, the requested permissions for a given page are snapshotted when the page is added to the enclave, i.e. when the enclave is built. Enclave pages that will be executable must originate from an a MAYEXEC VMA, e.g. the source page can't come from a noexec file system. The ->mprotect() hook allows SGX to reject mprotect() if userspace is declaring permissions beyond what are allowed, e.g. trying to map an enclave page with EXEC permissions when the page was added to the enclave without EXEC. Future LSM policies have a similar need due to vm_file always pointing at /dev/sgx/enclave, e.g. policies couldn't be attached to a specific enclave. ->mprotect() again allows enforcing permissions at map time that were checked at enclave build time, e.g. via an LSM hook. Deferring ->mprotect() until LSM support is added (if it ever is) would be problematic due to SGX2. With SGX2, userspace can extend permissions of an enclave page (for the CPU's EPC Map entry, not the kernel's page tables) without bouncing through the kernel. Without ->mprotect () enforcement. userspace could do EADD(RW) -> mprotect(RWX) -> EMODPE(X) to gain W+X. We want to disallow such a flow now, i.e. force userspace to do EADD(RW,X), so that the hypothetical LSM hook would have all information at EADD(), i.e. would be aware of the EXEC permission, without creating divergent behavior based on whether or not an LSM is active.
