Re: [RFC PATCH] audit: minimize our use of audit_log_format()

WARNING: completely untested patch! There are several cases where we are using audit_log_format() when we could be using audit_log_string(), which should be quicker. There are also some cases where we are making multiple audit_log_format() calls in a row, for no apparent reason. This patch fixes the problems above in the core audit code, the other kernel subsystems are left for another time. Signed-off-by: Paul Moore <paul(a)paul-moore.com&gt;

--- kernel/audit.c | 37 ++++++++++++++++++------------------- kernel/audit_fsnotify.c | 2 +- kernel/audit_tree.c | 3 +-- kernel/audit_watch.c | 2 +- kernel/auditsc.c | 19 +++++++++---------- 5 files changed, 30 insertions(+), 33 deletions(-) diff --git a/kernel/audit.c b/kernel/audit.c index 160144f7e5f9..a0f57f4f9944 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -1347,7 +1347,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) else { int size; - audit_log_format(ab, " data="); + audit_log_string(ab, " data="); size = nlmsg_len(nlh); if (size > 0 && ((unsigned char *)data)[size - 1] == '\0') @@ -1375,7 +1375,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) case AUDIT_TRIM: audit_trim_trees(); audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE); - audit_log_format(ab, " op=trim res=1"); + audit_log_string(ab, " op=trim res=1"); audit_log_end(ab); break; case AUDIT_MAKE_EQUIV: { @@ -1406,9 +1406,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE); - audit_log_format(ab, " op=make_equiv old="); + audit_log_string(ab, " op=make_equiv old="); audit_log_untrustedstring(ab, old); - audit_log_format(ab, " new="); + audit_log_string(ab, " new="); audit_log_untrustedstring(ab, new); audit_log_format(ab, " res=%d", !err); audit_log_end(ab); @@ -2021,7 +2021,7 @@ void audit_log_d_path(struct audit_buffer *ab, const char *prefix, char *p, *pathname; if (prefix) - audit_log_format(ab, "%s", prefix); + audit_log_string(ab, prefix); /* We will allow 11 spaces for ' (deleted)' to be appended */ pathname = kmalloc(PATH_MAX+11, ab->gfp_mask); @@ -2048,11 +2048,11 @@ void audit_log_session_info(struct audit_buffer *ab) void audit_log_key(struct audit_buffer *ab, char *key) { - audit_log_format(ab, " key="); + audit_log_string(ab, " key="); if (key) audit_log_untrustedstring(ab, key); else - audit_log_format(ab, "(null)"); + audit_log_string(ab, "(null)"); } void audit_log_cap(struct audit_buffer *ab, char *prefix, kernel_cap_t *cap) @@ -2134,7 +2134,7 @@ void audit_log_name(struct audit_context *context, struct audit_names *n, switch (n->name_len) { case AUDIT_NAME_FULL: /* log the full path */ - audit_log_format(ab, " name="); + audit_log_string(ab, " name="); audit_log_untrustedstring(ab, n->name->name); break; case 0: @@ -2144,12 +2144,12 @@ void audit_log_name(struct audit_context *context, struct audit_names *n, break; default: /* log the name's directory component */ - audit_log_format(ab, " name="); + audit_log_string(ab, " name="); audit_log_n_untrustedstring(ab, n->name->name, n->name_len); } } else - audit_log_format(ab, " name=(null)"); + audit_log_string(ab, " name=(null)"); if (n->ino != AUDIT_INO_UNSET) audit_log_format(ab, " inode=%lu" @@ -2178,22 +2178,21 @@ void audit_log_name(struct audit_context *context, struct audit_names *n, } /* log the audit_names record type */ - audit_log_format(ab, " nametype="); switch(n->type) { case AUDIT_TYPE_NORMAL: - audit_log_format(ab, "NORMAL"); + audit_log_string(ab, "nametype=NORMAL"); break; case AUDIT_TYPE_PARENT: - audit_log_format(ab, "PARENT"); + audit_log_string(ab, "nametype=PARENT"); break; case AUDIT_TYPE_CHILD_DELETE: - audit_log_format(ab, "DELETE"); + audit_log_string(ab, "nametype=DELETE"); break; case AUDIT_TYPE_CHILD_CREATE: - audit_log_format(ab, "CREATE"); + audit_log_string(ab, "nametype=CREATE"); break; default: - audit_log_format(ab, "UNKNOWN"); + audit_log_string(ab, "nametype=UNKNOWN"); break; } @@ -2245,7 +2244,7 @@ void audit_log_d_path_exe(struct audit_buffer *ab, fput(exe_file); return; out_null: - audit_log_format(ab, " exe=(null)"); + audit_log_string(ab, " exe=(null)"); } struct tty_struct *audit_get_tty(void) @@ -2294,7 +2293,7 @@ void audit_log_task_info(struct audit_buffer *ab) tty ? tty_name(tty) : "(none)", audit_get_sessionid(current)); audit_put_tty(tty); - audit_log_format(ab, " comm="); + audit_log_string(ab, " comm="); audit_log_untrustedstring(ab, get_task_comm(comm, current)); audit_log_d_path_exe(ab, current->mm); audit_log_task_context(ab); @@ -2318,7 +2317,7 @@ void audit_log_link_denied(const char *operation) return; audit_log_format(ab, "op=%s", operation); audit_log_task_info(ab); - audit_log_format(ab, " res=0"); + audit_log_string(ab, " res=0"); audit_log_end(ab); } diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c index fba78047fb37..27d29103333c 100644 --- a/kernel/audit_fsnotify.c +++ b/kernel/audit_fsnotify.c @@ -133,7 +133,7 @@ static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, c audit_log_format(ab, "auid=%u ses=%u op=%s", from_kuid(&init_user_ns, audit_get_loginuid(current)), audit_get_sessionid(current), op); - audit_log_format(ab, " path="); + audit_log_string(ab, " path="); audit_log_untrustedstring(ab, audit_mark->path); audit_log_key(ab, rule->filterkey); audit_log_format(ab, " list=%d res=1", rule->listnr); diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c index 9f6eaeb6919f..f01bce6d1b23 100644 --- a/kernel/audit_tree.c +++ b/kernel/audit_tree.c @@ -502,8 +502,7 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule) ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE); if (unlikely(!ab)) return; - audit_log_format(ab, "op=remove_rule"); - audit_log_format(ab, " dir="); + audit_log_string(ab, "op=remove_rule dir="); audit_log_untrustedstring(ab, rule->tree->pathname); audit_log_key(ab, rule->filterkey); audit_log_format(ab, " list=%d res=1", rule->listnr); diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c index 787c7afdf829..0ce85fe25a53 100644 --- a/kernel/audit_watch.c +++ b/kernel/audit_watch.c @@ -248,7 +248,7 @@ static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watc audit_log_format(ab, "auid=%u ses=%u op=%s", from_kuid(&init_user_ns, audit_get_loginuid(current)), audit_get_sessionid(current), op); - audit_log_format(ab, " path="); + audit_log_string(ab, " path="); audit_log_untrustedstring(ab, w->path); audit_log_key(ab, r->filterkey); audit_log_format(ab, " list=%d res=1", r->listnr); diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 8b12e525306e..f370930265ea 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -954,14 +954,14 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid, from_kuid(&init_user_ns, uid), sessionid); if (sid) { if (security_secid_to_secctx(sid, &ctx, &len)) { - audit_log_format(ab, " obj=(none)"); + audit_log_string(ab, " obj=(none)"); rc = 1; } else { audit_log_format(ab, " obj=%s", ctx); security_release_secctx(ctx, len); } } - audit_log_format(ab, " ocomm="); + audit_log_string(ab, " ocomm="); audit_log_untrustedstring(ab, comm); audit_log_end(ab); @@ -1104,7 +1104,7 @@ static void audit_log_execve_info(struct audit_context *context, abuf[sizeof(abuf) - 1] = '\0'; /* log the arg in the audit record */ - audit_log_format(*ab, "%s", abuf); + audit_log_string(*ab, abuf); len_rem -= len_tmp; len_tmp = len_buf; if (encode) { @@ -1240,7 +1240,7 @@ static void show_special(struct audit_context *context, int *call_panic) audit_log_execve_info(context, &ab); break; case AUDIT_KERN_MODULE: - audit_log_format(ab, "name="); + audit_log_string(ab, "name="); audit_log_untrustedstring(ab, context->module.name); kfree(context->module.name); break; @@ -1276,7 +1276,7 @@ static void audit_log_proctitle(void) if (!ab) return; /* audit_panic or being filtered */ - audit_log_format(ab, "proctitle="); + audit_log_string(ab, "proctitle="); /* Not cached */ if (!context->proctitle.value) { @@ -1405,7 +1405,7 @@ static void audit_log_exit(int ret_valid, long ret_code) if (context->sockaddr_len) { ab = audit_log_start(context, GFP_KERNEL, AUDIT_SOCKADDR); if (ab) { - audit_log_format(ab, "saddr="); + audit_log_string(ab, "saddr="); audit_log_n_hex(ab, (void *)context->sockaddr, context->sockaddr_len); audit_log_end(ab); @@ -2498,10 +2498,9 @@ void audit_seccomp_actions_logged(const char *names, const char *old_names, if (unlikely(!ab)) return; - audit_log_format(ab, "op=seccomp-logging"); - audit_log_format(ab, " actions=%s", names); - audit_log_format(ab, " old-actions=%s", old_names); - audit_log_format(ab, " res=%d", res); + audit_log_string(ab, "op=seccomp-logging"); + audit_log_format(ab, " actions=%s old-actions=%s res=%d", + names, old_names, res); audit_log_end(ab); } -- Linux-audit mailing list Linux-audit(a)redhat.com https://www.redhat.com/mailman/listinfo/linux-audit