* [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp
@ 2014-01-14 2:56 Eric Paris
2014-01-14 14:48 ` Steve Grubb
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Eric Paris @ 2014-01-14 2:56 UTC (permalink / raw)
To: sgrubb, pauldaviesc, keescook; +Cc: linux-audit
We have a helper function which writes out all of the interesting
identity information about tasks, audit_log_task_info(). We then have a
second helper, audit_log_task(), which is only used by audit_core_dumps()
and __audit_seccomp(). It is a light weight and only outputs some of the
information about the task. There does not appear to be rational for
its existence except audit_core_dumps() originally did it this way. At
the time audit_log_task_info() did not exist. When __audit_seccomp came
along audit_core_dumps() was split into this helper and reused. But
there was a better helper in audit.c.
This does reorder the records for audit_core_dumps() and
__audit_seccomp(). The new record order is below. The number in () is
the order in the old record. Entries without a () do not exist in the
old record.
audit_log_task_info:
ppid pid (6) auid (1) uid (2) gid (3) euid
suid fsuid egid sgid fsgid tty
ses (4) comm (7) exe (8) subj (5)
audit_log_task:
auid uid gid ses subj pid comm exe
It seems that reusing the task info pattern throughout records should
allow for faster simpler more streamlined userspace records parsing, but
changing order like this might be a deal breaker.
Signed-off-by: Eric Paris <eparis@redhat.com>
---
kernel/auditsc.c | 32 ++------------------------------
1 file changed, 2 insertions(+), 30 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 62500fe..9434e3b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2352,34 +2352,6 @@ void __audit_mmap_fd(int fd, int flags)
context->type = AUDIT_MMAP;
}
-static void audit_log_task(struct audit_buffer *ab)
-{
- kuid_t auid, uid;
- kgid_t gid;
- unsigned int sessionid;
- struct mm_struct *mm = current->mm;
-
- auid = audit_get_loginuid(current);
- sessionid = audit_get_sessionid(current);
- current_uid_gid(&uid, &gid);
-
- audit_log_format(ab, "auid=%u uid=%u gid=%u ses=%u",
- from_kuid(&init_user_ns, auid),
- from_kuid(&init_user_ns, uid),
- from_kgid(&init_user_ns, gid),
- sessionid);
- audit_log_task_context(ab);
- audit_log_format(ab, " pid=%d comm=", current->pid);
- audit_log_untrustedstring(ab, current->comm);
- if (mm) {
- down_read(&mm->mmap_sem);
- if (mm->exe_file)
- audit_log_d_path(ab, " exe=", &mm->exe_file->f_path);
- up_read(&mm->mmap_sem);
- } else
- audit_log_format(ab, " exe=(null)");
-}
-
/**
* audit_core_dumps - record information about processes that end abnormally
* @signr: signal value
@@ -2400,7 +2372,7 @@ void audit_core_dumps(long signr)
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_ANOM_ABEND);
if (unlikely(!ab))
return;
- audit_log_task(ab);
+ audit_log_task_info(ab, current);
audit_log_format(ab, " sig=%ld", signr);
audit_log_end(ab);
}
@@ -2412,7 +2384,7 @@ void __audit_seccomp(unsigned long syscall, long signr, int code)
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_SECCOMP);
if (unlikely(!ab))
return;
- audit_log_task(ab);
+ audit_log_task_info(ab, current);
audit_log_format(ab, " sig=%ld", signr);
audit_log_format(ab, " syscall=%ld", syscall);
audit_log_format(ab, " compat=%d", is_compat_task());
--
1.8.4.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp
2014-01-14 2:56 [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp Eric Paris
@ 2014-01-14 14:48 ` Steve Grubb
2014-01-14 19:07 ` Richard Guy Briggs
2014-01-14 18:39 ` Kees Cook
2014-01-15 17:20 ` Richard Guy Briggs
2 siblings, 1 reply; 6+ messages in thread
From: Steve Grubb @ 2014-01-14 14:48 UTC (permalink / raw)
To: Eric Paris; +Cc: linux-audit
On Monday, January 13, 2014 09:56:35 PM Eric Paris wrote:
> It seems that reusing the task info pattern throughout records should
> allow for faster simpler more streamlined userspace records parsing, but
> changing order like this might be a deal breaker.
Have you tried using the ausearch test suite? I published it so that it can be
found out what all these patches will do to the stability of user space. I'd
delete your logs, reboot into test kernel, generate as many kind of events as
possible, then extract the logs and test with the test suite.
-Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp
2014-01-14 2:56 [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp Eric Paris
2014-01-14 14:48 ` Steve Grubb
@ 2014-01-14 18:39 ` Kees Cook
2014-01-15 17:20 ` Richard Guy Briggs
2 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2014-01-14 18:39 UTC (permalink / raw)
To: Eric Paris; +Cc: linux-audit
On Mon, Jan 13, 2014 at 6:56 PM, Eric Paris <eparis@redhat.com> wrote:
> We have a helper function which writes out all of the interesting
> identity information about tasks, audit_log_task_info(). We then have a
> second helper, audit_log_task(), which is only used by audit_core_dumps()
> and __audit_seccomp(). It is a light weight and only outputs some of the
> information about the task. There does not appear to be rational for
> its existence except audit_core_dumps() originally did it this way. At
> the time audit_log_task_info() did not exist. When __audit_seccomp came
> along audit_core_dumps() was split into this helper and reused. But
> there was a better helper in audit.c.
>
> This does reorder the records for audit_core_dumps() and
> __audit_seccomp(). The new record order is below. The number in () is
> the order in the old record. Entries without a () do not exist in the
> old record.
>
> audit_log_task_info:
> ppid pid (6) auid (1) uid (2) gid (3) euid
> suid fsuid egid sgid fsgid tty
> ses (4) comm (7) exe (8) subj (5)
>
> audit_log_task:
> auid uid gid ses subj pid comm exe
>
> It seems that reusing the task info pattern throughout records should
> allow for faster simpler more streamlined userspace records parsing, but
> changing order like this might be a deal breaker.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
Sounds fine to me. Thanks!
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp
2014-01-14 14:48 ` Steve Grubb
@ 2014-01-14 19:07 ` Richard Guy Briggs
2014-01-14 19:09 ` Steve Grubb
0 siblings, 1 reply; 6+ messages in thread
From: Richard Guy Briggs @ 2014-01-14 19:07 UTC (permalink / raw)
To: Steve Grubb; +Cc: linux-audit
On 14/01/14, Steve Grubb wrote:
> On Monday, January 13, 2014 09:56:35 PM Eric Paris wrote:
> > It seems that reusing the task info pattern throughout records should
> > allow for faster simpler more streamlined userspace records parsing, but
> > changing order like this might be a deal breaker.
>
> Have you tried using the ausearch test suite? I published it so that it can be
> found out what all these patches will do to the stability of user space. I'd
> delete your logs, reboot into test kernel, generate as many kind of events as
> possible, then extract the logs and test with the test suite.
Do you have a script of rules and a script of commands to accomplish the
"generate as many kind of events as possible"?
> -Steve
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp
2014-01-14 19:07 ` Richard Guy Briggs
@ 2014-01-14 19:09 ` Steve Grubb
0 siblings, 0 replies; 6+ messages in thread
From: Steve Grubb @ 2014-01-14 19:09 UTC (permalink / raw)
To: Richard Guy Briggs; +Cc: linux-audit
On Tuesday, January 14, 2014 02:07:26 PM Richard Guy Briggs wrote:
> On 14/01/14, Steve Grubb wrote:
> > On Monday, January 13, 2014 09:56:35 PM Eric Paris wrote:
> > > It seems that reusing the task info pattern throughout records should
> > > allow for faster simpler more streamlined userspace records parsing, but
> > > changing order like this might be a deal breaker.
> >
> > Have you tried using the ausearch test suite? I published it so that it
> > can be found out what all these patches will do to the stability of user
> > space. I'd delete your logs, reboot into test kernel, generate as many
> > kind of events as possible, then extract the logs and test with the test
> > suite.
>
> Do you have a script of rules and a script of commands to accomplish the
> "generate as many kind of events as possible"?
Nope. But its very important to make sure all events are well formed and
searchable by existing tools.
-Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp
2014-01-14 2:56 [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp Eric Paris
2014-01-14 14:48 ` Steve Grubb
2014-01-14 18:39 ` Kees Cook
@ 2014-01-15 17:20 ` Richard Guy Briggs
2 siblings, 0 replies; 6+ messages in thread
From: Richard Guy Briggs @ 2014-01-15 17:20 UTC (permalink / raw)
To: Eric Paris; +Cc: linux-audit
On 14/01/13, Eric Paris wrote:
> We have a helper function which writes out all of the interesting
> identity information about tasks, audit_log_task_info(). We then have a
> second helper, audit_log_task(), which is only used by audit_core_dumps()
> and __audit_seccomp(). It is a light weight and only outputs some of the
> information about the task. There does not appear to be rational for
> its existence except audit_core_dumps() originally did it this way. At
> the time audit_log_task_info() did not exist. When __audit_seccomp came
> along audit_core_dumps() was split into this helper and reused. But
> there was a better helper in audit.c.
>
> This does reorder the records for audit_core_dumps() and
> __audit_seccomp(). The new record order is below. The number in () is
> the order in the old record. Entries without a () do not exist in the
> old record.
>
> audit_log_task_info:
> ppid pid (6) auid (1) uid (2) gid (3) euid
> suid fsuid egid sgid fsgid tty
> ses (4) comm (7) exe (8) subj (5)
>
> audit_log_task:
> auid uid gid ses subj pid comm exe
>
> It seems that reusing the task info pattern throughout records should
> allow for faster simpler more streamlined userspace records parsing, but
> changing order like this might be a deal breaker.
>
> Signed-off-by: Eric Paris <eparis@redhat.com>
I would be very happy to see this consolidation. Eric, thanks for doing
the itemization above to quantify our previous discussion.
> ---
> kernel/auditsc.c | 32 ++------------------------------
> 1 file changed, 2 insertions(+), 30 deletions(-)
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 62500fe..9434e3b 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2352,34 +2352,6 @@ void __audit_mmap_fd(int fd, int flags)
> context->type = AUDIT_MMAP;
> }
>
> -static void audit_log_task(struct audit_buffer *ab)
> -{
> - kuid_t auid, uid;
> - kgid_t gid;
> - unsigned int sessionid;
> - struct mm_struct *mm = current->mm;
> -
> - auid = audit_get_loginuid(current);
> - sessionid = audit_get_sessionid(current);
> - current_uid_gid(&uid, &gid);
> -
> - audit_log_format(ab, "auid=%u uid=%u gid=%u ses=%u",
> - from_kuid(&init_user_ns, auid),
> - from_kuid(&init_user_ns, uid),
> - from_kgid(&init_user_ns, gid),
> - sessionid);
> - audit_log_task_context(ab);
> - audit_log_format(ab, " pid=%d comm=", current->pid);
> - audit_log_untrustedstring(ab, current->comm);
> - if (mm) {
> - down_read(&mm->mmap_sem);
> - if (mm->exe_file)
> - audit_log_d_path(ab, " exe=", &mm->exe_file->f_path);
> - up_read(&mm->mmap_sem);
> - } else
> - audit_log_format(ab, " exe=(null)");
> -}
> -
> /**
> * audit_core_dumps - record information about processes that end abnormally
> * @signr: signal value
> @@ -2400,7 +2372,7 @@ void audit_core_dumps(long signr)
> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_ANOM_ABEND);
> if (unlikely(!ab))
> return;
> - audit_log_task(ab);
> + audit_log_task_info(ab, current);
> audit_log_format(ab, " sig=%ld", signr);
> audit_log_end(ab);
> }
> @@ -2412,7 +2384,7 @@ void __audit_seccomp(unsigned long syscall, long signr, int code)
> ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_SECCOMP);
> if (unlikely(!ab))
> return;
> - audit_log_task(ab);
> + audit_log_task_info(ab, current);
> audit_log_format(ab, " sig=%ld", signr);
> audit_log_format(ab, " syscall=%ld", syscall);
> audit_log_format(ab, " compat=%d", is_compat_task());
> --
> 1.8.4.2
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-01-15 17:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-14 2:56 [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp Eric Paris
2014-01-14 14:48 ` Steve Grubb
2014-01-14 19:07 ` Richard Guy Briggs
2014-01-14 19:09 ` Steve Grubb
2014-01-14 18:39 ` Kees Cook
2014-01-15 17:20 ` Richard Guy Briggs
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox