On Thu, Apr 21, 2016 at 2:14 PM, Richard Guy Briggs <rgb(a)redhat.com> wrote:
The tty field was missing from AUDIT_LOGIN events.
Refactor code to create a new function audit_get_tty(), using it to
replace the call in audit_log_task_info() and to add it to
audit_log_set_loginuid(). Lock and bump the kref to protect it, adding
audit_put_tty() alias to decrement it.
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
V4: Add missing prototype for audit_put_tty() when audit syscall is not
enabled (MIPS).
V3: Introduce audit_put_tty() alias to decrement kref.
V2: Use kref to protect tty signal struct while in use.
---
include/linux/audit.h | 24 ++++++++++++++++++++++++
kernel/audit.c | 18 +++++-------------
kernel/auditsc.c | 8 ++++++--
3 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index b40ed5d..32cdafb 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -26,6 +26,7 @@
#include <linux/sched.h>
#include <linux/ptrace.h>
#include <uapi/linux/audit.h>
+#include <linux/tty.h>
#define AUDIT_INO_UNSET ((unsigned long)-1)
#define AUDIT_DEV_UNSET ((dev_t)-1)
@@ -343,6 +344,23 @@ static inline unsigned int audit_get_sessionid(struct task_struct
*tsk)
return tsk->sessionid;
}
+static inline struct tty_struct *audit_get_tty(struct task_struct *tsk)
+{
+ 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);
+ return tty;
+}
+
+static inline void audit_put_tty(struct tty_struct *tty)
+{
+ tty_kref_put(tty);
+}
I'm generally not a big fan of defining functions as inlines in header
files, with the general exception of dummy functions that will get
compiled out. Although I guess there might be some performance
argument to be made wrt audit_log_task_info().
I guess I'm fine with this, but what was the idea behind the static
inlines in audit.h? Performance, or something else?
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 195ffae..71e14d8 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1980,6 +1980,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t
kloginuid,
{
struct audit_buffer *ab;
uid_t uid, oldloginuid, loginuid;
+ struct tty_struct *tty;
if (!audit_enabled)
return;
@@ -1987,14 +1988,17 @@ 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);
ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_LOGIN);
if (!ab)
return;
audit_log_format(ab, "pid=%d uid=%u", task_pid_nr(current), uid);
audit_log_task_context(ab);
- audit_log_format(ab, " old-auid=%u auid=%u old-ses=%u ses=%u res=%d",
- oldloginuid, loginuid, oldsessionid, sessionid, !rc);
+ audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u
res=%d",
+ oldloginuid, loginuid, tty ? tty_name(tty) :
"(none)",
+ oldsessionid, sessionid, !rc);
+ audit_put_tty(tty);
audit_log_end(ab);
}
Because we are still using the crappy fixed string format for
kernel<->userspace communication, this patch will likely "break the
world" ... let's check with Steve but I believe the way to handle this
is to add the tty information to the end of the record.
--
paul moore
www.paul-moore.com