[PATCH 3/4] prctl: move MMF_EXE_FILE_CHANGED into exe_file struct

[Davidlohr Bueso]

Given the introduction of the exe_file structure, this
functionality should be associated with. Because this is
a very specific prctl property, it is easily to do so. As
of now, when the file has already changed, mmap_sem is not
taken at all (however we do need it of course to check the
old mapping, but this is now shared) and we maintain the
test-and-set logic ensuring nothing raced when we were not
holding the exe_file lock.

Now, the downside is that this patch makes MMF_EXE_FILE_CHANGED
functionality general, of course trivially enlarging the mm_struct
to users that don't care about this - which is the most usual case.
But I don't see this of any importance really. Similarly the funcs
that prctl makes use of are also global, in fork.c -- I preferred
leaving things generic for any(?) future user(s), but it could very
well be moved to sys.c.

Signed-off-by: Davidlohr Bueso <dbueso@xxxxxxx>
Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: Cyrill Gorcunov <gorcunov@xxxxxxxxxx>
Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
CC: Konstantin Khlebnikov <koct9i@xxxxxxxxx>
---
 include/linux/mm.h       |  5 +++--
 include/linux/mm_types.h |  1 +
 include/linux/sched.h    |  5 ++---
 kernel/fork.c            | 37 +++++++++++++++++++++++++++---
 kernel/sys.c             | 58 +++++++++++++++++++++++++-----------------------
 5 files changed, 70 insertions(+), 36 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0c0720d..90eae9f 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1911,9 +1911,10 @@ extern void mm_drop_all_locks(struct mm_struct *mm);
 
 /* mm->exe_file handling */
 extern void set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file);
-extern void set_mm_exe_file_locked(struct mm_struct *mm,
-				   struct file *new_exe_file);
 extern struct file *get_mm_exe_file(struct mm_struct *mm);
+extern bool test_and_set_mm_exe_file(struct mm_struct *mm,
+				     struct file *new_exe_file);
+extern bool mm_exe_file_changed(struct mm_struct *mm);
 
 extern int may_expand_vm(struct mm_struct *mm, unsigned long npages);
 extern struct vm_area_struct *_install_special_mapping(struct mm_struct *mm,
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 1fc994e..2d8b06b 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -328,6 +328,7 @@ struct core_state {
 
 struct exe_file {
 	rwlock_t lock;
+	bool changed; /* see prctl_set_mm_exe_file() */
 	struct file *file;
 };
 
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6d77432..0caf62e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -494,10 +494,9 @@ static inline int get_dumpable(struct mm_struct *mm)
 					/* leave room for more dump flags */
 #define MMF_VM_MERGEABLE	16	/* KSM may merge identical pages */
 #define MMF_VM_HUGEPAGE		17	/* set when VM_HUGEPAGE is set on vma */
-#define MMF_EXE_FILE_CHANGED	18	/* see prctl_set_mm_exe_file() */
 
-#define MMF_HAS_UPROBES		19	/* has uprobes */
-#define MMF_RECALC_UPROBES	20	/* MMF_HAS_UPROBES can be wrong */
+#define MMF_HAS_UPROBES		18	/* has uprobes */
+#define MMF_RECALC_UPROBES	19	/* MMF_HAS_UPROBES can be wrong */
 
 #define MMF_INIT_MASK		(MMF_DUMPABLE_MASK | MMF_DUMP_FILTER_MASK)
 
diff --git a/kernel/fork.c b/kernel/fork.c
index aa0332b..54b0b91 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -675,18 +675,50 @@ void mmput(struct mm_struct *mm)
 }
 EXPORT_SYMBOL_GPL(mmput);
 
-void set_mm_exe_file_locked(struct mm_struct *mm, struct file *new_exe_file)
+/*
+ * exe_file handling is differentiated by the caller's need to
+ * be aware of the file being changed -- which will always require
+ * holding the exe_file lock. As such, the following functions
+ * keep track of this are (currently only used by prctl):
+ *   - test_and_set_mm_exe_file()
+ *   - mm_exe_file_changed()
+ *
+ * The rest of the callers should only stick to:
+ *   - set_mm_exe_file()
+ *   - get_mm_exe_file()
+ */
+bool test_and_set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
 {
+	bool ret = false;
+
+	write_lock(&mm->exe_file.lock);
+	if (mm->exe_file.changed)
+		goto done;
+
 	if (new_exe_file)
 		get_file(new_exe_file);
 	if (mm->exe_file.file)
 		fput(mm->exe_file.file);
 
-	write_lock(&mm->exe_file.lock);
 	mm->exe_file.file = new_exe_file;
+	ret = mm->exe_file.changed = true;
+done:
 	write_unlock(&mm->exe_file.lock);
+	return ret;
 }
 
+bool mm_exe_file_changed(struct mm_struct *mm)
+{
+	bool ret;
+
+	read_lock(&mm->exe_file.lock);
+	ret = mm->exe_file.changed;
+	read_lock(&mm->exe_file.lock);
+
+	return ret;
+}
+
+/* lockless -- see each caller for justification */
 void set_mm_exe_file(struct mm_struct *mm, struct file *new_exe_file)
 {
 	if (new_exe_file)
@@ -711,7 +743,6 @@ struct file *get_mm_exe_file(struct mm_struct *mm)
 }
 EXPORT_SYMBOL(get_mm_exe_file);
 
-
 static void dup_mm_exe_file(struct mm_struct *oldmm, struct mm_struct *newmm)
 {
 	/*
diff --git a/kernel/sys.c b/kernel/sys.c
index a4d70f0..a82d0c4 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1649,17 +1649,27 @@ SYSCALL_DEFINE1(umask, int, mask)
 	return mask;
 }
 
-static int __prctl_set_mm_exe_file(struct mm_struct *mm, struct fd exe)
+static int __prctl_set_mm_exe_file(struct mm_struct *mm, struct fd exefd)
 {
-	int err;
 	struct file *exe_file;
 
 	/*
+	 * The symlink can be changed only once, just to disallow arbitrary
+	 * transitions malicious software might bring in. This means one
+	 * could make a snapshot over all processes running and monitor
+	 * /proc/pid/exe changes to notice unusual activity if needed.
+	 */
+	if (mm_exe_file_changed(mm))
+		return -EPERM;
+
+	/*
 	 * Forbid mm->exe_file change if old file still mapped.
 	 */
 	exe_file = get_mm_exe_file(mm);
-	err = -EBUSY;
-	down_write(&mm->mmap_sem);
+	if (!exe_file)
+		goto set_file;
+
+	down_read(&mm->mmap_sem);
 	if (exe_file) {
 		struct vm_area_struct *vma;
 
@@ -1669,44 +1679,36 @@ static int __prctl_set_mm_exe_file(struct mm_struct *mm, struct fd exe)
 			if (path_equal(&vma->vm_file->f_path,
 				       &exe_file->f_path)) {
 				fput(exe_file);
-				goto exit_err;
+				up_read(&mm->mmap_sem);
+				return -EBUSY;
 			}
 		}
 
 		fput(exe_file);
 	}
+	up_read(&mm->mmap_sem);
 
+set_file:
 	/*
-	 * The symlink can be changed only once, just to disallow arbitrary
-	 * transitions malicious software might bring in. This means one
-	 * could make a snapshot over all processes running and monitor
-	 * /proc/pid/exe changes to notice unusual activity if needed.
+	 * Recheck the file state again before setting.
+	 * This grabs a reference to exefd.file.
 	 */
-	err = -EPERM;
-	if (test_and_set_bit(MMF_EXE_FILE_CHANGED, &mm->flags))
-		goto exit_err;
-
-	up_write(&mm->mmap_sem);
-
-	/* this grabs a reference to exe.file */
-	set_mm_exe_file_locked(mm, exe.file);
-	return 0;
-exit_err:
-	up_write(&mm->mmap_sem);
-	return err;
+	if (test_and_set_mm_exe_file(mm, exefd.file))
+		return 0;
+	return -EPERM;
 }
 
 static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
 {
-	struct fd exe;
+	struct fd exefd;
 	struct inode *inode;
 	int err;
 
-	exe = fdget(fd);
-	if (!exe.file)
+	exefd = fdget(fd);
+	if (!exefd.file)
 		return -EBADF;
 
-	inode = file_inode(exe.file);
+	inode = file_inode(exefd.file);
 
 	/*
 	 * Because the original mm->exe_file points to executable file, make
@@ -1715,16 +1717,16 @@ static int prctl_set_mm_exe_file(struct mm_struct *mm, unsigned int fd)
 	 */
 	err = -EACCES;
 	if (!S_ISREG(inode->i_mode)	||
-	    exe.file->f_path.mnt->mnt_flags & MNT_NOEXEC)
+	    exefd.file->f_path.mnt->mnt_flags & MNT_NOEXEC)
 		goto exit;
 
 	err = inode_permission(inode, MAY_EXEC);
 	if (err)
 		goto exit;
 
-	err = __prctl_set_mm_exe_file(mm, exe);
+	err = __prctl_set_mm_exe_file(mm, exefd);
 exit:
-	fdput(exe);
+	fdput(exefd);
 	return err;
 }
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/