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;