Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752789AbcDNAbX (ORCPT ); Wed, 13 Apr 2016 20:31:23 -0400 Received: from mail-pf0-f178.google.com ([209.85.192.178]:34153 "EHLO mail-pf0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750928AbcDNAbV (ORCPT ); Wed, 13 Apr 2016 20:31:21 -0400 Subject: Re: [PATCH] audit: add tty field to LOGIN event To: Richard Guy Briggs , linux-audit@redhat.com, linux-kernel@vger.kernel.org References: <4587fd4a69c5d41f9596c0644ce22dc38db47d04.1460589810.git.rgb@redhat.com> Cc: sgrubb@redhat.com, pmoore@redhat.com, eparis@redhat.com From: Peter Hurley Message-ID: <570EE4D4.4080903@hurleysoftware.com> Date: Wed, 13 Apr 2016 17:31:16 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <4587fd4a69c5d41f9596c0644ce22dc38db47d04.1460589810.git.rgb@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4457 Lines: 151 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 > --- > 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 > #include > #include > +#include > > #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 > #endif > #include > -#include > #include > #include > > @@ -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); > } > >