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