9:53 a.m.
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 <eparis(a)redhat.com> 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 <eparis(a)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);
> > >
> > > }
>
--
Regards,
Paul Davies C
vivafoss.blogspot.com