<p dir="ltr">The race was non existent. I had the VMA locked. I switched to this to keep the code that gets the cmdline value almost unchanged to try and reduce bugs. I can still author a patch on top of this later to optimize. However the buffer is smaller. Before it was page size, now its path max....iirc is smaller.</p>

<div class="gmail_quote">On Jan 14, 2014 5:45 PM, "Richard Guy Briggs" <<a href="mailto:rgb@redhat.com">rgb@redhat.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 14/01/06, William Roberts wrote:<br>
> During an audit event, cache and print the value of the process's<br>
> cmdline value (proc/<pid>/cmdline). This is useful in situations<br>
> where processes are started via fork'd virtual machines where the<br>
> comm field is incorrect. Often times, setting the comm field still<br>
> is insufficient as the comm width is not very wide and most<br>
> virtual machine "package names" do not fit. Also, during execution,<br>
> many threads have their comm field set as well. By tying it back to<br>
> the global cmdline value for the process, audit records will be more<br>
> complete in systems with these properties. An example of where this<br>
> is useful and applicable is in the realm of Android. With Android,<br>
> their is no fork/exec for VM instances. The bare, preloaded Dalvik<br>
> VM listens for a fork and specialize request. When this request comes<br>
> in, the VM forks, and the loads the specific application (specializing).<br>
> This was done to take advantage of COW and to not require a load of<br>
> basic packages by the VM on very app spawn. When this spawn occurs,<br>
> the package name is set via setproctitle() and shows up in procfs.<br>
> Many of these package names are longer then 16 bytes, the historical<br>
> width of task->comm. Having the cmdline in the audit records will<br>
> couple the application back to the record directly. Also, on my<br>
> Debian development box, some audit records were more useful then<br>
> what was printed under comm.<br>
<br>
So...  What happenned to allocating only what you need instead of the<br>
full 4k buffer?  Your test results showed promise with only 64 or 128<br>
bytes allocated.  I recall seeing some discussion about a race between<br>
testing for the size needed and actually filling the buffer, but was<br>
hoping that would be worked on rather than reverting back to the full<br>
4k.<br>
<br>
> The cached cmdline is tied to the life-cycle of the audit_context<br>
> structure and is built on demand.<br>
><br>
> Example denial prior to patch (Ubuntu):<br>
> CALL msg=audit(1387828084.070:361): arch=c000003e syscall=82 success=yes exit=0 a0=4184bf a1=418547 a2=0 a3=0 items=0 ppid=1 pid=1329 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 ses=4294967295 tty=(none) comm="console-kit-dae" exe="/usr/sbin/console-kit-daemon" subj=system_u:system_r:consolekit_t:s0-s0:c0.c255 key=(null)<br>

><br>
> After Patches (Ubuntu):<br>
> type=SYSCALL msg=audit(1387828084.070:361): arch=c000003e syscall=82 success=yes exit=0 a0=4184bf a1=418547 a2=0 a3=0 items=0 ppid=1 pid=1329 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 ses=4294967295 tty=(none) comm="console-kit-dae" exe="/usr/sbin/console-kit-daemon" subj=system_u:system_r:consolekit_t:s0-s0:c0.c255 cmdline="/usr/lib/dbus-1.0/dbus-daemon-launch-helper" key=(null)<br>

><br>
> Example denial prior to patch (Android):<br>
> type=1300 msg=audit(248323.940:247): arch=40000028 syscall=54 per=840000 success=yes exit=0 a0=39 a1=540b a2=2 a3=750eecec items=0 ppid=224 pid=1858 auid=4294967295 uid=1002 gid=1002 euid=1002 suid=1002 fsuid=1002 egid=1002 sgid=1002 fsgid=1002 tty=(none) ses=4294967295 comm="bt_hc_worker" exe="/system/bin/app_process" subj=u:r:bluetooth:s0 key=(null)<br>

><br>
> After Patches (Android):<br>
> type=1300 msg=audit(248323.940:247): arch=40000028 syscall=54 per=840000 success=yes exit=0 a0=39 a1=540b a2=2 a3=750eecec items=0 ppid=224 pid=1858 auid=4294967295 uid=1002 gid=1002 euid=1002 suid=1002 fsuid=1002 egid=1002 sgid=1002 fsgid=1002 tty=(none) ses=4294967295 comm="bt_hc_worker" exe="/system/bin/app_process" cmdline="com.android.bluetooth" subj=u:r:bluetooth:s0 key=(null)<br>

><br>
> Signed-off-by: William Roberts <<a href="mailto:wroberts@tresys.com">wroberts@tresys.com</a>><br>
> ---<br>
>  kernel/audit.h   |    1 +<br>
>  kernel/auditsc.c |   32 ++++++++++++++++++++++++++++++++<br>
>  2 files changed, 33 insertions(+)<br>
><br>
> diff --git a/kernel/audit.h b/kernel/audit.h<br>
> index b779642..bd6211f 100644<br>
> --- a/kernel/audit.h<br>
> +++ b/kernel/audit.h<br>
> @@ -202,6 +202,7 @@ struct audit_context {<br>
>               } execve;<br>
>       };<br>
>       int fds[2];<br>
> +     char *cmdline;<br>
><br>
>  #if AUDIT_DEBUG<br>
>       int                 put_count;<br>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c<br>
> index 90594c9..a4c2003 100644<br>
> --- a/kernel/auditsc.c<br>
> +++ b/kernel/auditsc.c<br>
> @@ -842,6 +842,12 @@ static inline struct audit_context *audit_get_context(struct task_struct *tsk,<br>
>       return context;<br>
>  }<br>
><br>
> +static inline void audit_cmdline_free(struct audit_context *context)<br>
> +{<br>
> +     kfree(context->cmdline);<br>
> +     context->cmdline = NULL;<br>
> +}<br>
> +<br>
>  static inline void audit_free_names(struct audit_context *context)<br>
>  {<br>
>       struct audit_names *n, *next;<br>
> @@ -955,6 +961,7 @@ static inline void audit_free_context(struct audit_context *context)<br>
>       audit_free_aux(context);<br>
>       kfree(context->filterkey);<br>
>       kfree(context->sockaddr);<br>
> +     audit_cmdline_free(context);<br>
>       kfree(context);<br>
>  }<br>
><br>
> @@ -1271,6 +1278,30 @@ static void show_special(struct audit_context *context, int *call_panic)<br>
>       audit_log_end(ab);<br>
>  }<br>
><br>
> +static void audit_log_cmdline(struct audit_buffer *ab, struct task_struct *tsk,<br>
> +                      struct audit_context *context)<br>
> +{<br>
> +     int res;<br>
> +     char *buf;<br>
> +     char *msg = "(null)";<br>
> +     audit_log_format(ab, " cmdline=");<br>
> +<br>
> +     /* Not  cached */<br>
> +     if (!context->cmdline) {<br>
> +             buf = kmalloc(PATH_MAX, GFP_KERNEL);<br>
> +             if (!buf)<br>
> +                     goto out;<br>
> +             res = get_cmdline(tsk, buf, PATH_MAX);<br>
> +             /* Ensure NULL terminated */<br>
> +             if (buf[res-1] != '\0')<br>
> +                     buf[res-1] = '\0';<br>
> +             context->cmdline = buf;<br>
> +     }<br>
> +     msg = context->cmdline;<br>
> +out:<br>
> +     audit_log_untrustedstring(ab, msg);<br>
> +}<br>
> +<br>
>  static void audit_log_exit(struct audit_context *context, struct task_struct *tsk)<br>
>  {<br>
>       int i, call_panic = 0;<br>
> @@ -1302,6 +1333,7 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts<br>
>                        context->name_count);<br>
><br>
>       audit_log_task_info(ab, tsk);<br>
> +     audit_log_cmdline(ab, tsk, context);<br>
>       audit_log_key(ab, context->filterkey);<br>
>       audit_log_end(ab);<br>
><br>
> --<br>
> 1.7.9.5<br>
><br>
> --<br>
> Linux-audit mailing list<br>
> <a href="mailto:Linux-audit@redhat.com">Linux-audit@redhat.com</a><br>
> <a href="https://www.redhat.com/mailman/listinfo/linux-audit" target="_blank">https://www.redhat.com/mailman/listinfo/linux-audit</a><br>
<br>
- RGB<br>
<br>
--<br>
Richard Guy Briggs <<a href="mailto:rbriggs@redhat.com">rbriggs@redhat.com</a>><br>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat<br>
Remote, Ottawa, Canada<br>
Voice: <a href="tel:%2B1.647.777.2635" value="+16477772635">+1.647.777.2635</a>, Internal: (81) 32635, Alt: <a href="tel:%2B1.613.693.0684x3545" value="+16136930684">+1.613.693.0684x3545</a><br>
</blockquote></div>