On 2020-02-27 20:02, Paul Moore wrote:
> On Tue, Feb 18, 2020 at 4:01 PM Richard Guy Briggs <rgb(a)redhat.com> wrote:
> >
> > When there are no audit rules registered, mandatory records (config,
> > etc.) are missing their accompanying records (syscall, proctitle, etc.).
> >
> > This is due to audit context dummy set on syscall entry based on absence
> > of rules that signals that no other records are to be printed.
> >
> > Clear the dummy bit in auditsc_set_stamp() when the first record of an
> > event is generated.
> >
> > Please see upstream github issue
> >
https://github.com/linux-audit/audit-kernel/issues/120
> >
> > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> > ---
> > kernel/auditsc.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 4effe01ebbe2..31195d122344 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -2176,6 +2176,8 @@ int auditsc_get_stamp(struct audit_context *ctx,
> > t->tv_sec = ctx->ctime.tv_sec;
> > t->tv_nsec = ctx->ctime.tv_nsec;
> > *serial = ctx->serial;
> > + if (ctx->dummy)
> > + ctx->dummy = 0;
>
> Two comments:
>
> * Why even bother checking to see if ctx->dummy is true? If it is
> true you set it to false/0; if it is already false you leave it alone.
> Either way ctx->dummy is going to be set to false when you are past
> these two lines, might as well just always set ctx->dummy to false/0.
Ok, no problem.
> * Why are you setting ->dummy to false in auditsc_get_stamp() and not
> someplace a bit more obvious like audit_log_start()? Is it because
> auditsc_get_stamp() only gets called once per event? I'm willing to
> take the "hit" of one extra assignment in audit_log_start() to keep
> this in a more obvious place and not buried in auditsc_get_stamp().
It is because the context is only available when syscall logging is
enabled (which is on most platforms and hopefully eventually all) and
makes for cleaner code and lack of need to check existance of the
context.
At the very least let's create some sort of accessor function for
dummy then, hiding this in auditsc_get_stamp() seems very wrong to me.
--
paul moore