On Tue, Jul 09, 2019 at 04:11:08PM -0700, Xing, Cedric wrote:
> On 7/9/2019 3:25 PM, Sean Christopherson wrote:
> >On Tue, Jul 09, 2019 at 01:41:28PM -0700, Xing, Cedric wrote:
> >>On 7/9/2019 10:09 AM, Sean Christopherson wrote:
> >>>Translating those to SGX, with a lot of input from Stephen, I ended up
> >>>with the following:
> >>>
> >>> - FILE__ENCLAVE_EXECUTE: equivalent to FILE__EXECUTE, required to gain X
> >>> on an enclave page loaded from a regular file
> >>>
> >>> - PROCESS2__ENCLAVE_EXECDIRTY: hybrid of EXECMOD and EXECUTE+WRITE,
> >>> required to gain W->X on an enclave page
> >>
> >>EXECMOD basically indicates a file containing self-modifying code. Your
> >>ENCLAVE_EXECDIRTY is however a process permission, which is illogical.
> >
> >How is it illogical? If a PROCESS wants to EXECute a DIRTY ENCLAVE page,
> >then it needs PROCESS2__ENCLAVE_EXECDIRTY
> Just think of the purpose of FILE__EXECMOD. It indicates to LSM the file has
> self-modifying code, hence W->X transition should be considered "normal" and
> allowed, regardless which process that file is loaded into.
>
> The same thing for enclaves here. Whether an enclave contains self-modifying
> code is specific to that enclave, regardless which process it is loaded
> into.
>
> But what are you doing is quite the opposite, and that's I mean by
> "illogical".
Ah. My intent was to minimize the number of new labels, and because W->X
scenarios are not guaranteed to be backed by a file, I went with a per
process permission. Ditto for EXECANON. I'm not opposed to also having a
per file permission that can be used when possible.
Something like this? And maybe merge EXECANON and EXECDIRTY into a single
permission?
Depending on whether sigstruct is required to be in a pinned file, EAUG
pages would need either EXECDIRTY or EXECMOD.
static int selinux_enclave_load(struct vm_area_struct *vma, unsigned long prot,
bool measured)
{
const struct cred *cred = current_cred();
u32 sid = cred_sid(cred);
int ret;
/* Currently supported only in noexec kernels. */
WARN_ON_ONCE(!default_noexec);
/* Only executable enclave pages are restricted in any way. */
if (!(prot & PROT_EXEC))
return 0;
if (!measured) {
ret = enclave_has_perm(sid, PROCESS2__ENCLAVE_EXECUNMR);
if (ret)
goto out;
}
if (!vma->vm_file || IS_PRIVATE(file_inode(vma->vm_file))) {
ret = enclave_has_perm(sid, PROCESS2__ENCLAVE_EXECANON);
if (ret)
goto out;
/* Ability to do W->X within the enclave. */
if (prot & PROT_WRITE)
ret = enclave_has_perm(sid,
PROCESS2__ENCLAVE_EXECDIRTY);
} else {
ret = file_has_perm(cred, vma->vm_file, FILE__ENCLAVE_EXECUTE);
if (ret)
goto out;
/*
* Load code from a modified private mapping, or from any file
* mapping with the ability to do W->X within the enclave.
*/
if (vma->anon_vma || (prot & PROT_WRITE))
ret = file_has_perm(cred, vma->vm_file,
FILE__ENCLAVE_EXECMOD);
}
out:
return ret;
}
