linux-kernel - Re: audit: EXECVE record

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1231514485.25714.11.camel@localhost.localdomain>
Date:	Fri, 09 Jan 2009 10:21:25 -0500
From:	Eric Paris <eparis@...hat.com>
To:	Jiri Pirko <jpirko@...hat.com>
Cc:	linux-kernel@...r.kernel.org, linux-audit@...hat.com,
	viro@...iv.linux.org.uk, Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: audit: EXECVE record - removed bogus newline

On Fri, 2009-01-09 at 13:21 +0100, Jiri Pirko wrote:
> On Thu, 08 Jan 2009 10:33:18 -0500
> Eric Paris <eparis@...hat.com> wrote:
> 
> > 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....)
> I think that calculation is correct only the comment needs to be changed like this:
> -       /* how many digits are in arg_num? 3 is the length of a=\n */
> +       /* how many digits are in arg_num? 3 is the length of " a=" */
> 
> the count is still 3 - with '\n' or with ' '.

You're right.  Sorry, I'd like to see the comment fixed.  That 3 does
matter and I don't want to see someone not realize how important getting
that len right is....

> > 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? 
> Is this a problem? We can do some workaround but is it necessary?

I'll go ahead and vote I don't care, even though I brought it up.  My
code left an extra space at the end of the record.  Yours puts it at the
beginning.  From the point of view of auditd the spaces don't mean
anything, so I'm ok with your method.  I guess it saves a call to
audit_log_format, and saving a call is benefit enough

Don't care if it's this window or the next, it's been like this for 3
weeks short of a year now  :)

Acked-by: Eric Paris <eparis@...hat.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/