Re: [PATCH 2/4] audit: rework execve audit

On Tue, 05 Jun 2007 17:05:25 +0200 Peter Zijlstra <a.p.zijlstra(a)chello.nl&gt; wrote: > 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. > > ... > Please try to avoid trigger-happiness with the BUG_ON()s.. > struct audit_aux_data_socketcall { > @@ -834,6 +834,47 @@ static int audit_log_pid_context(struct > return rc; > } > > +static void audit_log_execve_info(struct audit_buffer *ab, > + struct audit_aux_data_execve *axi) > +{ > + int i; > + long len; > + const char __user *p = (const char __user *)axi->mm->arg_start; > + > + if (axi->mm != current->mm) > + return; /* execve failed, no additional info */ > + > + for (i = 0; i < axi->argc; i++, p += len) { > + long ret; > + char *tmp; > + > + len = strnlen_user(p, MAX_ARG_PAGES*PAGE_SIZE); > + /* > + * We just created this mm, if we can't find the strings > + * we just copied in something is _very_ wrong. > + */ > + BUG_ON(!len); > + > + tmp = kmalloc(len, GFP_KERNEL); > + if (!tmp) { > + audit_panic("out of memory for argv string\n"); > + break; > + } > + > + ret = copy_from_user(tmp, p, len); > + /* > + * There is no reason for this copy to be short. > + */ > + BUG_ON(ret); You sure? What happens if another thread does munmap() in parallel? I think I'll make this WARN_ON just out of principle.

> @@ -1208,9 +1209,11 @@ int do_execve(char * filename, > if (retval < 0) > goto out; > > + tmp = bprm->p; > retval = copy_strings(bprm->argc, argv, bprm); > if (retval < 0) > goto out; > + bprm->argv_len = tmp - bprm->p; --- a/include/linux/kernel.h~a +++ a/include/linux/kernel.h @@ -5,6 +5,8 @@ * 'kernel.h' contains some often-used function prototypes etc */ +#define tmp don't call your variables tmp! + #ifdef __KERNEL__ #include <stdarg.h>