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

[Steve Grubb <sgrubb@redhat.com>]

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.

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);
> >  
> >  }