On Thu, May 30, 2019 at 11:01:10AM -0700, Sean Christopherson wrote:

  - Requires enclave builder to mark enclave pages executable in the
    non-enclave VMAs, which may unnecessarily require EXECMOD on the
    source file, or even worse, EXECMEM, and potentially increases the
    attack surface since the file must be executable.

Enclave builder marks *non-enclave pages*? Not following.

W^X handling:
  - mmap() to /dev/sgx/enclave only allowed with PROT_NONE, i.e. force
    userspace through mprotect() to simplify the kernel implementation.
  - Add vm_ops mprotect() ops hook (I'll refer to SGX's implementation
    as SGX.mprotect())
  - Take explicit ALLOW_WRITE at ADD_REGION, a.k.a. EADD
  - ADD_REGION also used to describe EAUG region (tentatively for SGX2).
  - Track "can be written at some point in time (past or future)" as
    ALLOW_WRITE (to avoid confusiong with MAY_WRITE).  A priori knowledge
    of writability avoids having to track/coordinate PROT_WRITE across
    VMAs and MMs.

Still not sure why you want to use vm_ops instead of file_operations.

The approach I've been proposing earlier in this email thread before
these new proposals can be summarized from hook perspective as:

- Allow mmap() only before ECREATE and require it to be size
  of the ELRANGE (ECREATE ioctl would check this). This would
  be with PROT_NONE.
- Disallow mprotect() before EINIT. Requires a new callback
  to file_operations like mmap() has.
- After EINIT check for each mprotect() that it matches the
  permissions of underlying enclave pages. Disallow mmap()
  after EINIT.

/Jarkko