[PATCH] audit: use current whenever possible

[Paul Moore]

There are many places, notably audit_log_task_info() and
audit_log_exit(), that take task_struct pointers but in reality they
are always working on the current task. This patch eliminates the
task_struct arguments and uses current directly which allows a number
of cleanups as well.

Signed-off-by: Paul Moore <***@paul-moore.com>
---
drivers/tty/tty_audit.c | 13 ++--
include/linux/audit.h | 6 +-
kernel/audit.c | 34 +++++-----
kernel/audit.h | 2 -
kernel/auditsc.c | 131 ++++++++++++++++++--------------------
security/integrity/ima/ima_api.c | 2 -
6 files changed, 90 insertions(+), 98 deletions(-)

diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 50f567b6a66e..28f87fd6a28e 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -61,20 +61,19 @@ static void tty_audit_log(const char *description, dev_t dev,
unsigned char *data, size_t size)
{
struct audit_buffer *ab;
- struct task_struct *tsk = current;
- pid_t pid = task_pid_nr(tsk);
- uid_t uid = from_kuid(&init_user_ns, task_uid(tsk));
- uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(tsk));
- unsigned int sessionid = audit_get_sessionid(tsk);
+ pid_t pid = task_pid_nr(current);
+ uid_t uid = from_kuid(&init_user_ns, task_uid(current));
+ uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(current));
+ unsigned int sessionid = audit_get_sessionid(current);

ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_TTY);
if (ab) {
- char name[sizeof(tsk->comm)];
+ char name[sizeof(current->comm)];

audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d"
" minor=%d comm=", description, pid, uid,
loginuid, sessionid, MAJOR(dev), MINOR(dev));
- get_task_comm(name, tsk);
+ get_task_comm(name, current);
audit_log_untrustedstring(ab, name);
audit_log_format(ab, " data=");
audit_log_n_hex(ab, data, size);
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 58cf665f597e..a625c29a2ea2 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -151,8 +151,7 @@ extern void audit_log_link_denied(const char *operation);
extern void audit_log_lost(const char *message);

extern int audit_log_task_context(struct audit_buffer *ab);
-extern void audit_log_task_info(struct audit_buffer *ab,
- struct task_struct *tsk);
+extern void audit_log_task_info(struct audit_buffer *ab);

extern int audit_update_lsm_rules(void);

@@ -200,8 +199,7 @@ static inline int audit_log_task_context(struct audit_buffer *ab)
{
return 0;
}
-static inline void audit_log_task_info(struct audit_buffer *ab,
- struct task_struct *tsk)
+static inline void audit_log_task_info(struct audit_buffer *ab)
{ }
#define audit_enabled AUDIT_OFF
#endif /* CONFIG_AUDIT */
diff --git a/kernel/audit.c b/kernel/audit.c
index d09298d3c2d2..779671883349 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1096,10 +1096,11 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature

if (audit_enabled == AUDIT_OFF)
return;
+
ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FEATURE_CHANGE);
if (!ab)
return;
- audit_log_task_info(ab, current);
+ audit_log_task_info(ab);
audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d",
audit_feature_names[which], !!old_feature, !!new_feature,
!!old_lock, !!new_lock, res);
@@ -2246,15 +2247,15 @@ void audit_log_d_path_exe(struct audit_buffer *ab,
audit_log_format(ab, " exe=(null)");
}

-struct tty_struct *audit_get_tty(struct task_struct *tsk)
+struct tty_struct *audit_get_tty(void)
{
struct tty_struct *tty = NULL;
unsigned long flags;

- spin_lock_irqsave(&tsk->sighand->siglock, flags);
- if (tsk->signal)
- tty = tty_kref_get(tsk->signal->tty);
- spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+ spin_lock_irqsave(&current->sighand->siglock, flags);
+ if (current->signal)
+ tty = tty_kref_get(current->signal->tty);
+ spin_unlock_irqrestore(&current->sighand->siglock, flags);
return tty;
}

@@ -2263,25 +2264,24 @@ void audit_put_tty(struct tty_struct *tty)
tty_kref_put(tty);
}

-void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
+void audit_log_task_info(struct audit_buffer *ab)
{
const struct cred *cred;
- char comm[sizeof(tsk->comm)];
+ char comm[sizeof(current->comm)];
struct tty_struct *tty;

if (!ab)
return;

- /* tsk == current */
cred = current_cred();
- tty = audit_get_tty(tsk);
+ tty = audit_get_tty();
audit_log_format(ab,
" ppid=%d pid=%d auid=%u uid=%u gid=%u"
" euid=%u suid=%u fsuid=%u"
" egid=%u sgid=%u fsgid=%u tty=%s ses=%u",
- task_ppid_nr(tsk),
- task_tgid_nr(tsk),
- from_kuid(&init_user_ns, audit_get_loginuid(tsk)),
+ task_ppid_nr(current),
+ task_tgid_nr(current),
+ from_kuid(&init_user_ns, audit_get_loginuid(current)),
from_kuid(&init_user_ns, cred->uid),
from_kgid(&init_user_ns, cred->gid),
from_kuid(&init_user_ns, cred->euid),
@@ -2291,11 +2291,11 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
from_kgid(&init_user_ns, cred->sgid),
from_kgid(&init_user_ns, cred->fsgid),
tty ? tty_name(tty) : "(none)",
- audit_get_sessionid(tsk));
+ audit_get_sessionid(current));
audit_put_tty(tty);
audit_log_format(ab, " comm=");
- audit_log_untrustedstring(ab, get_task_comm(comm, tsk));
- audit_log_d_path_exe(ab, tsk->mm);
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ audit_log_d_path_exe(ab, current->mm);
audit_log_task_context(ab);
}
EXPORT_SYMBOL(audit_log_task_info);
@@ -2316,7 +2316,7 @@ void audit_log_link_denied(const char *operation)
if (!ab)
return;
audit_log_format(ab, "op=%s", operation);
- audit_log_task_info(ab, current);
+ audit_log_task_info(ab);
audit_log_format(ab, " res=0");
audit_log_end(ab);
}
diff --git a/kernel/audit.h b/kernel/audit.h
index 0b5295aeaebb..91421679a168 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -264,7 +264,7 @@ extern struct audit_entry *audit_dupe_rule(struct audit_krule *old);
extern void audit_log_d_path_exe(struct audit_buffer *ab,
struct mm_struct *mm);

-extern struct tty_struct *audit_get_tty(struct task_struct *tsk);
+extern struct tty_struct *audit_get_tty(void);
extern void audit_put_tty(struct tty_struct *tty);

/* audit watch functions */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 51e735aedf58..6593a5207fb0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -830,44 +830,6 @@ void audit_filter_inodes(struct task_struct *tsk, struct audit_context *ctx)
rcu_read_unlock();
}

-/* Transfer the audit context pointer to the caller, clearing it in the tsk's struct */
-static inline struct audit_context *audit_take_context(struct task_struct *tsk,
- int return_valid,
- long return_code)
-{
- struct audit_context *context = tsk->audit_context;
-
- if (!context)
- return NULL;
- context->return_valid = return_valid;
-
- /*
- * we need to fix up the return code in the audit logs if the actual
- * return codes are later going to be fixed up by the arch specific
- * signal handlers
- *
- * This is actually a test for:
- * (rc == ERESTARTSYS ) || (rc == ERESTARTNOINTR) ||
- * (rc == ERESTARTNOHAND) || (rc == ERESTART_RESTARTBLOCK)
- *
- * but is faster than a bunch of ||
- */
- if (unlikely(return_code <= -ERESTARTSYS) &&
- (return_code >= -ERESTART_RESTARTBLOCK) &&
- (return_code != -ENOIOCTLCMD))
- context->return_code = -EINTR;
- else
- context->return_code = return_code;
-
- if (context->in_syscall && !context->dummy) {
- audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_EXIT]);
- audit_filter_inodes(tsk, context);
- }
-
- audit_set_context(tsk, NULL);
- return context;
-}
-
static inline void audit_proctitle_free(struct audit_context *context)
{
kfree(context->proctitle.value);
@@ -1296,15 +1258,18 @@ static inline int audit_proctitle_rtrim(char *proctitle, int len)
return len;
}

-static void audit_log_proctitle(struct task_struct *tsk,
- struct audit_context *context)
+static void audit_log_proctitle(void)
{
int res;
char *buf;
char *msg = "(null)";
int len = strlen(msg);
+ struct audit_context *context = audit_context();
struct audit_buffer *ab;

+ if (!context || context->dummy)
+ return;
+
ab = audit_log_start(context, GFP_KERNEL, AUDIT_PROCTITLE);
if (!ab)
return; /* audit_panic or being filtered */
@@ -1317,7 +1282,7 @@ static void audit_log_proctitle(struct task_struct *tsk,
if (!buf)
goto out;
/* Historically called this from procfs naming */
- res = get_cmdline(tsk, buf, MAX_PROCTITLE_AUDIT_LEN);
+ res = get_cmdline(current, buf, MAX_PROCTITLE_AUDIT_LEN);
if (res == 0) {
kfree(buf);
goto out;
@@ -1337,15 +1302,15 @@ static void audit_log_proctitle(struct task_struct *tsk,
audit_log_end(ab);
}

-static void audit_log_exit(struct audit_context *context, struct task_struct *tsk)
+static void audit_log_exit(void)
{
int i, call_panic = 0;
+ struct audit_context *context = audit_context();
struct audit_buffer *ab;
struct audit_aux_data *aux;
struct audit_names *n;

- /* tsk == current */
- context->personality = tsk->personality;
+ context->personality = current->personality;

ab = audit_log_start(context, GFP_KERNEL, AUDIT_SYSCALL);
if (!ab)
@@ -1367,7 +1332,7 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
context->argv[3],
context->name_count);

- audit_log_task_info(ab, tsk);
+ audit_log_task_info(ab);
audit_log_key(ab, context->filterkey);
audit_log_end(ab);

@@ -1456,7 +1421,7 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
audit_log_name(context, n, NULL, i++, &call_panic);
}

- audit_log_proctitle(tsk, context);
+ audit_log_proctitle();

/* Send end of event record to help user space know we are finished */
ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
@@ -1474,22 +1439,31 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
*/
void __audit_free(struct task_struct *tsk)
{
- struct audit_context *context;
+ struct audit_context *context = tsk->audit_context;

- context = audit_take_context(tsk, 0, 0);
if (!context)
return;

- /* Check for system calls that do not go through the exit
- * function (e.g., exit_group), then free context block.
- * We use GFP_ATOMIC here because we might be doing this
- * in the context of the idle thread */
- /* that can happen only if we are called from do_exit() */
- if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
- audit_log_exit(context, tsk);
+ /* We are called either by do_exit() or the fork() error handling code;
+ * in the former case tsk == current and in the latter tsk is a
+ * random task_struct that doesn't doesn't have any meaningful data we
+ * need to log via audit_log_exit().
+ */
+ if (tsk == current && !context->dummy && context->in_syscall) {
+ context->return_valid = 0;
+ context->return_code = 0;
+
+ audit_filter_syscall(tsk, context,
+ &audit_filter_list[AUDIT_FILTER_EXIT]);
+ audit_filter_inodes(tsk, context);
+ if (context->current_state == AUDIT_RECORD_CONTEXT)
+ audit_log_exit();
+ }
+
if (!list_empty(&context->killed_trees))
audit_kill_trees(&context->killed_trees);

+ audit_set_context(tsk, NULL);
audit_free_context(context);
}

@@ -1559,17 +1533,40 @@ void __audit_syscall_exit(int success, long return_code)
{
struct audit_context *context;

- if (success)
- success = AUDITSC_SUCCESS;
- else
- success = AUDITSC_FAILURE;
-
- context = audit_take_context(current, success, return_code);
+ context = audit_context();
if (!context)
return;

- if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
- audit_log_exit(context, current);
+ if (!context->dummy && context->in_syscall) {
+ if (success)
+ context->return_valid = AUDITSC_SUCCESS;
+ else
+ context->return_valid = AUDITSC_FAILURE;
+
+ /*
+ * we need to fix up the return code in the audit logs if the
+ * actual return codes are later going to be fixed up by the
+ * arch specific signal handlers
+ *
+ * This is actually a test for:
+ * (rc == ERESTARTSYS ) || (rc == ERESTARTNOINTR) ||
+ * (rc == ERESTARTNOHAND) || (rc == ERESTART_RESTARTBLOCK)
+ *
+ * but is faster than a bunch of ||
+ */
+ if (unlikely(return_code <= -ERESTARTSYS) &&
+ (return_code >= -ERESTART_RESTARTBLOCK) &&
+ (return_code != -ENOIOCTLCMD))
+ context->return_code = -EINTR;
+ else
+ context->return_code = return_code;
+
+ audit_filter_syscall(current, context,
+ &audit_filter_list[AUDIT_FILTER_EXIT]);
+ audit_filter_inodes(current, context);
+ if (context->current_state == AUDIT_RECORD_CONTEXT)
+ audit_log_exit();
+ }

context->in_syscall = 0;
context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
@@ -1591,7 +1588,6 @@ void __audit_syscall_exit(int success, long return_code)
kfree(context->filterkey);
context->filterkey = NULL;
}
- audit_set_context(current, context);
}

static inline void handle_one(const struct inode *inode)
@@ -2025,7 +2021,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
uid = from_kuid(&init_user_ns, task_uid(current));
oldloginuid = from_kuid(&init_user_ns, koldloginuid);
loginuid = from_kuid(&init_user_ns, kloginuid),
- tty = audit_get_tty(current);
+ tty = audit_get_tty();

audit_log_format(ab, "pid=%d uid=%u", task_tgid_nr(current), uid);
audit_log_task_context(ab);
@@ -2046,7 +2042,6 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
*/
int audit_set_loginuid(kuid_t loginuid)
{
- struct task_struct *task = current;
unsigned int oldsessionid, sessionid = AUDIT_SID_UNSET;
kuid_t oldloginuid;
int rc;
@@ -2065,8 +2060,8 @@ int audit_set_loginuid(kuid_t loginuid)
sessionid = (unsigned int)atomic_inc_return(&session_id);
}

- task->sessionid = sessionid;
- task->loginuid = loginuid;
+ current->sessionid = sessionid;
+ current->loginuid = loginuid;
out:
audit_log_set_loginuid(oldloginuid, loginuid, oldsessionid, sessionid, rc);
return rc;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 99dd1d53fc35..af134588ab4e 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -336,7 +336,7 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
audit_log_untrustedstring(ab, filename);
audit_log_format(ab, " hash=\"%s:%s\"", algo_name, hash);

- audit_log_task_info(ab, current);
+ audit_log_task_info(ab);
audit_log_end(ab);

iint->flags |= IMA_AUDITED;

[Richard Guy Briggs]

Post by Paul Moore
There are many places, notably audit_log_task_info() and
audit_log_exit(), that take task_struct pointers but in reality they
are always working on the current task. This patch eliminates the
task_struct arguments and uses current directly which allows a number
of cleanups as well.

Ack. This looks like a welcome simplification. This will cause a merge
conflict with my ghak59 patch 4&5 (but Jan's patch didn't) which is
fine. I'd like to rebase ghak59 on this once this is merged to get the
EOE record in the right place after kill_trees...

One minor comment in __audit_free() below once we've established tsk ==
current...

Post by Paul Moore
---
drivers/tty/tty_audit.c | 13 ++--
include/linux/audit.h | 6 +-
kernel/audit.c | 34 +++++-----
kernel/audit.h | 2 -
kernel/auditsc.c | 131 ++++++++++++++++++--------------------
security/integrity/ima/ima_api.c | 2 -
6 files changed, 90 insertions(+), 98 deletions(-)
diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c
index 50f567b6a66e..28f87fd6a28e 100644
--- a/drivers/tty/tty_audit.c
+++ b/drivers/tty/tty_audit.c
@@ -61,20 +61,19 @@ static void tty_audit_log(const char *description, dev_t dev,
unsigned char *data, size_t size)
{
struct audit_buffer *ab;
- struct task_struct *tsk = current;
- pid_t pid = task_pid_nr(tsk);
- uid_t uid = from_kuid(&init_user_ns, task_uid(tsk));
- uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(tsk));
- unsigned int sessionid = audit_get_sessionid(tsk);
+ pid_t pid = task_pid_nr(current);
+ uid_t uid = from_kuid(&init_user_ns, task_uid(current));
+ uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(current));
+ unsigned int sessionid = audit_get_sessionid(current);
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_TTY);
if (ab) {
- char name[sizeof(tsk->comm)];
+ char name[sizeof(current->comm)];
audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d"
" minor=%d comm=", description, pid, uid,
loginuid, sessionid, MAJOR(dev), MINOR(dev));
- get_task_comm(name, tsk);
+ get_task_comm(name, current);
audit_log_untrustedstring(ab, name);
audit_log_format(ab, " data=");
audit_log_n_hex(ab, data, size);
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 58cf665f597e..a625c29a2ea2 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -151,8 +151,7 @@ extern void audit_log_link_denied(const char *operation);
extern void audit_log_lost(const char *message);
extern int audit_log_task_context(struct audit_buffer *ab);
-extern void audit_log_task_info(struct audit_buffer *ab,
- struct task_struct *tsk);
+extern void audit_log_task_info(struct audit_buffer *ab);
extern int audit_update_lsm_rules(void);
@@ -200,8 +199,7 @@ static inline int audit_log_task_context(struct audit_buffer *ab)
{
return 0;
}
-static inline void audit_log_task_info(struct audit_buffer *ab,
- struct task_struct *tsk)
+static inline void audit_log_task_info(struct audit_buffer *ab)
{ }
#define audit_enabled AUDIT_OFF
#endif /* CONFIG_AUDIT */
diff --git a/kernel/audit.c b/kernel/audit.c
index d09298d3c2d2..779671883349 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1096,10 +1096,11 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature
if (audit_enabled == AUDIT_OFF)
return;
+
ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FEATURE_CHANGE);
if (!ab)
return;
- audit_log_task_info(ab, current);
+ audit_log_task_info(ab);
audit_log_format(ab, " feature=%s old=%u new=%u old_lock=%u new_lock=%u res=%d",
audit_feature_names[which], !!old_feature, !!new_feature,
!!old_lock, !!new_lock, res);
@@ -2246,15 +2247,15 @@ void audit_log_d_path_exe(struct audit_buffer *ab,
audit_log_format(ab, " exe=(null)");
}
-struct tty_struct *audit_get_tty(struct task_struct *tsk)
+struct tty_struct *audit_get_tty(void)
{
struct tty_struct *tty = NULL;
unsigned long flags;
- spin_lock_irqsave(&tsk->sighand->siglock, flags);
- if (tsk->signal)
- tty = tty_kref_get(tsk->signal->tty);
- spin_unlock_irqrestore(&tsk->sighand->siglock, flags);
+ spin_lock_irqsave(&current->sighand->siglock, flags);
+ if (current->signal)
+ tty = tty_kref_get(current->signal->tty);
+ spin_unlock_irqrestore(&current->sighand->siglock, flags);
return tty;
}
@@ -2263,25 +2264,24 @@ void audit_put_tty(struct tty_struct *tty)
tty_kref_put(tty);
}
-void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
+void audit_log_task_info(struct audit_buffer *ab)
{
const struct cred *cred;
- char comm[sizeof(tsk->comm)];
+ char comm[sizeof(current->comm)];
struct tty_struct *tty;
if (!ab)
return;
- /* tsk == current */
cred = current_cred();
- tty = audit_get_tty(tsk);
+ tty = audit_get_tty();
audit_log_format(ab,
" ppid=%d pid=%d auid=%u uid=%u gid=%u"
" euid=%u suid=%u fsuid=%u"
" egid=%u sgid=%u fsgid=%u tty=%s ses=%u",
- task_ppid_nr(tsk),
- task_tgid_nr(tsk),
- from_kuid(&init_user_ns, audit_get_loginuid(tsk)),
+ task_ppid_nr(current),
+ task_tgid_nr(current),
+ from_kuid(&init_user_ns, audit_get_loginuid(current)),
from_kuid(&init_user_ns, cred->uid),
from_kgid(&init_user_ns, cred->gid),
from_kuid(&init_user_ns, cred->euid),
@@ -2291,11 +2291,11 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
from_kgid(&init_user_ns, cred->sgid),
from_kgid(&init_user_ns, cred->fsgid),
tty ? tty_name(tty) : "(none)",
- audit_get_sessionid(tsk));
+ audit_get_sessionid(current));
audit_put_tty(tty);
audit_log_format(ab, " comm=");
- audit_log_untrustedstring(ab, get_task_comm(comm, tsk));
- audit_log_d_path_exe(ab, tsk->mm);
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ audit_log_d_path_exe(ab, current->mm);
audit_log_task_context(ab);
}
EXPORT_SYMBOL(audit_log_task_info);
@@ -2316,7 +2316,7 @@ void audit_log_link_denied(const char *operation)
if (!ab)
return;
audit_log_format(ab, "op=%s", operation);
- audit_log_task_info(ab, current);
+ audit_log_task_info(ab);
audit_log_format(ab, " res=0");
audit_log_end(ab);
}
diff --git a/kernel/audit.h b/kernel/audit.h
index 0b5295aeaebb..91421679a168 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -264,7 +264,7 @@ extern struct audit_entry *audit_dupe_rule(struct audit_krule *old);
extern void audit_log_d_path_exe(struct audit_buffer *ab,
struct mm_struct *mm);
-extern struct tty_struct *audit_get_tty(struct task_struct *tsk);
+extern struct tty_struct *audit_get_tty(void);
extern void audit_put_tty(struct tty_struct *tty);
/* audit watch functions */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 51e735aedf58..6593a5207fb0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -830,44 +830,6 @@ void audit_filter_inodes(struct task_struct *tsk, struct audit_context *ctx)
rcu_read_unlock();
}
-/* Transfer the audit context pointer to the caller, clearing it in the tsk's struct */
-static inline struct audit_context *audit_take_context(struct task_struct *tsk,
- int return_valid,
- long return_code)
-{
- struct audit_context *context = tsk->audit_context;
-
- if (!context)
- return NULL;
- context->return_valid = return_valid;
-
- /*
- * we need to fix up the return code in the audit logs if the actual
- * return codes are later going to be fixed up by the arch specific
- * signal handlers
- *
- * (rc == ERESTARTSYS ) || (rc == ERESTARTNOINTR) ||
- * (rc == ERESTARTNOHAND) || (rc == ERESTART_RESTARTBLOCK)
- *
- * but is faster than a bunch of ||
- */
- if (unlikely(return_code <= -ERESTARTSYS) &&
- (return_code >= -ERESTART_RESTARTBLOCK) &&
- (return_code != -ENOIOCTLCMD))
- context->return_code = -EINTR;
- else
- context->return_code = return_code;
-
- if (context->in_syscall && !context->dummy) {
- audit_filter_syscall(tsk, context, &audit_filter_list[AUDIT_FILTER_EXIT]);
- audit_filter_inodes(tsk, context);
- }
-
- audit_set_context(tsk, NULL);
- return context;
-}
-
static inline void audit_proctitle_free(struct audit_context *context)
{
kfree(context->proctitle.value);
@@ -1296,15 +1258,18 @@ static inline int audit_proctitle_rtrim(char *proctitle, int len)
return len;
}
-static void audit_log_proctitle(struct task_struct *tsk,
- struct audit_context *context)
+static void audit_log_proctitle(void)
{
int res;
char *buf;
char *msg = "(null)";
int len = strlen(msg);
+ struct audit_context *context = audit_context();
struct audit_buffer *ab;
+ if (!context || context->dummy)
+ return;
+
ab = audit_log_start(context, GFP_KERNEL, AUDIT_PROCTITLE);
if (!ab)
return; /* audit_panic or being filtered */
@@ -1317,7 +1282,7 @@ static void audit_log_proctitle(struct task_struct *tsk,
if (!buf)
goto out;
/* Historically called this from procfs naming */
- res = get_cmdline(tsk, buf, MAX_PROCTITLE_AUDIT_LEN);
+ res = get_cmdline(current, buf, MAX_PROCTITLE_AUDIT_LEN);
if (res == 0) {
kfree(buf);
goto out;
@@ -1337,15 +1302,15 @@ static void audit_log_proctitle(struct task_struct *tsk,
audit_log_end(ab);
}
-static void audit_log_exit(struct audit_context *context, struct task_struct *tsk)
+static void audit_log_exit(void)
{
int i, call_panic = 0;
+ struct audit_context *context = audit_context();
struct audit_buffer *ab;
struct audit_aux_data *aux;
struct audit_names *n;
- /* tsk == current */
- context->personality = tsk->personality;
+ context->personality = current->personality;
ab = audit_log_start(context, GFP_KERNEL, AUDIT_SYSCALL);
if (!ab)
@@ -1367,7 +1332,7 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
context->argv[3],
context->name_count);
- audit_log_task_info(ab, tsk);
+ audit_log_task_info(ab);
audit_log_key(ab, context->filterkey);
audit_log_end(ab);
@@ -1456,7 +1421,7 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
audit_log_name(context, n, NULL, i++, &call_panic);
}
- audit_log_proctitle(tsk, context);
+ audit_log_proctitle();
/* Send end of event record to help user space know we are finished */
ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
@@ -1474,22 +1439,31 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
*/
void __audit_free(struct task_struct *tsk)
{
- struct audit_context *context;
+ struct audit_context *context = tsk->audit_context;
- context = audit_take_context(tsk, 0, 0);
if (!context)
return;
- /* Check for system calls that do not go through the exit
- * function (e.g., exit_group), then free context block.
- * We use GFP_ATOMIC here because we might be doing this
- * in the context of the idle thread */
- /* that can happen only if we are called from do_exit() */
- if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
- audit_log_exit(context, tsk);
+ /* We are called either by do_exit() or the fork() error handling code;
+ * in the former case tsk == current and in the latter tsk is a
+ * random task_struct that doesn't doesn't have any meaningful data we
+ * need to log via audit_log_exit().
+ */
+ if (tsk == current && !context->dummy && context->in_syscall) {
+ context->return_valid = 0;
+ context->return_code = 0;
+
+ audit_filter_syscall(tsk, context,
+ &audit_filter_list[AUDIT_FILTER_EXIT]);
+ audit_filter_inodes(tsk, context);

Once we've established that tsk == current, I think it would read better
if we used current in the two filter calls and in fact simplify it even
more and just eliminate the task_struct parameter from both audit_filter
calls since it will only ever be current.

Post by Paul Moore
+ if (context->current_state == AUDIT_RECORD_CONTEXT)
+ audit_log_exit();
+ }
+
if (!list_empty(&context->killed_trees))
audit_kill_trees(&context->killed_trees);
+ audit_set_context(tsk, NULL);
audit_free_context(context);
}
@@ -1559,17 +1533,40 @@ void __audit_syscall_exit(int success, long return_code)
{
struct audit_context *context;
- if (success)
- success = AUDITSC_SUCCESS;
- else
- success = AUDITSC_FAILURE;
-
- context = audit_take_context(current, success, return_code);
+ context = audit_context();
if (!context)
return;
- if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
- audit_log_exit(context, current);
+ if (!context->dummy && context->in_syscall) {
+ if (success)
+ context->return_valid = AUDITSC_SUCCESS;
+ else
+ context->return_valid = AUDITSC_FAILURE;
+
+ /*
+ * we need to fix up the return code in the audit logs if the
+ * actual return codes are later going to be fixed up by the
+ * arch specific signal handlers
+ *
+ * (rc == ERESTARTSYS ) || (rc == ERESTARTNOINTR) ||
+ * (rc == ERESTARTNOHAND) || (rc == ERESTART_RESTARTBLOCK)
+ *
+ * but is faster than a bunch of ||
+ */
+ if (unlikely(return_code <= -ERESTARTSYS) &&
+ (return_code >= -ERESTART_RESTARTBLOCK) &&
+ (return_code != -ENOIOCTLCMD))
+ context->return_code = -EINTR;
+ else
+ context->return_code = return_code;
+
+ audit_filter_syscall(current, context,
+ &audit_filter_list[AUDIT_FILTER_EXIT]);
+ audit_filter_inodes(current, context);
+ if (context->current_state == AUDIT_RECORD_CONTEXT)
+ audit_log_exit();
+ }
context->in_syscall = 0;
context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
@@ -1591,7 +1588,6 @@ void __audit_syscall_exit(int success, long return_code)
kfree(context->filterkey);
context->filterkey = NULL;
}
- audit_set_context(current, context);
}
static inline void handle_one(const struct inode *inode)
@@ -2025,7 +2021,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
uid = from_kuid(&init_user_ns, task_uid(current));
oldloginuid = from_kuid(&init_user_ns, koldloginuid);
loginuid = from_kuid(&init_user_ns, kloginuid),
- tty = audit_get_tty(current);
+ tty = audit_get_tty();
audit_log_format(ab, "pid=%d uid=%u", task_tgid_nr(current), uid);
audit_log_task_context(ab);
@@ -2046,7 +2042,6 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
*/
int audit_set_loginuid(kuid_t loginuid)
{
- struct task_struct *task = current;
unsigned int oldsessionid, sessionid = AUDIT_SID_UNSET;
kuid_t oldloginuid;
int rc;
@@ -2065,8 +2060,8 @@ int audit_set_loginuid(kuid_t loginuid)
sessionid = (unsigned int)atomic_inc_return(&session_id);
}
- task->sessionid = sessionid;
- task->loginuid = loginuid;
+ current->sessionid = sessionid;
+ current->loginuid = loginuid;
audit_log_set_loginuid(oldloginuid, loginuid, oldsessionid, sessionid, rc);
return rc;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 99dd1d53fc35..af134588ab4e 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -336,7 +336,7 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
audit_log_untrustedstring(ab, filename);
audit_log_format(ab, " hash=\"%s:%s\"", algo_name, hash);
- audit_log_task_info(ab, current);
+ audit_log_task_info(ab);
audit_log_end(ab);
iint->flags |= IMA_AUDITED;

- RGB

--
Richard Guy Briggs <***@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

[Paul Moore]

Post by Richard Guy Briggs

Post by Paul Moore
There are many places, notably audit_log_task_info() and
audit_log_exit(), that take task_struct pointers but in reality they
are always working on the current task. This patch eliminates the
task_struct arguments and uses current directly which allows a number
of cleanups as well.

Ack. This looks like a welcome simplification. This will cause a merge
conflict with my ghak59 patch 4&5 (but Jan's patch didn't) which is
fine. I'd like to rebase ghak59 on this once this is merged to get the
EOE record in the right place after kill_trees...

Thanks for the review, I just merged it into audit/next so you should
be able to rebase your patches now.

Post by Richard Guy Briggs
One minor comment in __audit_free() below once we've established tsk ==
current...

...

Post by Richard Guy Briggs
Once we've established that tsk == current, I think it would read better
if we used current in the two filter calls and in fact simplify it even
more and just eliminate the task_struct parameter from both audit_filter
calls since it will only ever be current.

Yeah, I went back and forth on this too, but eventually decided to
leave this as-is for the moment - another one of those judgement
calls. In the case of the filter functions, both were pretty small
and I could *maybe* see them being useful for tasks other than current
... maybe not. We can always convert them in the future too.