Re: [PATCH 1/2] audit: log binding and unbinding to netlink multicast socket

[Paul Moore <pmoore@redhat.com>]

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 <sgrubb@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() ... 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.

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

-- 
paul moore
security @ redhat