On 16/04/13, Peter Hurley wrote: > Hi Richard, Hi Peter,
> 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 <[email protected]> > > --- > > 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. Understood. The other option is to copy the value out... Thanks for the helpful review. Rev 2 coming... > > + 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); > > } > > > > > - RGB -- Richard Guy Briggs <[email protected]> Kernel Security Engineering, Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635
