Hi Andy,
On 12/3/2021 4:42 PM, Andy Lutomirski wrote:
On 12/3/21 14:34, Reinette Chatre wrote:
On 12/3/2021 11:38 AM, Andy Lutomirski wrote:
On 12/1/21 11:23, Reinette Chatre wrote:
In the initial (SGX1) version of SGX, pages in an enclave need to be
created with permissions that support all usages of the pages, from the
time the enclave is initialized until it is unloaded. For example,
pages used by a JIT compiler or when code needs to otherwise be
relocated need to always have RWX permissions.
SGX2 includes two functions that can be used to modify the enclave page
permissions of regular enclave pages within an initialized enclave.
ENCLS[EMODPR] is run from the OS and used to restrict enclave page
permissions while ENCLU[EMODPE] is run from within the enclave to
extend enclave page permissions.
Enclave page permission changes need to be approached with care and
for this reason this initial support is to allow enclave page
permission changes _only_ if the new permissions are the same or
more restrictive that the permissions originally vetted at the time the
pages were added to the enclave. Support for extending enclave page
permissions beyond what was originally vetted is deferred.
I may well be missing something, but off the top of my head,
literally the only reason that EMODPR needs CPL0 (i.e. ENCLS) is that
it requires a TLB flush IPI to take effect. (Score one for AMD for
being having superior hardware in this regard.)
My understanding also is that it is the need for TLB flush that
require the privilege but I am trying to get more information here.
Given that, I don't see any reason for the EMODPR operation to be
treated as security sensitive -- it just needs to be implemented
correctly. I don't even see why the host should (or even can!) do
any useful tracking of the EPCM state.
The OS needs to know the EPCM permissions to be able to install the
appropriate PTEs. If the enclave chooses to change the enclave page
permissions from within the enclave then user space needs to let the
OS know via the SGX_IOC_PAGE_MODP ioctl to ensure that the OS can
install correct PTEs in support of the permission change.
(But I am confused about one thing: to the extent an enclave actually
needs EMODPR, is there anything in the hardware or anything that the
enclave can do short of actually poking the page from all threads and
confirming that a fault occurs to make sure the OS actually flushed
the TLB? ISTM a malicious host could attack an enclave by omitting
the TLB flush and then exploiting an enclave but that would have been
mitigated if the flush occurred.)
When enclave page permissions are restricted it requires the enclave
to accept the new permissions from within the enclave by running
ENCLU[EACCEPT]. This instruction requires that (it will fail
otherwise) the OS completed an ENCLS[ETRACK] on the affected page -
essentially ENCLU[EACCEPT] can only succeed if no cached
linear-to-physical address mappings are present. The ETRACK flow is
elaborate and I attempted to document it in patch 06/25. Essentially,
SGX hardware flushes all cached linear-to-physical mappings when an
enclave is exited and with ETRACK it can be ensured that all threads
that were in an enclave at the time the tracking started (in this case
after ENCLS[EMODPR]), have exited.
Does the enclave do something before asking for the ioctl to put the
page in a state where the tracking is armed? I read the SDM, but I
probably read the wrong part of the SDM, and I may have missed this.
No, when restricting permissions the enclave does not do anything
special to the page before calling the ioctl.
The (non enclave) userspace calls the SGX_IOC_PAGE_MODP ioctl that will
call ENCLS[EMODPR] to restrict the permissions as well as the
ENCLS[ETRACK] to start the tracking before sending all CPUs that may be
accessing the enclave an IPI. The enclave then runs ENCLU[EACCEPT] to
accept the permission changes and this would fail if the host attempted
to omit the TLB flush.
You can see an example of EPCM permission changes from user space
perspective in the form of a selftest found in the patch that follows
this one.
Reinette