Re: [PATCH 05/25] x86/sgx: Introduce runtime protection bits
From: Reinette Chatre
Date: Tue Jan 18 2022 - 16:18:29 EST
Hi Jarkko,
On 1/15/2022 8:49 AM, Jarkko Sakkinen wrote:
> On Sat, Jan 15, 2022 at 01:15:53AM +0200, Jarkko Sakkinen wrote:
>>> After running ENCLU[EMODPE] user space uses SGX_IOC_ENCLAVE_MOD_PROTECTIONS
>>
>> OK, great.
>>
>> A minor nit: please call it SGX_IOC_ENCLAVE_MODIFY_PROTECTIONS.
>
> I'm not confident after looking through the test case and ioctl
> about EMODPE support but I do not want disturb this anymore. Bunch
> of things have been nailed and I'm now running the code, which is
> great.
>
> The obviously wrong implementation choice in this ioctl is that
> it is multi-function. It should be just split it into two ioctls:
> sgx_restrict_page_permissions and sgx_extend_page_permissions.
Sure, I can move it to two ioctls.
To keep the naming consistent, what do you think of:
SGX_IOC_ENCLAVE_RESTRICT_PERMISSIONS
SGX_IOC_ENCLAVE_RELAX_PERMISSIONS
Please refer to message below as motivation for the "relax" term:
https://lore.kernel.org/lkml/24447a03-139a-c7e0-9ad5-34e2019c4df5@xxxxxxxxx/
>
> They are conceptually different flows and I'm also basing this on earlier
> discussion in this mailing list from which I conclude that it is also
> consensus to not have such ioctls.
>
> Might sound clanky but it is much easier to comprehend what is going
> on "in the blackbox" by doing that split.
Reinette