summaryrefslogtreecommitdiffstats
path: root/audit-fix-a-double-fetch-in-audit_log_single_execve_arg.patch
diff options
context:
space:
mode:
Diffstat (limited to 'audit-fix-a-double-fetch-in-audit_log_single_execve_arg.patch')
-rw-r--r--audit-fix-a-double-fetch-in-audit_log_single_execve_arg.patch413
1 files changed, 0 insertions, 413 deletions
diff --git a/audit-fix-a-double-fetch-in-audit_log_single_execve_arg.patch b/audit-fix-a-double-fetch-in-audit_log_single_execve_arg.patch
deleted file mode 100644
index 6ee750466..000000000
--- a/audit-fix-a-double-fetch-in-audit_log_single_execve_arg.patch
+++ /dev/null
@@ -1,413 +0,0 @@
-From 43761473c254b45883a64441dd0bc85a42f3645c Mon Sep 17 00:00:00 2001
-From: Paul Moore <paul@paul-moore.com>
-Date: Tue, 19 Jul 2016 17:42:57 -0400
-Subject: [PATCH] audit: fix a double fetch in audit_log_single_execve_arg()
-
-There is a double fetch problem in audit_log_single_execve_arg()
-where we first check the execve(2) argumnets for any "bad" characters
-which would require hex encoding and then re-fetch the arguments for
-logging in the audit record[1]. Of course this leaves a window of
-opportunity for an unsavory application to munge with the data.
-
-This patch reworks things by only fetching the argument data once[2]
-into a buffer where it is scanned and logged into the audit
-records(s). In addition to fixing the double fetch, this patch
-improves on the original code in a few other ways: better handling
-of large arguments which require encoding, stricter record length
-checking, and some performance improvements (completely unverified,
-but we got rid of some strlen() calls, that's got to be a good
-thing).
-
-As part of the development of this patch, I've also created a basic
-regression test for the audit-testsuite, the test can be tracked on
-GitHub at the following link:
-
- * https://github.com/linux-audit/audit-testsuite/issues/25
-
-[1] If you pay careful attention, there is actually a triple fetch
-problem due to a strnlen_user() call at the top of the function.
-
-[2] This is a tiny white lie, we do make a call to strnlen_user()
-prior to fetching the argument data. I don't like it, but due to the
-way the audit record is structured we really have no choice unless we
-copy the entire argument at once (which would require a rather
-wasteful allocation). The good news is that with this patch the
-kernel no longer relies on this strnlen_user() value for anything
-beyond recording it in the log, we also update it with a trustworthy
-value whenever possible.
-
-Reported-by: Pengfei Wang <wpengfeinudt@gmail.com>
-Cc: <stable@vger.kernel.org>
-Signed-off-by: Paul Moore <paul@paul-moore.com>
----
- kernel/auditsc.c | 332 +++++++++++++++++++++++++++----------------------------
- 1 file changed, 164 insertions(+), 168 deletions(-)
-
-diff --git a/kernel/auditsc.c b/kernel/auditsc.c
-index aa3feec..c65af21 100644
---- a/kernel/auditsc.c
-+++ b/kernel/auditsc.c
-@@ -73,6 +73,7 @@
- #include <linux/compat.h>
- #include <linux/ctype.h>
- #include <linux/string.h>
-+#include <linux/uaccess.h>
- #include <uapi/linux/limits.h>
-
- #include "audit.h"
-@@ -82,7 +83,8 @@
- #define AUDITSC_SUCCESS 1
- #define AUDITSC_FAILURE 2
-
--/* no execve audit message should be longer than this (userspace limits) */
-+/* no execve audit message should be longer than this (userspace limits),
-+ * see the note near the top of audit_log_execve_info() about this value */
- #define MAX_EXECVE_AUDIT_LEN 7500
-
- /* max length to print of cmdline/proctitle value during audit */
-@@ -992,184 +994,178 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
- return rc;
- }
-
--/*
-- * to_send and len_sent accounting are very loose estimates. We aren't
-- * really worried about a hard cap to MAX_EXECVE_AUDIT_LEN so much as being
-- * within about 500 bytes (next page boundary)
-- *
-- * why snprintf? an int is up to 12 digits long. if we just assumed when
-- * logging that a[%d]= was going to be 16 characters long we would be wasting
-- * space in every audit message. In one 7500 byte message we can log up to
-- * about 1000 min size arguments. That comes down to about 50% waste of space
-- * if we didn't do the snprintf to find out how long arg_num_len was.
-- */
--static int audit_log_single_execve_arg(struct audit_context *context,
-- struct audit_buffer **ab,
-- int arg_num,
-- size_t *len_sent,
-- const char __user *p,
-- char *buf)
-+static void audit_log_execve_info(struct audit_context *context,
-+ struct audit_buffer **ab)
- {
-- char arg_num_len_buf[12];
-- const char __user *tmp_p = p;
-- /* how many digits are in arg_num? 5 is the length of ' a=""' */
-- size_t arg_num_len = snprintf(arg_num_len_buf, 12, "%d", arg_num) + 5;
-- size_t len, len_left, to_send;
-- size_t max_execve_audit_len = MAX_EXECVE_AUDIT_LEN;
-- unsigned int i, has_cntl = 0, too_long = 0;
-- int ret;
--
-- /* strnlen_user includes the null we don't want to send */
-- len_left = len = strnlen_user(p, MAX_ARG_STRLEN) - 1;
--
-- /*
-- * We just created this mm, if we can't find the strings
-- * we just copied into it something is _very_ wrong. Similar
-- * for strings that are too long, we should not have created
-- * any.
-- */
-- if (WARN_ON_ONCE(len < 0 || len > MAX_ARG_STRLEN - 1)) {
-- send_sig(SIGKILL, current, 0);
-- return -1;
-+ long len_max;
-+ long len_rem;
-+ long len_full;
-+ long len_buf;
-+ long len_abuf;
-+ long len_tmp;
-+ bool require_data;
-+ bool encode;
-+ unsigned int iter;
-+ unsigned int arg;
-+ char *buf_head;
-+ char *buf;
-+ const char __user *p = (const char __user *)current->mm->arg_start;
-+
-+ /* NOTE: this buffer needs to be large enough to hold all the non-arg
-+ * data we put in the audit record for this argument (see the
-+ * code below) ... at this point in time 96 is plenty */
-+ char abuf[96];
-+
-+ /* NOTE: we set MAX_EXECVE_AUDIT_LEN to a rather arbitrary limit, the
-+ * current value of 7500 is not as important as the fact that it
-+ * is less than 8k, a setting of 7500 gives us plenty of wiggle
-+ * room if we go over a little bit in the logging below */
-+ WARN_ON_ONCE(MAX_EXECVE_AUDIT_LEN > 7500);
-+ len_max = MAX_EXECVE_AUDIT_LEN;
-+
-+ /* scratch buffer to hold the userspace args */
-+ buf_head = kmalloc(MAX_EXECVE_AUDIT_LEN + 1, GFP_KERNEL);
-+ if (!buf_head) {
-+ audit_panic("out of memory for argv string");
-+ return;
- }
-+ buf = buf_head;
-
-- /* walk the whole argument looking for non-ascii chars */
-+ audit_log_format(*ab, "argc=%d", context->execve.argc);
-+
-+ len_rem = len_max;
-+ len_buf = 0;
-+ len_full = 0;
-+ require_data = true;
-+ encode = false;
-+ iter = 0;
-+ arg = 0;
- do {
-- if (len_left > MAX_EXECVE_AUDIT_LEN)
-- to_send = MAX_EXECVE_AUDIT_LEN;
-- else
-- to_send = len_left;
-- ret = copy_from_user(buf, tmp_p, to_send);
-- /*
-- * There is no reason for this copy to be short. We just
-- * copied them here, and the mm hasn't been exposed to user-
-- * space yet.
-- */
-- if (ret) {
-- WARN_ON(1);
-- send_sig(SIGKILL, current, 0);
-- return -1;
-- }
-- buf[to_send] = '\0';
-- has_cntl = audit_string_contains_control(buf, to_send);
-- if (has_cntl) {
-- /*
-- * hex messages get logged as 2 bytes, so we can only
-- * send half as much in each message
-- */
-- max_execve_audit_len = MAX_EXECVE_AUDIT_LEN / 2;
-- break;
-- }
-- len_left -= to_send;
-- tmp_p += to_send;
-- } while (len_left > 0);
--
-- len_left = len;
--
-- if (len > max_execve_audit_len)
-- too_long = 1;
--
-- /* rewalk the argument actually logging the message */
-- for (i = 0; len_left > 0; i++) {
-- int room_left;
--
-- if (len_left > max_execve_audit_len)
-- to_send = max_execve_audit_len;
-- else
-- to_send = len_left;
--
-- /* do we have space left to send this argument in this ab? */
-- room_left = MAX_EXECVE_AUDIT_LEN - arg_num_len - *len_sent;
-- if (has_cntl)
-- room_left -= (to_send * 2);
-- else
-- room_left -= to_send;
-- if (room_left < 0) {
-- *len_sent = 0;
-- audit_log_end(*ab);
-- *ab = audit_log_start(context, GFP_KERNEL, AUDIT_EXECVE);
-- if (!*ab)
-- return 0;
-- }
-+ /* NOTE: we don't ever want to trust this value for anything
-+ * serious, but the audit record format insists we
-+ * provide an argument length for really long arguments,
-+ * e.g. > MAX_EXECVE_AUDIT_LEN, so we have no choice but
-+ * to use strncpy_from_user() to obtain this value for
-+ * recording in the log, although we don't use it
-+ * anywhere here to avoid a double-fetch problem */
-+ if (len_full == 0)
-+ len_full = strnlen_user(p, MAX_ARG_STRLEN) - 1;
-+
-+ /* read more data from userspace */
-+ if (require_data) {
-+ /* can we make more room in the buffer? */
-+ if (buf != buf_head) {
-+ memmove(buf_head, buf, len_buf);
-+ buf = buf_head;
-+ }
-+
-+ /* fetch as much as we can of the argument */
-+ len_tmp = strncpy_from_user(&buf_head[len_buf], p,
-+ len_max - len_buf);
-+ if (len_tmp == -EFAULT) {
-+ /* unable to copy from userspace */
-+ send_sig(SIGKILL, current, 0);
-+ goto out;
-+ } else if (len_tmp == (len_max - len_buf)) {
-+ /* buffer is not large enough */
-+ require_data = true;
-+ /* NOTE: if we are going to span multiple
-+ * buffers force the encoding so we stand
-+ * a chance at a sane len_full value and
-+ * consistent record encoding */
-+ encode = true;
-+ len_full = len_full * 2;
-+ p += len_tmp;
-+ } else {
-+ require_data = false;
-+ if (!encode)
-+ encode = audit_string_contains_control(
-+ buf, len_tmp);
-+ /* try to use a trusted value for len_full */
-+ if (len_full < len_max)
-+ len_full = (encode ?
-+ len_tmp * 2 : len_tmp);
-+ p += len_tmp + 1;
-+ }
-+ len_buf += len_tmp;
-+ buf_head[len_buf] = '\0';
-
-- /*
-- * first record needs to say how long the original string was
-- * so we can be sure nothing was lost.
-- */
-- if ((i == 0) && (too_long))
-- audit_log_format(*ab, " a%d_len=%zu", arg_num,
-- has_cntl ? 2*len : len);
--
-- /*
-- * normally arguments are small enough to fit and we already
-- * filled buf above when we checked for control characters
-- * so don't bother with another copy_from_user
-- */
-- if (len >= max_execve_audit_len)
-- ret = copy_from_user(buf, p, to_send);
-- else
-- ret = 0;
-- if (ret) {
-- WARN_ON(1);
-- send_sig(SIGKILL, current, 0);
-- return -1;
-+ /* length of the buffer in the audit record? */
-+ len_abuf = (encode ? len_buf * 2 : len_buf + 2);
- }
-- buf[to_send] = '\0';
--
-- /* actually log it */
-- audit_log_format(*ab, " a%d", arg_num);
-- if (too_long)
-- audit_log_format(*ab, "[%d]", i);
-- audit_log_format(*ab, "=");
-- if (has_cntl)
-- audit_log_n_hex(*ab, buf, to_send);
-- else
-- audit_log_string(*ab, buf);
--
-- p += to_send;
-- len_left -= to_send;
-- *len_sent += arg_num_len;
-- if (has_cntl)
-- *len_sent += to_send * 2;
-- else
-- *len_sent += to_send;
-- }
-- /* include the null we didn't log */
-- return len + 1;
--}
-
--static void audit_log_execve_info(struct audit_context *context,
-- struct audit_buffer **ab)
--{
-- int i, len;
-- size_t len_sent = 0;
-- const char __user *p;
-- char *buf;
-+ /* write as much as we can to the audit log */
-+ if (len_buf > 0) {
-+ /* NOTE: some magic numbers here - basically if we
-+ * can't fit a reasonable amount of data into the
-+ * existing audit buffer, flush it and start with
-+ * a new buffer */
-+ if ((sizeof(abuf) + 8) > len_rem) {
-+ len_rem = len_max;
-+ audit_log_end(*ab);
-+ *ab = audit_log_start(context,
-+ GFP_KERNEL, AUDIT_EXECVE);
-+ if (!*ab)
-+ goto out;
-+ }
-
-- p = (const char __user *)current->mm->arg_start;
-+ /* create the non-arg portion of the arg record */
-+ len_tmp = 0;
-+ if (require_data || (iter > 0) ||
-+ ((len_abuf + sizeof(abuf)) > len_rem)) {
-+ if (iter == 0) {
-+ len_tmp += snprintf(&abuf[len_tmp],
-+ sizeof(abuf) - len_tmp,
-+ " a%d_len=%lu",
-+ arg, len_full);
-+ }
-+ len_tmp += snprintf(&abuf[len_tmp],
-+ sizeof(abuf) - len_tmp,
-+ " a%d[%d]=", arg, iter++);
-+ } else
-+ len_tmp += snprintf(&abuf[len_tmp],
-+ sizeof(abuf) - len_tmp,
-+ " a%d=", arg);
-+ WARN_ON(len_tmp >= sizeof(abuf));
-+ abuf[sizeof(abuf) - 1] = '\0';
-+
-+ /* log the arg in the audit record */
-+ audit_log_format(*ab, "%s", abuf);
-+ len_rem -= len_tmp;
-+ len_tmp = len_buf;
-+ if (encode) {
-+ if (len_abuf > len_rem)
-+ len_tmp = len_rem / 2; /* encoding */
-+ audit_log_n_hex(*ab, buf, len_tmp);
-+ len_rem -= len_tmp * 2;
-+ len_abuf -= len_tmp * 2;
-+ } else {
-+ if (len_abuf > len_rem)
-+ len_tmp = len_rem - 2; /* quotes */
-+ audit_log_n_string(*ab, buf, len_tmp);
-+ len_rem -= len_tmp + 2;
-+ /* don't subtract the "2" because we still need
-+ * to add quotes to the remaining string */
-+ len_abuf -= len_tmp;
-+ }
-+ len_buf -= len_tmp;
-+ buf += len_tmp;
-+ }
-
-- audit_log_format(*ab, "argc=%d", context->execve.argc);
-+ /* ready to move to the next argument? */
-+ if ((len_buf == 0) && !require_data) {
-+ arg++;
-+ iter = 0;
-+ len_full = 0;
-+ require_data = true;
-+ encode = false;
-+ }
-+ } while (arg < context->execve.argc);
-
-- /*
-- * we need some kernel buffer to hold the userspace args. Just
-- * allocate one big one rather than allocating one of the right size
-- * for every single argument inside audit_log_single_execve_arg()
-- * should be <8k allocation so should be pretty safe.
-- */
-- buf = kmalloc(MAX_EXECVE_AUDIT_LEN + 1, GFP_KERNEL);
-- if (!buf) {
-- audit_panic("out of memory for argv string");
-- return;
-- }
-+ /* NOTE: the caller handles the final audit_log_end() call */
-
-- for (i = 0; i < context->execve.argc; i++) {
-- len = audit_log_single_execve_arg(context, ab, i,
-- &len_sent, p, buf);
-- if (len <= 0)
-- break;
-- p += len;
-- }
-- kfree(buf);
-+out:
-+ kfree(buf_head);
- }
-
- static void show_special(struct audit_context *context, int *call_panic)
generated by cgit (git 2.34.1) at 2024-05-20 04:17:50 +0000