Re: [PATCH v38 10/24] mm: Add vm_ops->mprotect()

From: Dave Hansen
Date: Thu Sep 24 2020 - 16:54:30 EST


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.

> Retroactively enforcing permissions at load time would break the ABI, or at
> least yield different behavior based on the mere existence of LSMs, e.g. if
> LSMs are supported, suddenly the ADD_PAGES w/ READ -> mmap(RWX) flow breaks,
> even if there is no LSM policy denying that behavior.

I'm a security dummy. All I know is that when I see something like this:

if (security_vm_enough_memory_mm(mm, grow))
...

I know to ignore it because I like my systems to boot and I'm not using
those hooks. :)

How could the mere presence of an LSM change the behavior of one of
these hooks? Don't they have to actually hook into the specific place
and actively go trying to change the behavior at that site?