[PATCH 4/4] SQUASH audit: Change cmdline get API to reduce locking

William Roberts bill.c.roberts at gmail.com
Thu Nov 21 01:29:19 UTC 2013


Each call to length copy required a call to get_task_mm() and mmput.
Just require the caller to aquire and pass a valid mm.

Change-Id: Id7069b80f1cbea5b30032a0a459dd54b7446f665
Signed-off-by: William Roberts <wroberts at tresys.com>
---
 fs/proc/base.c          |   63 +++++++++++++++--------------------------------
 include/linux/proc_fs.h |   13 +++++++---
 kernel/auditsc.c        |   28 +++++++++++++++------
 3 files changed, 51 insertions(+), 53 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index a0751ed..4d74830 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -210,51 +210,21 @@ struct mm_struct *mm_for_maps(struct task_struct *task)
 }
 
 /*
- * Returns the length of the VM area containing the tasks cmdline info.
- * 0 indicates success
+ * Copy's the tasks cmdline data into buf, truncating at buflen
+ * must call holding semaphore on mm
  */
-int proc_pid_cmdline_length(struct task_struct *task, unsigned int *len)
-{
-	int res = -1;
-	struct mm_struct *mm;
-
-	if (!task || !len)
-		return 0;
-
-	mm = get_task_mm(task);
-	if (!mm)
-		goto out;
-	if (!mm->arg_end)
-		goto out_mm;	/* Shh! No looking before we're done */
-
-	*len = mm->arg_end - mm->arg_start;
-	res = 0;
-out_mm:
-	mmput(mm);
-out:
-	return res;
-}
-
-/* Copy's the tasks cmdline data into buf, truncating at buflen */
-int proc_pid_copy_cmdline_to_buf(struct task_struct *task, char *buf,
-				 unsigned int buflen)
+int proc_pid_copy_cmdline_to_buf(struct task_struct *task, struct mm_struct *mm,
+				 char *buf, unsigned int buflen)
 {
 	int res = 0;
 	unsigned int len;
-	struct mm_struct *mm;
 
-	if (!buflen || !buf)
+	if (!task || !mm || !buf)
 		return 0;
 
-	mm = get_task_mm(task);
-	if (!mm)
-		goto out;
-	if (!mm->arg_end)
-		goto out_mm;	/* Shh! No looking before we're done */
-
 	res = access_process_vm(task, mm->arg_start, buf, buflen, 0);
 	if (res <= 0)
-		goto out_mm;
+		return 0;
 
 	/* Truncate to res if buflen is longer */
 	if (res > buflen)
@@ -281,29 +251,36 @@ int proc_pid_copy_cmdline_to_buf(struct task_struct *task, char *buf,
 			res = strnlen(buf, res);
 		}
 	}
-out_mm:
-	mmput(mm);
-out:
 	return res;
 }
 
 static int proc_pid_cmdline(struct task_struct *task, char *buffer)
 {
-	unsigned int len;
-	int res = proc_pid_cmdline_length(task, &len);
-	if (res)
+	int res = 0;
+	unsigned int len = 0;
+	struct mm_struct *mm = get_task_mm(task);
+	if (!mm)
 		return 0;
 
+	len = proc_pid_cmdline_length(mm);
+	if (!len)
+		goto mm_out;
+
 	/* The caller of this allocates a page */
 	if (len > PAGE_SIZE)
 		len = PAGE_SIZE;
 
-	res = proc_pid_copy_cmdline_to_buf(task, buffer, len);
+	res = proc_pid_copy_cmdline_to_buf(task, mm, buffer, len);
+	if (!res)
+		goto mm_out;
+
 	/*
 	 * Ensure NULL terminated! Application could
 	 * could be using setproctitle(3).
 	 */
 	buffer[res-1] = '\0';
+mm_out:
+	mmput(mm);
 	return res;
 }
 
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index f76deb3..8bc2718 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -6,6 +6,7 @@
 #include <linux/spinlock.h>
 #include <linux/magic.h>
 #include <linux/atomic.h>
+#include <linux/mm.h>
 
 struct net;
 struct completion;
@@ -119,10 +120,16 @@ struct pid_namespace;
 extern int pid_ns_prepare_proc(struct pid_namespace *ns);
 extern void pid_ns_release_proc(struct pid_namespace *ns);
 
-extern int proc_pid_cmdline_length(struct task_struct *task, unsigned int *len);
-extern int proc_pid_copy_cmdline_to_buf(struct task_struct *task, char *buf,
-					unsigned int buflen);
+/* must call holding semaphore on mm */
+static inline unsigned int proc_pid_cmdline_length(struct mm_struct *mm)
+{
+	 return mm->arg_end ? mm->arg_end - mm->arg_start : 0;
+}
 
+extern int proc_pid_copy_cmdline_to_buf(struct task_struct *task,
+					struct mm_struct *mm,
+					char *buf,
+					unsigned int buflen);
 /*
  * proc_tty.c
  */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 34a6c1d..8bd0549 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1165,29 +1165,37 @@ error_path:
 EXPORT_SYMBOL(audit_log_task_context);
 
 static char *audit_cmdline_get(struct audit_buffer *ab,
-				  struct task_struct *tsk)
+			       struct task_struct *tsk)
 {
 	int len;
 	int res;
 	char *buf;
+	struct mm_struct *mm;
+
+	if (!ab || !tsk)
+		return NULL;
 
-	res = proc_pid_cmdline_length(tsk, &len);
-	if (res != 0 || len == 0)
+	mm = get_task_mm(tsk);
+	if (!mm)
 		return NULL;
 
+	len = proc_pid_cmdline_length(mm);
+	if (!len)
+		goto mm_err;
+
 	if (len > PATH_MAX)
 		len = PATH_MAX;
 
 	buf = kmalloc(len, GFP_KERNEL);
 	if (!buf)
-		return NULL;
+		goto mm_err;
 
-	res = proc_pid_copy_cmdline_to_buf(tsk, buf, len);
+	res = proc_pid_copy_cmdline_to_buf(tsk, mm, buf, len);
 	if (res <= 0) {
-		kfree(buf);
-		return NULL;
+		goto alloc_err;
 	}
 
+	mmput(mm);
 	/*
 	 * res is guarenteed not to be longer than
 	 * the buf as it was truncated to len
@@ -1201,6 +1209,12 @@ static char *audit_cmdline_get(struct audit_buffer *ab,
 	 */
 	buf[len-1] = '\0';
 	return buf;
+
+alloc_err:
+	kfree(buf);
+mm_err:
+	mmput(mm);
+	return NULL;
 }
 
 static void audit_log_cmdline(struct audit_buffer *ab, struct task_struct *tsk,
-- 
1.7.9.5




More information about the Linux-audit mailing list