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

[Jarkko Sakkinen]

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.

/Jarkk