selinux: fix incorrect execmem checks on overlayfs

The commit fixing the overlayfs mmap() and mprotect() access checks
failed to skip the execmem check in __file_map_prot_check() for the case
where the "mounter check" is being performed. This check should be
performed only against the credentials of the task that is calling
mmap()/mprotect(), since it doesn't pertain to the file itself, but
rather just gates the ability of the calling task to get an executable
memory mapping in general.

The purpose of the "mounter check" is to guard against using an
overlayfs mount to gain file access that would otherwise be denied to
the mounter. For execmem this is not relevant, as there is no further
file access granted based on it (notice that the file's context is not
used as the target in the check), so checking it also against the
mounter credentials would be incorrect.

Fix this by passing a boolean to [__]file_map_prot_check() and
selinux_mmap_file_common() that indicates if we are doing the "mounter
check" and skiping the execmem check in that case. Since this boolean
also indicates if we use current_cred() or the mounter cred as the
subject, also remove the "cred" argument from these functions and
determine it based on the boolean and the file struct.

Cc: stable@vger.kernel.org
Fixes: 82544d36b172 ("selinux: fix overlayfs mmap() and mprotect() access checks")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 70a3388c047d922b22fbba0b46457f5855029712..8d6945edae7aacf34fd48a1876469305160c1a93 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3969,9 +3969,9 @@ static int selinux_file_ioctl_compat(struct file *file, unsigned int cmd,
 
 static int default_noexec __ro_after_init;
 
-static int __file_map_prot_check(const struct cred *cred,
-                                const struct file *file, unsigned long prot,
-                                bool shared, bool bf_user_file)
+static int __file_map_prot_check(const struct file *file, unsigned long prot,
+                                bool shared, bool mounter_check,
+                                bool bf_user_file)
 {
        struct inode *inode = NULL;
        bool prot_exec = prot & PROT_EXEC;
@@ -3984,10 +3984,10 @@ static int __file_map_prot_check(const struct cred *cred,
                        inode = file_inode(file);
        }
 
-       if (default_noexec && prot_exec &&
+       if (!mounter_check && default_noexec && prot_exec &&
            (!file || IS_PRIVATE(inode) || (!shared && prot_write))) {
                int rc;
-               u32 sid = cred_sid(cred);
+               u32 sid = current_sid();
 
                /*
                 * We are making executable an anonymous mapping or a private
@@ -4000,6 +4000,8 @@ static int __file_map_prot_check(const struct cred *cred,
        }
 
        if (file) {
+               const struct cred *cred = mounter_check ?
+                               file->f_cred : current_cred();
                /* "read" always possible, "write" only if shared */
                u32 av = FILE__READ;
                if (shared && prot_write)
@@ -4013,11 +4015,11 @@ static int __file_map_prot_check(const struct cred *cred,
        return 0;
 }
 
-static inline int file_map_prot_check(const struct cred *cred,
-                                     const struct file *file,
-                                     unsigned long prot, bool shared)
+static inline int file_map_prot_check(const struct file *file,
+                                     unsigned long prot, bool shared,
+                                     bool mounter_check)
 {
-       return __file_map_prot_check(cred, file, prot, shared, false);
+       return __file_map_prot_check(file, prot, shared, mounter_check, false);
 }
 
 static int selinux_mmap_addr(unsigned long addr)
@@ -4033,12 +4035,14 @@ static int selinux_mmap_addr(unsigned long addr)
        return rc;
 }
 
-static int selinux_mmap_file_common(const struct cred *cred, struct file *file,
-                                   unsigned long prot, bool shared)
+static int selinux_mmap_file_common(struct file *file, unsigned long prot,
+                                   bool shared, bool mounter_check)
 {
        if (file) {
                int rc;
                struct common_audit_data ad;
+               const struct cred *cred = mounter_check ?
+                               file->f_cred : current_cred();
 
                ad.type = LSM_AUDIT_DATA_FILE;
                ad.u.file = file;
@@ -4047,15 +4051,16 @@ static int selinux_mmap_file_common(const struct cred *cred, struct file *file,
                        return rc;
        }
 
-       return file_map_prot_check(cred, file, prot, shared);
+       return file_map_prot_check(file, prot, shared, mounter_check);
 }
 
 static int selinux_mmap_file(struct file *file,
                             unsigned long reqprot __always_unused,
                             unsigned long prot, unsigned long flags)
 {
-       return selinux_mmap_file_common(current_cred(), file, prot,
-                                       (flags & MAP_TYPE) == MAP_SHARED);
+       return selinux_mmap_file_common(file, prot,
+                                       (flags & MAP_TYPE) == MAP_SHARED,
+                                       false);
 }
 
 /**
@@ -4087,8 +4092,9 @@ static int selinux_mmap_backing_file(struct vm_area_struct *vma,
        if (vma->vm_flags & VM_EXEC)
                prot |= PROT_EXEC;
 
-       return selinux_mmap_file_common(backing_file->f_cred, backing_file,
-                                       prot, vma->vm_flags & VM_SHARED);
+       return selinux_mmap_file_common(backing_file, prot,
+                                       vma->vm_flags & VM_SHARED,
+                                       true);
 }
 
 static int selinux_file_mprotect(struct vm_area_struct *vma,
@@ -4149,11 +4155,11 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
                }
        }
 
-       rc = __file_map_prot_check(cred, file, prot, shared, backing_file);
+       rc = __file_map_prot_check(file, prot, shared, false, backing_file);
        if (rc)
                return rc;
        if (backing_file) {
-               rc = file_map_prot_check(file->f_cred, file, prot, shared);
+               rc = file_map_prot_check(file, prot, shared, true);
                if (rc)
                        return rc;
        }