Re: [PATCH] audit: use audit_log_task_info in audit_core_dumps and __audit_seccomp

[Richard Guy Briggs <rgb@redhat.com>]

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