From: Jerome Marchand <jmarchan@redhat.com>
Date: Thu, 31 Jul 2008 15:08:01 +0200
Subject: [misc] remove MAX_ARG_PAGES limit: rework execve audit
Message-id: 4891B931.3090109@redhat.com
O-Subject: Re: [RHEL5.3 2/3] Remove MAX_ARG_PAGES limit: rework execve audit
Bugzilla: 443659
RH-Acked-by: Eric Paris <eparis@redhat.com>
RH-Acked-by: Peter Zijlstra <pzijlstr@redhat.com>

Remove MAX_ARG_PAGES limit: rework execve audit

Bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=443659

Description: (from cvs commit)
The purpose of audit_bprm() is to log the argv array to a userspace
daemon at the end of the execve system call.  Since user-space hasn't
had time to run, this array is still in pristine state on the process'
stack; so no need to copy it, we can just grab it from there.

In order to minimize the damage to audit_log_*() copy each string into
a temporary kernel buffer first.

Currently the audit code requires that the full argument vector fits
in a single packet.  So currently it does clip the argv size to a
(sysctl) limit, but only when execve auditing is enabled.

If the audit protocol gets extended to allow for multiple packets this
check can be removed.

Upstream status:
commit: bdf4c48af20a3b0f01671799ace345e3d49576da
commit: 040b3a2df2dd26c3e401823f3b0ce3fe99e966c5 (some bugfixes)

diff --git a/fs/exec.c b/fs/exec.c
index 66a69d2..c76ed66 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1131,6 +1131,7 @@ int do_execve(char * filename,
 {
 	struct linux_binprm *bprm;
 	struct file *file;
+	unsigned long env_p;
 	int retval;
 	int i;
 
@@ -1185,9 +1186,11 @@ int do_execve(char * filename,
 	if (retval < 0)
 		goto out;
 
+	env_p = bprm->p;
 	retval = copy_strings(bprm->argc, argv, bprm);
 	if (retval < 0)
 		goto out;
+	bprm->argv_len = env_p - bprm->p;
 
 	retval = search_binary_handler(bprm,regs);
 	if (retval >= 0) {
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index f6f8753..ea9e87f 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -38,6 +38,9 @@ struct linux_binprm{
 	unsigned interp_flags;
 	unsigned interp_data;
 	unsigned long loader, exec;
+#ifndef __GENKSYMS__
+	unsigned long argv_len;
+#endif
 };
 
 #define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index f91b7d6..8996a7d 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -160,7 +160,7 @@ struct audit_aux_data_execve {
 	struct audit_aux_data	d;
 	int argc;
 	int envc;
-	char mem[0];
+	struct mm_struct *mm;
 };
 
 struct audit_aux_data_socketcall {
@@ -1004,30 +1004,70 @@ static int audit_log_single_execve_arg(struct audit_context *context,
 					struct audit_buffer **ab,
 					int arg_num,
 					size_t *len_sent,
-					const char *p)
+					const char __user *p,
+					char *buf)
 {
 	char arg_num_len_buf[12];
+	const char __user *tmp_p = p;
 	/* how many digits are in arg_num? 3 is the length of a=\n */
 	size_t arg_num_len = snprintf(arg_num_len_buf, 12, "%d", arg_num) + 3;
 	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 = strlen(p);
+	len_left = len = strnlen_user(p, MAX_ARG_STRLEN) - 1;
 
-	has_cntl = audit_string_contains_control(p, len);
-	if (has_cntl)
+	/*
+	 * 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 (unlikely((len == -1) || len > MAX_ARG_STRLEN - 1)) {
+		WARN_ON(1);
+		send_sig(SIGKILL, current, 0);
+		return -1;
+	}
+
+	/* walk the whole argument looking for non-ascii chars */
+	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);
 		/*
-		 * hex messages get logged as 2 bytes, so we can only
-		 * send half as much in each message
+		 * 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.
 		 */
-		max_execve_audit_len = MAX_EXECVE_AUDIT_LEN / 2;
+		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;
 
-	/* walk the argument actually logging the message */
+	/* rewalk the argument actually logging the message */
 	for (i = 0; len_left > 0; i++) {
 		int room_left;
 
@@ -1058,15 +1098,31 @@ static int audit_log_single_execve_arg(struct audit_context *context,
 			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;
+		}
+		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_hex(*ab, p, to_send);
+			audit_log_hex(*ab, buf, to_send);
 		else
-			audit_log_n_string(*ab, to_send, p);
+			audit_log_format(*ab, "\"%s\"", buf);
 		audit_log_format(*ab, "\n");
 
 		p += to_send;
@@ -1077,7 +1133,8 @@ static int audit_log_single_execve_arg(struct audit_context *context,
 		else
 			*len_sent += to_send;
 	}
-	return len;
+	/* include the null we didn't log */
+	return len + 1;
 }
 
 static void audit_log_execve_info(struct audit_context *context,
@@ -1086,20 +1143,38 @@ static void audit_log_execve_info(struct audit_context *context,
 {
 	int i;
 	size_t len, len_sent = 0;
-	const char *p;
+	const char __user *p;
+	char *buf;
 
-	p = axi->mem;
+	if (axi->mm != current->mm)
+		return; /* execve failed, no additional info */
+
+	p = (const char __user *)axi->mm->arg_start;
 
 	audit_log_format(*ab, "argc=%d ", axi->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\n");
+		return;
+	}
+
 	for (i = 0; i < axi->argc; i++) {
-		len = audit_log_single_execve_arg(context, ab, i, &len_sent, p);
+		len = audit_log_single_execve_arg(context, ab, i,
+						  &len_sent, p, buf);
 		if (len <= 0)
 			break;
-		/* skip the null */
-		p += len + 1;
+		p += len;
 	}
+	kfree(buf);
 }
+
 static void audit_log_exit(struct audit_context *context, struct task_struct *tsk)
 {
 	int i, call_panic = 0;
@@ -2194,28 +2269,17 @@ int audit_bprm(struct linux_binprm *bprm)
 {
 	struct audit_aux_data_execve *ax;
 	struct audit_context *context = current->audit_context;
-	unsigned long p, next;
-	void *to;
 
 	if (likely(!audit_enabled || !context || context->dummy))
 		return 0;
 
-	ax = kmalloc(sizeof(*ax) + PAGE_SIZE * MAX_ARG_PAGES - bprm->p,
-				GFP_KERNEL);
+	ax = kmalloc(sizeof(*ax), GFP_KERNEL);
 	if (!ax)
 		return -ENOMEM;
 
 	ax->argc = bprm->argc;
 	ax->envc = bprm->envc;
-	for (p = bprm->p, to = ax->mem; p < MAX_ARG_PAGES*PAGE_SIZE; p = next) {
-		struct page *page = bprm->page[p / PAGE_SIZE];
-		void *kaddr = kmap(page);
-		next = (p + PAGE_SIZE) & ~(PAGE_SIZE - 1);
-		memcpy(to, kaddr + (p & (PAGE_SIZE - 1)), next - p);
-		to += next - p;
-		kunmap(page);
-	}
-
+	ax->mm = bprm->mm;
 	ax->d.type = AUDIT_EXECVE;
 	ax->d.next = context->aux;
 	context->aux = (void *)ax;