Re: SGX vs LSM (Re: [PATCH v20 00/28] Intel SGX1 support)

[Jarkko Sakkinen]

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