Re: [PATCH] Prevent per,success and exit fields from disappearing in syscall audit log

[Steve Grubb <sgrubb@redhat.com>]

On Mon, 20 Jan 2014 11:09:45 -0500
Eric Paris <eparis@redhat.com> wrote:
> On Mon, 2014-01-20 at 10:51 -0500, Steve Grubb wrote:
> > On Mon, 20 Jan 2014 18:50:43 +0530
> > Paul Davies C <pauldaviesc@gmail.com> wrote:
> > 
> > > This patch prevents the per, success and the exit fields from
> > > disappearing from the audit system call log.
> 
> Wasn't you who said that fields cannot sometimes exist and sometimes
> not?

I don't want suddenly appearing fields in general.

>  It seems as though Paul is following your directives and fixing
> the present inconsistencies...

However, I believe these have been encoded into audit for so long that
software understands it. For example, adding personality to every
syscall record will do nothing but eat disk space. I hate eating disk
space. If we decided that wasting disk space was the right thing to do,
then personality is a hex integer and not a string.


> > (null) is an unrecognized value for all these fields. It would be
> > invalid for all currently written software that processes logs.
> 
> > Also, there is a reason why a check for return valid exists. Once
> > upon a time about 5-6 years ago, it was found that we can actually
> > get a syscall record where the return is not valid. You'd have to
> > search the archives to see what caused the patch to be written
> 
> I don't understand this comment?  He didn't remove the test...  What
> are you trying to convey?

There is a reason for the way that the code is. It wasn't just done for
the heck of it. We should see why it is the way it is before fixing it.
I would bet it has something to do with PPC, S390, or another record
pulling a syscall record in to emit an event before the syscall was
even finished. If we had to have something, I'd rather go with '?'
since it eats 1 byte instead of 6. But there was a problem and
a discussion that caused the code to be the way it is.

-Steve


> > > Signed-off-by: Paul Davies C <pauldaviesc@gmail.com>
> > > ---
> > >  kernel/auditsc.c |    4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index 9845cb3..3871466 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -1300,10 +1300,14 @@ static void audit_log_exit(struct
> > > audit_context *context, struct task_struct *ts context->arch,
> > > context->major); if (context->personality != PER_LINUX)
> > >  		audit_log_format(ab, " per=%lx",
> > > context->personality);
> > > +	else
> > > +		audit_log_format(ab, " per=(null)");
> > >  	if (context->return_valid)
> > >  		audit_log_format(ab, " success=%s exit=%ld",
> > >  				 (context->return_valid==AUDITSC_SUCCESS)?"yes":"no",
> > >  				 context->return_code);
> > > +	else
> > > +		audit_log_format(ab, " success=(null)
> > > exit=(null)"); 
> > >  	audit_log_format(ab,
> > >  			 " a0=%lx a1=%lx a2=%lx a3=%lx items=%d",
> > 
> 
>