On Thu, Sep 24, 2020 at 01:54:09PM -0700, Dave Hansen wrote: > On 9/24/20 1:25 PM, Sean Christopherson wrote: > ... > >> Why don't we just declare enclave memory as "out of scope for noexec" in > >> the same way that anonymous memory is, and just discard this patch? > >> That doesn't seem too much of a stretch. > > > > Because we lose line of sight to LSM support. Without enforcing "declare perms > > at load time" in the initial series, we would create an ABI where userspace > > could load an enclave page with only READ permissions and then map the enclave > > with whatever permissions it wants, without any convenient way for SGX to call > > into the LSM. > > This argument holds no water for me. LSMs are all about taking what > would otherwise be perfectly acceptable behavior and breaking them in > the name of security. They fundamentally break applications that used > to work just fine and also did totally sane things. It's not about LSMs breaking things, they can obviously do that without any help. The concern is that, unless we lay the groundwork now, adding support for LSMs in the future will break existing applications or create an unholy mess of an ABI. If we want to support LSM policy for enclave page permissions, checking LSM policies at load time and hooking mprotect() to enforce the policy at run time is by far the cleanest solution of the many ideas we discussed. The problem is that enforcing permissions via mprotect() needs to be done unconditionally, otherwise we end up with weird behavior where the existence of an LSM will change what is/isn't allowed, even if the LSM(s) has no SGX policy whatsover. And on the flip side, enforcing permissions unconditionally will break backwards compatibility. I'm a-ok if we want to kill off the ->mprotect() hook, so long as we acknowledge that in doing so we are likely throwing in the towel on supporting LSM policies for enclave page permissions.
