On Fri, Apr 22, 2016 at 1:16 PM, Peter Hurley <pe...@hurleysoftware.com> wrote: > On 04/21/2016 11:14 AM, Richard Guy Briggs wrote: >> 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);
I just merged Richard's patch, if nothing else it is better than it was. However, I would like to talk about improving things, see below. > Not that I'm objecting because I get that you're just refactoring > existing code, but I thought I'd point out some stuff. > > 1. There's no need to check if signal_struct is NULL (ie. tsk->signal) > because if it is, this will blow up trying to dereference the > sighand_struct (ie tsk->sighand). > > 2. The existing usage is always tsk==current Yep, there is only one caller I found that even works on task_structs other than current (see audit_log_exit() via audit_free()), although even then when it ends up calling into audit_log_task_info() tsk should always be current. I've got a patch compiling now to get rid of passing around current as a a task_struct argument, assuming nothing blows up in testing I'll post/merge it. > 3. If the idea is to make this invulnerable to tsk being gone, then > the usage is unsafe anyway. I don't think that is our concern here. > So ultimately (but not necessarily for this patch) I'd prefer that either > a. audit use existing tty api instead of open-coding, or > b. add any tty api functions required. I'm open to suggestions, care to elaborate on either option? Feel free to elaborate by patch too ;) -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit