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

[Dave Hansen]

On 9/22/20 5:58 AM, Jarkko Sakkinen wrote:
> Intel Sofware Guard eXtensions (SGX) allows creation of executable blobs
> called enclaves, of which page permissions are defined when the enclave

"of which" => "for which"

> is first loaded. Once an enclave is loaded and initialized, it can be
> mapped to the process address space.

Could you compare and contrast this a *bit* with existing executables?
What's special about SGX?  ELF executables have page permissions inside
the binary too.  Why don't we use this mechanism for them?

> Enclave permissions can be dynamically modified by using ENCLS[EMODPE]

I'm not sure this sentence matters.  I'm not sure why I care what the
instruction is named that does this.  But, it _sounds_ here like an
enclave can adjust its own permissions directly with ENCLS[EMODPE].

> instruction. We want to limit its use to not allow higher permissions than
> the ones defined when the enclave was first created.

Rather than higher and lower, please use stronger and weaker.

Also, please get rid of the "we".

> Add 'mprotect' hook to vm_ops, so that we can implement a callback for SGX
> that will check that {mmap, mprotect}() permissions do not surpass any of
> the page permissions in the address range defined.

"check" => "ensure"

> This is required in order to be able to make any access control decisions
> when enclave pages are loaded.
Now I'm confused.  I actually don't think I have a strong understanding
of how an enclave actually gets loaded, how mmap() and mprotect() are
normally used and what this hook is intended to thwart.