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 <[email protected]>
---
 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
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to