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    [W:4.154 / U:0.460 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site