From: Eric Paris <eparis@redhat.com>
To: Paul Davies C <pauldaviesc@gmail.com>
Cc: viro@zeniv.linux.org.uk, linux-audit@redhat.com
Subject: Re: [PATCH] Fixed reason field in audit signal logging
Date: Thu, 07 Nov 2013 11:05:52 -0500 [thread overview]
Message-ID: <1383840352.2938.49.camel@localhost> (raw)
In-Reply-To: <CAA80vrCbDgB9bQxmsPCnV1Wh1JbuuPayEZsW6jrCM8CN_W5xaw@mail.gmail.com>
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@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@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@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 prev parent reply other threads:[~2013-11-07 16:05 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-07 13:39 [PATCH] Fixed reason field in audit signal logging Paul Davies C
2013-11-07 14:43 ` Eric Paris
2013-11-07 14:52 ` LC Bruzenak
2013-11-07 15:05 ` Steve Grubb
2013-11-07 15:13 ` LC Bruzenak
2013-11-07 15:42 ` Eric Paris
2013-11-07 15:51 ` Paul Davies C
2013-11-07 15:53 ` Eric Paris
2013-11-07 16:00 ` Paul Davies C
2013-11-07 16:05 ` Eric Paris [this message]
2013-11-07 16:11 ` Steve Grubb
2013-11-07 18:07 ` Steve Grubb
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1383840352.2938.49.camel@localhost \
--to=eparis@redhat.com \
--cc=linux-audit@redhat.com \
--cc=pauldaviesc@gmail.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox