Re: [PATCH v26 10/22] x86/sgx: Linux Enclave Driver

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Thu, Feb 20, 2020 at 10:51:37AM -0800, Andy Lutomirski wrote:
> On Thu, Feb 20, 2020 at 10:13 AM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> > More than likely, the READ_IMPLIES_EXECUTE (RIE) crud rears its head
> > because part of the enclave loader is written in assembly.  Unless
> > explicitly told otherwise, the linker assumes that any program with
> > assembly code may need an executable stack, which leads to the RIE
> > personality being set for the process.  Here's a fantastic write up for
> > more details: https://www.airs.com/blog/archives/518
> >
> > There are essentially two paths we can take:
> >
> >  1) Exempt EPC pages from RIE during mmap()/mprotect(), i.e. don't add
> >     PROT_EXEC for enclaves.
> 
> Seems reasonable.
> 
> Honestly, it probably makes sense to try to exempt almost everything
> from RIE.  I'd be a bit surprised if RIE is actually useful for
> anything other than plain anonymous pages and private file mappings.

Hmm, last I looked at this I was focused on adding a generic protections
manipulator, e.g. vm_ops->mprotect_adjust() and f_op->???, and I thought
those options were too ugly to pursue.

But if we want to start killing RIE specifically, adding a boolean flag
to and f_op wouldn't be _that_ heinous, e.g.

static int do_mprotect_pkey(...)
{
	...

		/* Does the application expect PROT_READ to imply PROT_EXEC */
		if (rier && (vma->vm_flags & VM_MAYEXEC) &&
		    (!vma->vm_file || !vma->vm_file->f_op->no_read_implies_exec))
			prot |= PROT_EXEC;
}

unsigned long do_mmap(...)
{
	if ((prot & PROT_READ) && (current->personality & READ_IMPLIES_EXEC))
		if (!file || (!path_noexec(&file->f_path) &&
			      !file->f_op->no_read_implies_exec))
			prot |= PROT_EXEC;
}