I don't suggest dropping the reason field entirely. I thought your patch to add signal information in the reason field was a good idea, but steve pointed out, the signal information is in the sig= field. So now I do not believe we need such fine grained 'reasons'
That brought me to the question is "memory violation" the right reason when really it was a 'signal' which caused such a record... Maybe a patch which just did s/memory violation/signal/ would be a good idea... On Thu, 2013-11-07 at 21:30 +0530, Paul Davies C wrote: > So we must drop the "reason" field for abnormal end due to signal > delivery. > > > On Thu, Nov 7, 2013 at 9:23 PM, Eric Paris <[email protected]> wrote: > On Thu, 2013-11-07 at 21:21 +0530, Paul Davies C wrote: > > So rather than logging the "reason=memory violation" when > process ends > > abnormally due to any signal delivery , it will be be better > if we > > leave "reason=undefined" . > > > reason=memory_violation or invalid_pointer etc > > although maybe it should be just 'signal'... and you can get > the signal > number from the record.... > > > Your thoughts? > > > > > > On Thu, Nov 7, 2013 at 9:12 PM, Eric Paris > <[email protected]> wrote: > > 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 <[email protected]> > > > > > > > > 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); > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > -- > > Regards, > > Paul Davies C > > vivafoss.blogspot.com > > > > > > > > -- > Regards, > Paul Davies C > vivafoss.blogspot.com -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
