Re: [PATCH V2 16/32] x86/sgx: Support restricting of enclave page permissions

Reinette Chatre
2022-03-17T22:08:23Z

Hi Jarkko,

On 3/16/2022 9:30 PM, Jarkko Sakkinen wrote:
> On Mon, Mar 14, 2022 at 08:32:28AM -0700, Reinette Chatre wrote:
>> Hi Jarkko,
>>
>> On 3/13/2022 8:42 PM, Jarkko Sakkinen wrote:
>>> On Fri, Mar 11, 2022 at 11:28:27AM -0800, Reinette Chatre wrote:
>>>> Supporting permission restriction in an ioctl() enables the runtime to manage
>>>> the enclave memory without needing to map it.
>>>
>>> Which is opposite what you do in EAUG. You can also augment pages without
>>> needing the map them. Sure you get that capability, but it is quite useless
>>> in practice.
>>>
>>>> I have considered the idea of supporting the permission restriction with
>>>> mprotect() but as you can see in this response I did not find it to be
>>>> practical.
>>>
>>> Where is it practical? What is your application? How is it practical to
>>> delegate the concurrency management of a split mprotect() to user space?
>>> How do we get rid off a useless up-call to the host?
>>>
>>
>> The email you responded to contained many obstacles against using mprotect()
>> but you chose to ignore them and snipped them all from your response. Could
>> you please address the issues instead of dismissing them?
>
> I did read the whole email but did not see anything that would make a case
> for fully exposed EMODPR, or having asymmetrical towards how EAUG works.

I believe that on its own each obstacle I shared with you is significant enough
to not follow that approach. You simply respond that I am just not making a
case without acknowledging any obstacle or providing a reason why the obstacles
are not valid.

To help me understand your view, could you please respond to each of the
obstacles I list below and how it is not an issue?

1) ABI change:
mprotect() is currently supported to modify VMA permissions
irrespective of EPCM permissions. Supporting EPCM permission
changes with mprotect() would change this behavior.
For example, currently it is possible to have RW enclave
memory and support multiple tasks accessing the memory. Two
tasks can map the memory RW and later one can run mprotect()
to reduce the VMA permissions to read-only without impacting
the access of the other task.
By moving EPCM permission changes to mprotect() this usage
will no longer be supported and current behavior will change.

2) Only half EPCM permission management:
Moving to mprotect() as a way to set EPCM permissions is
not a clear interface for EPCM permission management because
the kernel can only restrict permissions. Even so, the kernel
has no insight into the current EPCM permissions and thus whether they
actually need to be restricted so every mprotect() call,
all except RWX, will need to be treated as a permission
restriction with all the implementation obstacles
that accompany it (more below).

There are two possible ways to implement permission restriction
as triggered by mprotect(), (a) during the mprotect() call or
(b) during a subsequent #PF (as suggested by you), each has
its own obstacles.

3) mprotect() implementation

When the user calls mprotect() the expectation is that the
call will either succeed or fail. If the call fails the user
expects the system to be unchanged. This is not possible if
permission restriction is done as part of mprotect().

(a) mprotect() may span multiple VMAs and involves VMA splits
that (from what I understand) cannot be undone. SGX memory
does not support VMA merges. If any SGX function
(EMODPR or ETRACK on any page) done after a VMA split fails
then the user will be left with fragmented memory.

(b) The EMODPR/ETRACK pair can fail on any of the pages provided
by the mprotect() call. If there is a failure then the
kernel cannot undo previously executed EMODPR since the kernel
cannot run EMODPE. The EPCM permissions are thus left in inconsistent
state since some of the pages would have changed EPCM permissions
and mprotect() does not have mechanism to communicate
partial success.
The partial success is needed to communicate to user space
(i) which pages need EACCEPT, (ii) which pages need to be
in new request (although user space does not have information
to help the new request succeed - see below).

(c) User space runtime has control over management of EPC memory
and accurate failure information would help it to do so.
Knowing the error code of the EMODPR failure would help
user space to take appropriate action. For example, EMODPR
can return "SGX_PAGE_NOT_MODIFIABLE" that helps the runtime
to learn that it needs to run EACCEPT on that page before
the EMODPR can succeed. Alternatively, if it learns that the
return is "SGX_EPC_PAGE_CONFLICT" then it could determine
that some other part of the runtime attempted an ENCLU
function on that page.
It is not possible to provide such detailed errors to user
space with mprotect().

4) #PF implementation

(a) There is more to restricting permissions than just running
ENCLS[EMODPR]. After running ENCLS[EMODPR] the kernel should
also initiate the ETRACK flow to ensure that any thread within
the enclave is interrupted by sending an IPI to the CPU,
this includes the thread that just triggered the #PF.

(b) Second consideration of the EMODPR and ETRACK flow is that
this has a large "blast radius" in that any thread in the
enclave needs to be interrupted. #PFs may arrive at any time
so setting up a page range where a fault into any page in the
page range will trigger enclave exits for all threads is
a significant yet random impact. I believe it would be better
to update all pages in the range at the same time and in this
way contain the impact of this significant EMODPR/ETRACK/IPIs
flow.

(c) How will the page fault handler know when EMODPR/ETRACK should
be run? Consider that the page fault handler can be called
significantly later than the mprotect() call and that
user space can call EMODPE any time to extend permissions.
This implies that EMODPR/ETRACK/IPIs should be run during
*every* page fault, irrespective of mprotect().

(d) If a page is in pending or modified state then EMODPR will
always fail. This is something that needs to be fixed by
user space runtime but the page fault will not be able
to communicate this.

Considering the above, could you please provide clear guidance on
how you envision permission restriction to be supported by mprotect()?

Reinette