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

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(a)redhat.com&gt; > > > > 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); > > > > > > } >