[PATCH] SELinux: fix bugs in mprotect hook

The patch below by Roland McGrath fixes two bugs in the implementation of
the selinux_file_mprotect hook:

  It calls selinux_file_mmap, which has two problems.  First, the stacked
  security module will get both mmap and mprotect callbacks for an
  mprotect call, which is wrong.  Secondly, the vm_flags value contains
  VM_* bits, and these do not match the MAP_* bits of the same name or
  function, so it passes bogus flags and causes every mprotect to be
  treated as if MAP_SHARED were in use.

  The patch shares the common code while not having one function call the
  other, and fixes these two bugs.

Signed-off-by: James Morris <jmorris@redhat.com>
Signed-off-by: Roland McGrath <roland@redhat.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 231d3f4863b3b4a848f24960ee52e0a7ee27a43f..f844e402b53c2f76f64a90fb7831cb7bb96b03a3 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2466,21 +2466,14 @@ static int selinux_file_ioctl(struct file *file, unsigned int cmd,
        return error;
 }
 
-static int selinux_file_mmap(struct file *file, unsigned long prot, unsigned long flags)
+static int file_map_prot_check(struct file *file, unsigned long prot, int shared)
 {
-       u32 av;
-       int rc;
-
-       rc = secondary_ops->file_mmap(file, prot, flags);
-       if (rc)
-               return rc;
-
        if (file) {
                /* read access is always possible with a mapping */
-               av = FILE__READ;
+               u32 av = FILE__READ;
 
                /* write access only matters if the mapping is shared */
-               if ((flags & MAP_TYPE) == MAP_SHARED && (prot & PROT_WRITE))
+               if (shared && (prot & PROT_WRITE))
                        av |= FILE__WRITE;
 
                if (prot & PROT_EXEC)
@@ -2491,6 +2484,18 @@ static int selinux_file_mmap(struct file *file, unsigned long prot, unsigned lon
        return 0;
 }
 
+static int selinux_file_mmap(struct file *file, unsigned long prot, unsigned long flags)
+{
+       int rc;
+
+       rc = secondary_ops->file_mmap(file, prot, flags);
+       if (rc)
+               return rc;
+
+       return file_map_prot_check(file, prot,
+                                  (flags & MAP_TYPE) == MAP_SHARED);
+}
+
 static int selinux_file_mprotect(struct vm_area_struct *vma,
                                 unsigned long prot)
 {
@@ -2500,7 +2505,7 @@ static int selinux_file_mprotect(struct vm_area_struct *vma,
        if (rc)
                return rc;
 
-       return selinux_file_mmap(vma->vm_file, prot, vma->vm_flags);
+       return file_map_prot_check(vma->vm_file, prot, vma->vm_flags&VM_SHARED);
 }
 
 static int selinux_file_lock(struct file *file, unsigned int cmd)