From: William Roberts <bill.c.roberts@gmail.com>
To: linux-audit@redhat.com, rgb@redhat.com
Cc: William Roberts <wroberts@tresys.com>
Subject: [PATCH 4/4] SQUASH audit: Change cmdline get API to reduce locking
Date: Wed, 20 Nov 2013 17:29:19 -0800 [thread overview]
Message-ID: <1384997359-23649-5-git-send-email-wroberts@tresys.com> (raw)
In-Reply-To: <1384997359-23649-1-git-send-email-wroberts@tresys.com>
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@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
next prev parent reply other threads:[~2013-11-21 1:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-21 1:29 [DRAFT v3.4] - audit cmdline updates William Roberts
2013-11-21 1:29 ` [PATCH 1/4] audit: Allow auditing of proc/self/cmdline value William Roberts
2013-11-21 1:29 ` [PATCH 2/4] audit: Enable cacheing of cmdline in audit_context William Roberts
2013-11-21 1:29 ` [PATCH 3/4] audit: dont allocate whole pages William Roberts
2013-11-21 1:29 ` William Roberts [this message]
2013-11-21 1:34 ` [DRAFT v3.4] - audit cmdline updates William Roberts
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1384997359-23649-5-git-send-email-wroberts@tresys.com \
--to=bill.c.roberts@gmail.com \
--cc=linux-audit@redhat.com \
--cc=rgb@redhat.com \
--cc=wroberts@tresys.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox