Messages in this thread Patch in this message | | | Date | Tue, 5 Jun 2007 16:39:15 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 2/4] audit: rework execve audit |
| |
On Tue, 05 Jun 2007 17:05:25 +0200 Peter Zijlstra <a.p.zijlstra@chello.nl> 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.
> + audit_log_format(ab, "a%d=", i); > + audit_log_untrustedstring(ab, tmp); > + audit_log_format(ab, "\n"); > + > + kfree(tmp); > + } > +} > + > > ... > > =================================================================== > --- linux-2.6-2.orig/fs/exec.c 2007-06-05 09:51:42.000000000 +0200 > +++ linux-2.6-2/fs/exec.c 2007-06-05 10:03:11.000000000 +0200 > @@ -1154,6 +1154,7 @@ int do_execve(char * filename, > { > struct linux_binprm *bprm; > struct file *file; > + unsigned long tmp; > int retval; > int i; > > @@ -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> _
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |