[PATCH] Fixed reason field in audit signal logging

[Paul Davies C]

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 at 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 at 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 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/linux-audit/attachments/20131107/db8a1f79/attachment.htm>