From dc9eb698f441889f2d7926b1cc6f1e14f0787f00 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 19 Apr 2013 13:23:09 -0400 Subject: audit: stop pushing loginid, uid, sessionid as arguments We always use current. Stop pulling this when the skb comes in and pushing it around as arguments. Just get it at the end when you need it. Signed-off-by: Eric Paris --- drivers/tty/tty_audit.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index 6953dc82850c..1e4e9f30ea09 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -202,10 +202,12 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch) * reference to the tty audit buffer if available. * Flush the buffer or return an appropriate error code. */ -int tty_audit_push_task(struct task_struct *tsk, kuid_t loginuid, u32 sessionid) +int tty_audit_push_task(struct task_struct *tsk) { struct tty_audit_buf *buf = ERR_PTR(-EPERM); unsigned long flags; + kuid_t loginuid = audit_get_loginuid(tsk); + u32 sessionid = audit_get_sessionid(tsk); if (!lock_task_sighand(tsk, &flags)) return -ESRCH; -- cgit From 152f497b9b5940f81de3205465840a5eb316458e Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Fri, 19 Apr 2013 13:56:11 -0400 Subject: audit: push loginuid and sessionid processing down Since we are always current, we can push a lot of this stuff to the bottom and get rid of useless interfaces and arguments. Signed-off-by: Eric Paris --- drivers/tty/tty_audit.c | 72 +++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 48 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index 1e4e9f30ea09..ea2e5ad71731 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -60,24 +60,22 @@ static void tty_audit_buf_put(struct tty_audit_buf *buf) tty_audit_buf_free(buf); } -static void tty_audit_log(const char *description, struct task_struct *tsk, - kuid_t loginuid, unsigned sessionid, int major, - int minor, unsigned char *data, size_t size) +static void tty_audit_log(const char *description, int major, int minor, + unsigned char *data, size_t size) { struct audit_buffer *ab; + struct task_struct *tsk = current; + uid_t uid = from_kuid(&init_user_ns, task_uid(tsk)); + uid_t loginuid = from_kuid(&init_user_ns, audit_get_loginuid(tsk)); + u32 sessionid = audit_get_sessionid(tsk); ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_TTY); if (ab) { char name[sizeof(tsk->comm)]; - kuid_t uid = task_uid(tsk); - - audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u " - "major=%d minor=%d comm=", description, - tsk->pid, - from_kuid(&init_user_ns, uid), - from_kuid(&init_user_ns, loginuid), - sessionid, - major, minor); + + audit_log_format(ab, "%s pid=%u uid=%u auid=%u ses=%u major=%d" + " minor=%d comm=", description, tsk->pid, uid, + loginuid, sessionid, major, minor); get_task_comm(name, tsk); audit_log_untrustedstring(ab, name); audit_log_format(ab, " data="); @@ -90,11 +88,9 @@ static void tty_audit_log(const char *description, struct task_struct *tsk, * tty_audit_buf_push - Push buffered data out * * Generate an audit message from the contents of @buf, which is owned by - * @tsk with @loginuid. @buf->mutex must be locked. + * the current task. @buf->mutex must be locked. */ -static void tty_audit_buf_push(struct task_struct *tsk, kuid_t loginuid, - unsigned int sessionid, - struct tty_audit_buf *buf) +static void tty_audit_buf_push(struct tty_audit_buf *buf) { if (buf->valid == 0) return; @@ -102,24 +98,10 @@ static void tty_audit_buf_push(struct task_struct *tsk, kuid_t loginuid, buf->valid = 0; return; } - tty_audit_log("tty", tsk, loginuid, sessionid, buf->major, buf->minor, - buf->data, buf->valid); + tty_audit_log("tty", buf->major, buf->minor, buf->data, buf->valid); buf->valid = 0; } -/** - * tty_audit_buf_push_current - Push buffered data out - * - * Generate an audit message from the contents of @buf, which is owned by - * the current task. @buf->mutex must be locked. - */ -static void tty_audit_buf_push_current(struct tty_audit_buf *buf) -{ - kuid_t auid = audit_get_loginuid(current); - unsigned int sessionid = audit_get_sessionid(current); - tty_audit_buf_push(current, auid, sessionid, buf); -} - /** * tty_audit_exit - Handle a task exit * @@ -138,7 +120,7 @@ void tty_audit_exit(void) return; mutex_lock(&buf->mutex); - tty_audit_buf_push_current(buf); + tty_audit_buf_push(buf); mutex_unlock(&buf->mutex); tty_audit_buf_put(buf); @@ -176,7 +158,7 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch) if (buf) { mutex_lock(&buf->mutex); if (buf->major == major && buf->minor == minor) - tty_audit_buf_push_current(buf); + tty_audit_buf_push(buf); mutex_unlock(&buf->mutex); tty_audit_buf_put(buf); } @@ -187,27 +169,21 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch) auid = audit_get_loginuid(current); sessionid = audit_get_sessionid(current); - tty_audit_log("ioctl=TIOCSTI", current, auid, sessionid, major, - minor, &ch, 1); + tty_audit_log("ioctl=TIOCSTI", major, minor, &ch, 1); } } /** - * tty_audit_push_task - Flush task's pending audit data - * @tsk: task pointer - * @loginuid: sender login uid - * @sessionid: sender session id + * tty_audit_push_current - Flush current's pending audit data * - * Called with a ref on @tsk held. Try to lock sighand and get a - * reference to the tty audit buffer if available. + * Try to lock sighand and get a reference to the tty audit buffer if available. * Flush the buffer or return an appropriate error code. */ -int tty_audit_push_task(struct task_struct *tsk) +int tty_audit_push_current(void) { struct tty_audit_buf *buf = ERR_PTR(-EPERM); + struct task_struct *tsk = current; unsigned long flags; - kuid_t loginuid = audit_get_loginuid(tsk); - u32 sessionid = audit_get_sessionid(tsk); if (!lock_task_sighand(tsk, &flags)) return -ESRCH; @@ -227,7 +203,7 @@ int tty_audit_push_task(struct task_struct *tsk) return PTR_ERR(buf); mutex_lock(&buf->mutex); - tty_audit_buf_push(tsk, loginuid, sessionid, buf); + tty_audit_buf_push(buf); mutex_unlock(&buf->mutex); tty_audit_buf_put(buf); @@ -311,7 +287,7 @@ void tty_audit_add_data(struct tty_struct *tty, unsigned char *data, minor = tty->driver->minor_start + tty->index; if (buf->major != major || buf->minor != minor || buf->icanon != icanon) { - tty_audit_buf_push_current(buf); + tty_audit_buf_push(buf); buf->major = major; buf->minor = minor; buf->icanon = icanon; @@ -327,7 +303,7 @@ void tty_audit_add_data(struct tty_struct *tty, unsigned char *data, data += run; size -= run; if (buf->valid == N_TTY_BUF_SIZE) - tty_audit_buf_push_current(buf); + tty_audit_buf_push(buf); } while (size != 0); mutex_unlock(&buf->mutex); tty_audit_buf_put(buf); @@ -359,7 +335,7 @@ void tty_audit_push(struct tty_struct *tty) minor = tty->driver->minor_start + tty->index; mutex_lock(&buf->mutex); if (buf->major == major && buf->minor == minor) - tty_audit_buf_push_current(buf); + tty_audit_buf_push(buf); mutex_unlock(&buf->mutex); tty_audit_buf_put(buf); } -- cgit From bde02ca858448cf54a4226774dd1481f3bcc455e Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 30 Apr 2013 11:01:14 -0400 Subject: audit: use spin_lock_irqsave/restore in audit tty code Some of the callers of the audit tty function use spin_lock_irqsave/restore. We were using the forced always enable version, which seems really bad. Since I don't know every one of these code paths well enough, it makes sense to just switch everything to the safe version. Maybe it's a little overzealous, but it's a lot better than an unlucky deadlock when we return to a caller with irq enabled and they expect it to be disabled. Signed-off-by: Eric Paris --- drivers/tty/tty_audit.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index ea2e5ad71731..755d418019c8 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -111,11 +111,12 @@ static void tty_audit_buf_push(struct tty_audit_buf *buf) void tty_audit_exit(void) { struct tty_audit_buf *buf; + unsigned long flags; - spin_lock_irq(¤t->sighand->siglock); + spin_lock_irqsave(¤t->sighand->siglock, flags); buf = current->signal->tty_audit_buf; current->signal->tty_audit_buf = NULL; - spin_unlock_irq(¤t->sighand->siglock); + spin_unlock_irqrestore(¤t->sighand->siglock, flags); if (!buf) return; @@ -133,9 +134,11 @@ void tty_audit_exit(void) */ void tty_audit_fork(struct signal_struct *sig) { - spin_lock_irq(¤t->sighand->siglock); + unsigned long flags; + + spin_lock_irqsave(¤t->sighand->siglock, flags); sig->audit_tty = current->signal->audit_tty; - spin_unlock_irq(¤t->sighand->siglock); + spin_unlock_irqrestore(¤t->sighand->siglock, flags); } /** @@ -145,13 +148,14 @@ void tty_audit_tiocsti(struct tty_struct *tty, char ch) { struct tty_audit_buf *buf; int major, minor, should_audit; + unsigned long flags; - spin_lock_irq(¤t->sighand->siglock); + spin_lock_irqsave(¤t->sighand->siglock, flags); should_audit = current->signal->audit_tty; buf = current->signal->tty_audit_buf; if (buf) atomic_inc(&buf->count); - spin_unlock_irq(¤t->sighand->siglock); + spin_unlock_irqrestore(¤t->sighand->siglock, flags); major = tty->driver->major; minor = tty->driver->minor_start + tty->index; @@ -221,10 +225,11 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty, unsigned icanon) { struct tty_audit_buf *buf, *buf2; + unsigned long flags; buf = NULL; buf2 = NULL; - spin_lock_irq(¤t->sighand->siglock); + spin_lock_irqsave(¤t->sighand->siglock, flags); if (likely(!current->signal->audit_tty)) goto out; buf = current->signal->tty_audit_buf; @@ -232,7 +237,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty, atomic_inc(&buf->count); goto out; } - spin_unlock_irq(¤t->sighand->siglock); + spin_unlock_irqrestore(¤t->sighand->siglock, flags); buf2 = tty_audit_buf_alloc(tty->driver->major, tty->driver->minor_start + tty->index, @@ -242,7 +247,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty, return NULL; } - spin_lock_irq(¤t->sighand->siglock); + spin_lock_irqsave(¤t->sighand->siglock, flags); if (!current->signal->audit_tty) goto out; buf = current->signal->tty_audit_buf; @@ -254,7 +259,7 @@ static struct tty_audit_buf *tty_audit_buf_get(struct tty_struct *tty, atomic_inc(&buf->count); /* Fall through */ out: - spin_unlock_irq(¤t->sighand->siglock); + spin_unlock_irqrestore(¤t->sighand->siglock, flags); if (buf2) tty_audit_buf_free(buf2); return buf; @@ -317,16 +322,17 @@ void tty_audit_add_data(struct tty_struct *tty, unsigned char *data, void tty_audit_push(struct tty_struct *tty) { struct tty_audit_buf *buf; + unsigned long flags; - spin_lock_irq(¤t->sighand->siglock); + spin_lock_irqsave(¤t->sighand->siglock, flags); if (likely(!current->signal->audit_tty)) { - spin_unlock_irq(¤t->sighand->siglock); + spin_unlock_irqrestore(¤t->sighand->siglock, flags); return; } buf = current->signal->tty_audit_buf; if (buf) atomic_inc(&buf->count); - spin_unlock_irq(¤t->sighand->siglock); + spin_unlock_irqrestore(¤t->sighand->siglock, flags); if (buf) { int major, minor; -- cgit From 46e959ea2969cc1668d09b0dc55226946cf781f1 Mon Sep 17 00:00:00 2001 From: Richard Guy Briggs Date: Fri, 3 May 2013 14:03:50 -0400 Subject: audit: add an option to control logging of passwords with pam_tty_audit Most commands are entered one line at a time and processed as complete lines in non-canonical mode. Commands that interactively require a password, enter canonical mode to do this while shutting off echo. This pair of features (icanon and !echo) can be used to avoid logging passwords by audit while still logging the rest of the command. Adding a member (log_passwd) to the struct audit_tty_status passed in by pam_tty_audit allows control of canonical mode without echo per task. Signed-off-by: Richard Guy Briggs Signed-off-by: Eric Paris --- drivers/tty/tty_audit.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers') diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index 755d418019c8..5f3868202183 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -138,6 +138,7 @@ void tty_audit_fork(struct signal_struct *sig) spin_lock_irqsave(¤t->sighand->siglock, flags); sig->audit_tty = current->signal->audit_tty; + sig->audit_tty_log_passwd = current->signal->audit_tty_log_passwd; spin_unlock_irqrestore(¤t->sighand->siglock, flags); } @@ -275,10 +276,18 @@ void tty_audit_add_data(struct tty_struct *tty, unsigned char *data, { struct tty_audit_buf *buf; int major, minor; + int audit_log_tty_passwd; + unsigned long flags; if (unlikely(size == 0)) return; + spin_lock_irqsave(¤t->sighand->siglock, flags); + audit_log_tty_passwd = current->signal->audit_tty_log_passwd; + spin_unlock_irqrestore(¤t->sighand->siglock, flags); + if (!audit_log_tty_passwd && icanon && !L_ECHO(tty)) + return; + if (tty->driver->type == TTY_DRIVER_TYPE_PTY && tty->driver->subtype == PTY_TYPE_MASTER) return; -- cgit From 2ce88dd04276839002a3b31161090d282316610b Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 30 Apr 2013 10:43:10 -0400 Subject: audit: do not needlessly take a spinlock in copy_signal current->signal->audit_* can only change from a netlink message from current. Obviously in this case we cannot be handling a netlink message from current. So there is no change these can change under us. No need to take a lock to read them. Signed-off-by: Eric Paris --- drivers/tty/tty_audit.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index 5f3868202183..a03a75163f02 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -134,12 +134,8 @@ void tty_audit_exit(void) */ void tty_audit_fork(struct signal_struct *sig) { - unsigned long flags; - - spin_lock_irqsave(¤t->sighand->siglock, flags); sig->audit_tty = current->signal->audit_tty; sig->audit_tty_log_passwd = current->signal->audit_tty_log_passwd; - spin_unlock_irqrestore(¤t->sighand->siglock, flags); } /** -- cgit From bee0a224e791cccbc7ecd7faf2d5932942668976 Mon Sep 17 00:00:00 2001 From: Eric Paris Date: Tue, 30 Apr 2013 10:46:46 -0400 Subject: audit: do not needlessly take a lock in tty_audit_exit We were doing spin_lock_irq and spin_unlock_irq. This is STOOPID. If we were in interupt context we were already screwed and called panic() in do_exit(). So the irq stuff is useless. Also, these values can only be changed by receiving a netlink message from current. Since we are in do_exit() clearly we aren't in the syscall sending the netlink message to change these values. Thus, just read them and go with it. Signed-off-by: Eric Paris --- drivers/tty/tty_audit.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers') diff --git a/drivers/tty/tty_audit.c b/drivers/tty/tty_audit.c index a03a75163f02..a4fdce74f883 100644 --- a/drivers/tty/tty_audit.c +++ b/drivers/tty/tty_audit.c @@ -111,12 +111,9 @@ static void tty_audit_buf_push(struct tty_audit_buf *buf) void tty_audit_exit(void) { struct tty_audit_buf *buf; - unsigned long flags; - spin_lock_irqsave(¤t->sighand->siglock, flags); buf = current->signal->tty_audit_buf; current->signal->tty_audit_buf = NULL; - spin_unlock_irqrestore(¤t->sighand->siglock, flags); if (!buf) return; -- cgit