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

--- 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); }

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&gt; But we really should wait for an Ack and thoughts from steve grubb....

> --- > > 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",

> + 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

> 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); > > }

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&gt; > > 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.

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); > > > > }

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" .

Your thoughts? On Thu, Nov 7, 2013 at 9:12 PM, Eric Paris <eparis(a)redhat.com&gt; 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&gt; > > > > 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

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&gt; 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&gt; 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&gt; > > > > > > 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