lkml.org 
[lkml]   [2022]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86/sgx: Enable PROT_EXEC for EAUG'd pages
On Mon, Mar 07, 2022 at 05:35:04PM +0200, Jarkko Sakkinen wrote:
> vm_max_permissions was created to control the pre-initialization content
> that contributes to MRSIGNATURE. It was never meant to be as a limit to
> dynamically added pages.
>
> E.g. static content could be used as a hook for LSM's to decide whether
> certain signature is qualified for EINIT. Dynamic content has nothing to
> do with that. The current mechanisms only add to the complexity on how
> to control PTE and EPCM permissions, and do not add anything else than
> obfuscity to security side of things.
>
> Thus add PROT_EXEC to the permissions assigned by the #PF handler.
>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> arch/x86/kernel/cpu/sgx/encl.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
> index 79e39bd99c09..0256918b2c2f 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -160,12 +160,11 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma,
> encl_page->encl = encl;
>
> /*
> - * Adding a regular page that is architecturally allowed to only
> - * be created with RW permissions.
> - * TBD: Interface with user space policy to support max permissions
> - * of RWX.
> + * Dynamic pages do not contribute to MRSIGNATURE, i.e. they are
> + * controlled only by PTE and EPCM permissions. Thus, the no limit
> + * is set here.
> */
> - prot = PROT_READ | PROT_WRITE;
> + prot = PROT_READ | PROT_WRITE | PROT_EXEC;
> encl_page->vm_run_prot_bits = calc_vm_prot_bits(prot, 0);
> encl_page->vm_max_prot_bits = encl_page->vm_run_prot_bits;
>
> --
> 2.35.1
>

This is really a show stopper. I think here's a logical mistake on for what
purpose vm_max_prot_bits are used for. They are meant for the static and
also signed content of the enclave.

These changes in the patch set that are related to vm_max_prot_bits only
messes up what already exists, and make incredibly hard to implement
anything decent on top of SGX2 features.

BR, Jarkko

\
 
 \ /
  Last update: 2022-03-07 17:05    [W:0.046 / U:0.392 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site