Re: audit: EXECVE record - removed bogus newline

Eric Paris
2009-01-08T15:33:18Z

On Thu, 2009-01-08 at 15:38 +0100, Jiri Pirko wrote:
> EXECVE records contain a newline after every argument. auditd converts
> "\n" to " " so you cannot see newlines even in raw logs, but they're
> there nevertheless. If you're not using auditd, you need to work round
> them. These '\n' chars are can be easily replaced by spaces when
> creating record in kernel. Note there is no need for trailing '\n' in
> an audit record.

While I completely agree the \n was my mistake and should be
dropped/fixed can you fix one more thing and look at another? First
arg_num_len is being miscalculated since I included the \n in that
calculation (might be the only place....) and I remember not wanting to
follow convention and put the space at the beginning of the aX= for some
reason. If you add thousands of arguments so this is larger than
MAX_EXECVE_AUDIT_LEN do you end up with an extra space somewhere in the
second EXECVE record? Or maybe it was a single argument that was larger
than the max, can't remember which, but I do remember having a random
extra space (maybe we'd rather have that for consistency?)

-Eric

>
> record before this patch:
> "type=EXECVE msg=audit(1231421801.566:31): argc=4 a0=\"./test\"\na1=\"a\"\na2=\"b\"\na3=\"c\"\n"
>
> record after this patch:
> "type=EXECVE msg=audit(1231421801.566:31): argc=4 a0=\"./test\" a1=\"a\" a2=\"b\" a3=\"c\""
>
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> kernel/auditsc.c | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 8cbddff..c7012e0 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1109,7 +1109,7 @@ static int audit_log_single_execve_arg(struct audit_context *context,
> * so we can be sure nothing was lost.
> */
> if ((i == 0) && (too_long))
> - audit_log_format(*ab, "a%d_len=%zu ", arg_num,
> + audit_log_format(*ab, " a%d_len=%zu", arg_num,
> has_cntl ? 2*len : len);
>
> /*
> @@ -1129,7 +1129,7 @@ static int audit_log_single_execve_arg(struct audit_context *context,
> buf[to_send] = '\0';
>
> /* actually log it */
> - audit_log_format(*ab, "a%d", arg_num);
> + audit_log_format(*ab, " a%d", arg_num);
> if (too_long)
> audit_log_format(*ab, "[%d]", i);
> audit_log_format(*ab, "=");
> @@ -1137,7 +1137,6 @@ static int audit_log_single_execve_arg(struct audit_context *context,
> audit_log_n_hex(*ab, buf, to_send);
> else
> audit_log_format(*ab, "\"%s\"", buf);
> - audit_log_format(*ab, "\n");
>
> p += to_send;
> len_left -= to_send;
> @@ -1165,7 +1164,7 @@ static void audit_log_execve_info(struct audit_context *context,
>
> p = (const char __user *)axi->mm->arg_start;
>
> - audit_log_format(*ab, "argc=%d ", axi->argc);
> + audit_log_format(*ab, "argc=%d", axi->argc);
>
> /*
> * we need some kernel buffer to hold the userspace args. Just