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 v1.2.3 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 +++++++++++++++++-------------------------------- include/linux/tty.h | 4 +-- kernel/audit.c | 2 +- 3 files changed, 27 insertions(+), 51 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); } diff --git a/include/linux/tty.h b/include/linux/tty.h index 78e378b3971c..96d640b32cd8 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -517,7 +517,7 @@ extern void tty_audit_exit(void); extern void tty_audit_fork(struct signal_struct *sig); extern void tty_audit_tiocsti(struct tty_struct *tty, char ch); extern void tty_audit_push(struct tty_struct *tty); -extern int tty_audit_push_task(struct task_struct *tsk); +extern int tty_audit_push_current(void); #else static inline void tty_audit_add_data(struct tty_struct *tty, unsigned char *data, size_t size, unsigned icanon) @@ -535,7 +535,7 @@ static inline void tty_audit_fork(struct signal_struct *sig) static inline void tty_audit_push(struct tty_struct *tty) { } -static inline int tty_audit_push_task(struct task_struct *tsk) +static inline int tty_audit_push_current(void) { return 0; } diff --git a/kernel/audit.c b/kernel/audit.c index bf1e1330cbb1..79b42fd14c22 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -727,7 +727,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) if (err == 1) { err = 0; if (msg_type == AUDIT_USER_TTY) { - err = tty_audit_push_task(current); + err = tty_audit_push_current(); if (err) break; } -- cgit v1.2.3 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 +++++++++++++++++++------------- kernel/audit.c | 10 ++++++---- 2 files changed, 25 insertions(+), 17 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; diff --git a/kernel/audit.c b/kernel/audit.c index 44803f25b236..241aa8593fa8 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -804,10 +804,11 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) case AUDIT_TTY_GET: { struct audit_tty_status s; struct task_struct *tsk = current; + unsigned long flags; - spin_lock_irq(&tsk->sighand->siglock); + spin_lock_irqsave(&tsk->sighand->siglock, flags); s.enabled = tsk->signal->audit_tty != 0; - spin_unlock_irq(&tsk->sighand->siglock); + spin_unlock_irqrestore(&tsk->sighand->siglock, flags); audit_send_reply(NETLINK_CB(skb).portid, seq, AUDIT_TTY_GET, 0, 0, &s, sizeof(s)); @@ -816,6 +817,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) case AUDIT_TTY_SET: { struct audit_tty_status *s; struct task_struct *tsk = current; + unsigned long flags; if (nlh->nlmsg_len < sizeof(struct audit_tty_status)) return -EINVAL; @@ -823,9 +825,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) if (s->enabled != 0 && s->enabled != 1) return -EINVAL; - spin_lock_irq(&tsk->sighand->siglock); + spin_lock_irqsave(&tsk->sighand->siglock, flags); tsk->signal->audit_tty = s->enabled != 0; - spin_unlock_irq(&tsk->sighand->siglock); + spin_unlock_irqrestore(&tsk->sighand->siglock, flags); break; } default: -- cgit v1.2.3 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 +++++++++ include/linux/sched.h | 1 + include/uapi/linux/audit.h | 3 ++- kernel/audit.c | 16 ++++++++++------ 4 files changed, 22 insertions(+), 7 deletions(-) (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; diff --git a/include/linux/sched.h b/include/linux/sched.h index d2112477ff5e..c4689fe92864 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -641,6 +641,7 @@ struct signal_struct { #endif #ifdef CONFIG_AUDIT unsigned audit_tty; + unsigned audit_tty_log_passwd; struct tty_audit_buf *tty_audit_buf; #endif #ifdef CONFIG_CGROUPS diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h index 9f096f1c0907..c058c24b97ac 100644 --- a/include/uapi/linux/audit.h +++ b/include/uapi/linux/audit.h @@ -369,7 +369,8 @@ struct audit_status { }; struct audit_tty_status { - __u32 enabled; /* 1 = enabled, 0 = disabled */ + __u32 enabled; /* 1 = enabled, 0 = disabled */ + __u32 log_passwd; /* 1 = enabled, 0 = disabled */ }; /* audit_rule_data supports filter rules with both integer and string diff --git a/kernel/audit.c b/kernel/audit.c index 241aa8593fa8..998a0d4155cf 100644 --- a/kernel/audit.c +++ b/kernel/audit.c @@ -49,6 +49,7 @@ #include #include #include +#include #include @@ -808,6 +809,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) spin_lock_irqsave(&tsk->sighand->siglock, flags); s.enabled = tsk->signal->audit_tty != 0; + s.log_passwd = tsk->signal->audit_tty_log_passwd; spin_unlock_irqrestore(&tsk->sighand->siglock, flags); audit_send_reply(NETLINK_CB(skb).portid, seq, @@ -815,18 +817,20 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) break; } case AUDIT_TTY_SET: { - struct audit_tty_status *s; + struct audit_tty_status s; struct task_struct *tsk = current; unsigned long flags; - if (nlh->nlmsg_len < sizeof(struct audit_tty_status)) - return -EINVAL; - s = data; - if (s->enabled != 0 && s->enabled != 1) + memset(&s, 0, sizeof(s)); + /* guard against past and future API changes */ + memcpy(&s, data, min(sizeof(s), (size_t)nlh->nlmsg_len)); + if ((s.enabled != 0 && s.enabled != 1) || + (s.log_passwd != 0 && s.log_passwd != 1)) return -EINVAL; spin_lock_irqsave(&tsk->sighand->siglock, flags); - tsk->signal->audit_tty = s->enabled != 0; + tsk->signal->audit_tty = s.enabled; + tsk->signal->audit_tty_log_passwd = s.log_passwd; spin_unlock_irqrestore(&tsk->sighand->siglock, flags); break; } -- cgit v1.2.3 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 v1.2.3 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 v1.2.3