Re: [PATCH] Fixed reason field in audit signal logging

[Eric Paris <eparis@redhat.com>]

On Thu, 2013-11-07 at 10:05 -0500, Steve Grubb wrote:
> On Thursday, November 07, 2013 09:43:24 AM Eric Paris wrote:
> > On Thu, 2013-11-07 at 19:09 +0530, Paul Davies C wrote:
> > > The audit system logs the signals that leads to abnormal end of a process.
> > > However , as of now , it always states the reason for failure of a process
> > > as "memory violation" regardless of the signal delivered. This is due to
> > > the audit_core_dumps() function pass the reason for failure blindly to
> > > the audit_log_abend() as "memory violation".
> > > 
> > > This patch changes the audit_core_dumps() function as to pass on the right
> > > reason to the audit_log_abend based on the signal received.
> > > 
> > > Signed-off-by:Paul Davies C
> > 
> > Acked-by: Eric Paris <eparis@redhat.com>
> > 
> > But we really should wait for an Ack and thoughts from steve grubb....
> 
> I am confused. This is the abnormal end event I have:
> 
> type=ANOM_ABEND msg=audit(1303339663.307:142): auid=4325 uid=0 gid=0 ses=1 
> subj=unconfined_u:unconfined_r:unconfined_t:s0 pid=3775 comm="aureport" sig=11
> 
> Why / when did we start adding text explanations? We should not do that. We 
> didn't have it before and it should not have been added. The signal number is 
> enough to identify the problem.

We started adding a reason when seccomp started sending ANOM_ABEND
events as well.  It doesn't do so with a signal.  Agreed, the " " is/was
a bad idea...

> 
> If we did need a reason= field, all these strings with spaces will get 
> separated on parsing. They should be like "memory-violation" or "recieved-
> abort". And would it be better to hide this in the audit_log_abend function? I 
> honestly don't understand why this was added.
> 
> -Steve
> 
> 
> > > ---
> > > 
> > >  kernel/auditsc.c |   31 ++++++++++++++++++++++++++++++-
> > >  1 file changed, 30 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index 9845cb3..3cafd13 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -2395,7 +2395,36 @@ void audit_core_dumps(long signr)
> > > 
> > >  	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_ANOM_ABEND);
> > >  	if (unlikely(!ab))
> > >  	
> > >  		return;
> > > 
> > > -	audit_log_abend(ab, "memory violation", signr);
> > > +
> > > +	/*Identify the reason for failure based on signal delivered.*/
> > > +	switch (signr) {
> > > +	case SIGABRT:
> > > +			audit_log_abend(ab, "received abort", signr);
> > > +			break;
> > > +	case SIGBUS:
> > > +			audit_log_abend(ab, "invalid pointer dereference", signr);
> > > +			break;
> > > +	case SIGFPE:
> > > +			audit_log_abend(ab, "invalid floating point instruction", 
> signr);
> > > +			break;
> > > +	case SIGILL:
> > > +			audit_log_abend(ab, "illegal instruction", signr);
> > > +			break;
> > > +	case SIGSEGV:
> > > +			audit_log_abend(ab, "memory violation", signr);
> > > +			break;
> > > +	case SIGTRAP:
> > > +			audit_log_abend(ab, "bad instruction / debugger generated 
> signal",
> > > signr); +			break;
> > > +	case SIGXCPU:
> > > +			audit_log_abend(ab, "cpu time violation", signr);
> > > +			break;
> > > +	case SIGXFSZ:
> > > +			audit_log_abend(ab, "file size violation", signr);
> > > +			break;
> > > +	default:
> > > +			audit_log_abend(ab, "not defined", signr);
> > > +	}
> > > 
> > >  	audit_log_end(ab);
> > >  
> > >  }
>