Re: [RFC PATCH v2 3/5] x86/sgx: Enforce noexec filesystem restriction for enclaves

[Stephen Smalley]

On 6/10/19 12:44 PM, Andy Lutomirski wrote:
> On Mon, Jun 10, 2019 at 9:00 AM Jarkko Sakkinen
> <jarkko.sakkinen@xxxxxxxxxxxxxxx> wrote:
> > On Wed, Jun 05, 2019 at 07:11:43PM -0700, Sean Christopherson wrote:
> > > +             goto out;
> > > +     }
> > > +
> > > +     /*
> > > +      * Query VM_MAYEXEC as an indirect path_noexec() check (see do_mmap()),
> > > +      * but with some future proofing against other cases that may deny
> > > +      * execute permissions.
> > > +      */
> > > +     if (!(vma->vm_flags & VM_MAYEXEC)) {
> > > +             ret = -EACCES;
> > > +             goto out;
> > > +     }
> > > +
> > > +     if (copy_from_user(dst, (void __user *)src, PAGE_SIZE))
> > > +             ret = -EFAULT;
> > > +     else
> > > +             ret = 0;
> > > +
> > > +out:
> > > +     up_read(&current->mm->mmap_sem);
> > > +
> > > +     return ret;
> > > +}
> >
> > I would suggest to express the above instead like this for clarity
> > and consistency:
> >
> >                  goto err_map_sem;
> >          }
> >
> >          /* Query VM_MAYEXEC as an indirect path_noexec() check
> >           * (see do_mmap()).
> >           */
> >          if (!(vma->vm_flags & VM_MAYEXEC)) {
> >                  ret = -EACCES;
> >                  goto err_mmap_sem;
> >          }
> >
> >          if (copy_from_user(dst, (void __user *)src, PAGE_SIZE)) {
> >                  ret = -EFAULT;
> >                  goto err_mmap_sem;
> >          }
> >
> >          return 0;
> >
> > err_mmap_sem:
> >          up_read(&current->mm->mmap_sem);
> >          return ret;
> > }
> >
> > The comment about future proofing is unnecessary.
>
> I'm also torn as to whether this patch is needed at all.  If we ever
> get O_MAYEXEC, then enclave loaders should use it to enforce noexec in
> userspace.  Otherwise I'm unconvinced it's that special.

What's a situation where we would want to allow this?  Why is it 
different than do_mmap()?