Re: [PATCH] audit: add tty field to LOGIN event

Hi Richard,

On 04/13/2016 04:25 PM, Richard Guy Briggs 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(). > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; > --- > include/linux/audit.h | 18 ++++++++++++++++++ > kernel/audit.c | 11 +---------- > kernel/auditsc.c | 5 +++-- > 3 files changed, 22 insertions(+), 12 deletions(-) > > diff --git a/include/linux/audit.h b/include/linux/audit.h > index b40ed5d..20c6649 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,19 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk) > return tsk->sessionid; > } > > +static inline char *audit_get_tty(struct task_struct *tsk) > +{ > + char *tty; > + > + spin_lock_irq(&tsk->sighand->siglock); > + if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name) > + tty = tsk->signal->tty->name; > + else > + tty = "(none)"; > + spin_unlock_irq(&tsk->sighand->siglock); This is unsafe because the tty could be immediately torn down after the siglock is dropped, and return a dangling ptr.

> + return tty; > +} > + > extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp); > extern void __audit_ipc_set_perm(unsigned long qbytes, uid_t uid, gid_t gid, umode_t mode); > extern void __audit_bprm(struct linux_binprm *bprm); > @@ -500,6 +514,10 @@ static inline unsigned int audit_get_sessionid(struct task_struct *tsk) > { > return -1; > } > +static inline char *audit_get_tty(struct task_struct *tsk) > +{ > + return "(invalid)"; > +} > static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp) > { } > static inline void audit_ipc_set_perm(unsigned long qbytes, uid_t uid, > diff --git a/kernel/audit.c b/kernel/audit.c > index 3a3e5de..fae11df 100644 > --- a/kernel/audit.c > +++ b/kernel/audit.c > @@ -64,7 +64,6 @@ > #include <linux/security.h> > #endif > #include <linux/freezer.h> > -#include <linux/tty.h> > #include <linux/pid_namespace.h> > #include <net/netns/generic.h> > > @@ -1873,7 +1872,6 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk) > { > const struct cred *cred; > char comm[sizeof(tsk->comm)]; > - char *tty; struct tty_struct *tty; > > if (!ab) > return; > @@ -1881,13 +1879,6 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk) > /* tsk == current */ > cred = current_cred(); > > - spin_lock_irq(&tsk->sighand->siglock); > - if (tsk->signal && tsk->signal->tty && tsk->signal->tty->name) > - tty = tsk->signal->tty->name; > - else > - tty = "(none)"; > - spin_unlock_irq(&tsk->sighand->siglock); tty = get_current_tty(); > - > audit_log_format(ab, > " ppid=%d pid=%d auid=%u uid=%u gid=%u" > " euid=%u suid=%u fsuid=%u" > @@ -1903,7 +1894,7 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk) > from_kgid(&init_user_ns, cred->egid), > from_kgid(&init_user_ns, cred->sgid), > from_kgid(&init_user_ns, cred->fsgid), > - tty, audit_get_sessionid(tsk)); > + audit_get_tty(tsk), audit_get_sessionid(tsk)); tty_name(tty), ....); ^^^^^^^^^^ returns "NULL tty" if tty == NULL tty_kref_put(tty); > > audit_log_format(ab, " comm="); > audit_log_untrustedstring(ab, get_task_comm(comm, tsk)); > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 195ffae..a0467fb 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -1993,8 +1993,9 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid, > 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); tty = get_current_tty(); > + audit_log_format(ab, " old-auid=%u auid=%u tty=%s old-ses=%u ses=%u res=%d", > + oldloginuid, loginuid, audit_get_tty(current), ......., tty_name(tty), > + oldsessionid, sessionid, !rc); tty_kref_put(tty); Regards, Peter Hurley > audit_log_end(ab); > } > >