[PATCH ghak99 v1] audit: print empty EXECVE args

[Paul Moore]

On Tue, Nov 6, 2018 at 3:23 PM Richard Guy Briggs <rgb at redhat.com> wrote:
> On 2018-11-05 17:05, Paul Moore wrote:
> > On Wed, Oct 10, 2018 at 4:24 PM Richard Guy Briggs <rgb at redhat.com> wrote:
> > > Empty executable arguments were being skipped when printing out the list
> > > of arguments in an EXECVE record, making it appear they were somehow
> > > lost.  Include empty arguments as an itemized empty string.
> > >
> > > Reproducer:
> > >         autrace /bin/ls "" "/etc"
> > >         ausearch --start recent -m execve -i | grep EXECVE
> > >         type=EXECVE msg=audit(10/03/2018 13:04:03.208:1391) : argc=3 a0=/bin/ls a2=/etc
> > >
> > > With fix:
> > >         type=EXECVE msg=audit(10/03/2018 21:51:38.290:194) : argc=3 a0=/bin/ls a1= a2=/etc
> > >         type=EXECVE msg=audit(1538617898.290:194): argc=3 a0="/bin/ls" a1="" a2="/etc"
> > >
> > > Passes audit-testsuite
> > > Based on: v4.19-rc2 (audit/next)
> > > See: https://github.com/linux-audit/audit-kernel/issues/99
> > > Signed-off-by: Richard Guy Briggs <rgb at redhat.com>
> >
> > Merged into audit/next, but I did some cleanup on your metadata and I
> > want you to limit yourself to the more conventional metadata in the
> > future (e.g. Signed-off-by, Fixes, etc.).
> >
> > The "Based on" information doesn't belong as metadata.  In fact I
> > would suggest that you shouldn't need to explicitly state the tree
> > your patch(set) is based on, it should be based on either the current
> > audit/next tree at the time of your posting (preferable) or Linus
> > master tree.  If you feel that you must provide the base of your
> > patch(set), either due to a wide cross-posting or some patch(set)
> > specific complexities, please do so in a cover letter.
> >
> > I'm less upset about the GH issue reference as metadata, but since
> > we're talking about these things, I'd prefer if it was included in the
> > main patch description instead of metadata.  Also a reminder that
> > linking the GH issue doesn't remove the need for you to adequately
> > describe the patch in the commit message.  The git log needs to
> > standalone as a useful source of information.  This particular patch
> > does a good job of that; this is just a reminder for others who are
> > following the mailing list.
>
> Ok, thanks, sorry for the noise.
>
> Would simply separating the metadata from the rest of the patch by a
> blank line be sufficient?

No.  Please don't do that either.

> I didn't really consider "Based on" to be
> metadata.  I understand about the lack of need for "Based on".  Is there
> a better label for "See:" similar to "Reported-by:" such as
> "Issue-tracker:"?  Similarly, "Reproducer:" I don't consider metadata.

I consider "XXXX: YYYY" anywhere in the commit description to be metadata.

-- 
paul moore
www.paul-moore.com