On Tuesday, July 28, 2015 10:31:54 AM Steve Grubb wrote: > On Friday, July 24, 2015 06:54:27 PM Paul Moore wrote: > > On Thursday, July 23, 2015 04:45:10 PM Steve Grubb wrote: > > > The audit subsystem could use a function that logs the commonly needed > > > fields for a typical audit event. This logs less that > > > audit_log_task_info > > > and reduces the need to hand code individual fields. > > > > > > Signed-off-by: Steve Grubb <sgr...@redhat.com> > > > --- > > > > > > include/linux/audit.h | 5 +++++ > > > kernel/audit.c | 35 +++++++++++++++++++++++++++++++++++ > > > 2 files changed, 40 insertions(+) > > > > Additional comments below, but I'd like to see this patch change > > audit_log_task_info() to call audit_log_task_simple() > > They really can't without messing up parsers. The order is different for a > reason. The audit_log_task_info records all kinds of stuff that is really > not needed. It does pids, current credentials, extended uid, extended gid, > and then tty and session, comm, exe, and then context. This wastes disk > space.
If we can't use _task_simple() inside of _task_info() then just use audit_log_task_info(). Yes, it probably wastes a few extra bytes each time these records are generated, but these records aren't likely to be frequent. > The new function is what should be used for most cases because it sticks to > what is necessary for "hardwired" events - those that are not dictated by > syscall or file watches. It provides pid, uid, auid, tty, session, context, > comm, exe. Because it jettisons all the stuff that doesn't matter, one > cannot call the other. Where can we use _task_simple() beyond these new records? Show me this has some reuse in the existing code base and I'll reconsider keeping _task_simple(), but right now it just looks like code duplication to me. > > ... or, why not just call audit_log_task_info() if the audit bind/unbind > > is going to be the only one to benefit from audit_log_task_simple()? Yes, > > I know that audit_log_task_info() records more than you need, but this > > duplication of code because of the record format mess makes me very > > grumpy. > > I'd rather see us move some other things to audit_log_task_simple over the > long term than hand code things. See above; we're not going to hand code things, just use _task_info(). Long term we are going to be ditching this awful fixed string format. > > > diff --git a/kernel/audit.c b/kernel/audit.c > > > index 1c13e42..29fb38b 100644 > > > --- a/kernel/audit.c > > > +++ b/kernel/audit.c > > > @@ -1100,6 +1100,41 @@ static void audit_receive(struct sk_buff *skb) > > > > > > mutex_unlock(&audit_cmd_mutex); > > > > > > } > > > > > > +/* This function logs the essential information needed to understand > > > + * what or who is causing the event */ > > > +void audit_log_task_simple(struct audit_buffer *ab, struct task_struct > > > *tsk) > > > > ... > > > > > + audit_log_format(ab, "pid=%u uid=%u auid=%u tty=%s ses=%u", > > > + task_pid_nr(tsk), > > > + from_kuid(&init_user_ns, cred->uid), > > > + from_kuid(&init_user_ns, audit_get_loginuid(tsk)), > > > + tty, audit_get_sessionid(tsk)); > > > > You should check the format string against audit_log_task_info(); they > > don't match. > > That is correct. It mostly matches the order of just about everything else. > For example, user space originating events get this: I was talking about some of the scalar format specifiers, e.g. "%u" vs "%d", but it doesn't matter so much anymore as it looks like we'll need to use _task_info(). -- paul moore security @ redhat -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
