summaryrefslogtreecommitdiff
path: root/drivers/tty/tty_io.c
diff options
context:
space:
mode:
authorJann Horn <jannh@google.com>2020-12-03 02:25:05 +0100
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2020-12-04 17:39:58 +0100
commitc8bcd9c5be24fb9e6132e97da5a35e55a83e36b9 (patch)
tree1b7e3191d3fd63c02d3029b19e45d88d322397df /drivers/tty/tty_io.c
parent54ffccbf053b5b6ca4f6e45094b942fab92a25fc (diff)
tty: Fix ->session locking
Currently, locking of ->session is very inconsistent; most places protect it using the legacy tty mutex, but disassociate_ctty(), __do_SAK(), tiocspgrp() and tiocgsid() don't. Two of the writers hold the ctrl_lock (because they already need it for ->pgrp), but __proc_set_tty() doesn't do that yet. On a PREEMPT=y system, an unprivileged user can theoretically abuse this broken locking to read 4 bytes of freed memory via TIOCGSID if tiocgsid() is preempted long enough at the right point. (Other things might also go wrong, especially if root-only ioctls are involved; I'm not sure about that.) Change the locking on ->session such that: - tty_lock() is held by all writers: By making disassociate_ctty() hold it. This should be fine because the same lock can already be taken through the call to tty_vhangup_session(). The tricky part is that we need to shorten the area covered by siglock to be able to take tty_lock() without ugly retry logic; as far as I can tell, this should be fine, since nothing in the signal_struct is touched in the `if (tty)` branch. - ctrl_lock is held by all writers: By changing __proc_set_tty() to hold the lock a little longer. - All readers that aren't holding tty_lock() hold ctrl_lock: By adding locking to tiocgsid() and __do_SAK(), and expanding the area covered by ctrl_lock in tiocspgrp(). Cc: stable@kernel.org Signed-off-by: Jann Horn <jannh@google.com> Reviewed-by: Jiri Slaby <jirislaby@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/tty/tty_io.c')
-rw-r--r--drivers/tty/tty_io.c7
1 files changed, 6 insertions, 1 deletions
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 9f8b9a567b35..56ade99ef99f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2897,10 +2897,14 @@ void __do_SAK(struct tty_struct *tty)
struct task_struct *g, *p;
struct pid *session;
int i;
+ unsigned long flags;
if (!tty)
return;
- session = tty->session;
+
+ spin_lock_irqsave(&tty->ctrl_lock, flags);
+ session = get_pid(tty->session);
+ spin_unlock_irqrestore(&tty->ctrl_lock, flags);
tty_ldisc_flush(tty);
@@ -2932,6 +2936,7 @@ void __do_SAK(struct tty_struct *tty)
task_unlock(p);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
+ put_pid(session);
#endif
}