lkml.org 
[lkml]   [2007]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2007-06-06 01:43    [from the cache]
©2003-2011 Jasper Spaans