diff options
Diffstat (limited to 'meta-openbmc-mods/meta-common/recipes-kernel/linux/linux-aspeed/CVE-2020-29660/0001-tty-Fix-session-locking.patch')
| -rw-r--r-- | meta-openbmc-mods/meta-common/recipes-kernel/linux/linux-aspeed/CVE-2020-29660/0001-tty-Fix-session-locking.patch | 204 |
1 files changed, 204 insertions, 0 deletions
diff --git a/meta-openbmc-mods/meta-common/recipes-kernel/linux/linux-aspeed/CVE-2020-29660/0001-tty-Fix-session-locking.patch b/meta-openbmc-mods/meta-common/recipes-kernel/linux/linux-aspeed/CVE-2020-29660/0001-tty-Fix-session-locking.patch new file mode 100644 index 000000000..a00ce2a95 --- /dev/null +++ b/meta-openbmc-mods/meta-common/recipes-kernel/linux/linux-aspeed/CVE-2020-29660/0001-tty-Fix-session-locking.patch @@ -0,0 +1,204 @@ +From 35ee9ac513280f46eeb1196bac82ed5320380412 Mon Sep 17 00:00:00 2001 +From: Jann Horn <jannh@google.com> +Date: Thu, 3 Dec 2020 02:25:05 +0100 +Subject: [PATCH] tty: Fix ->session locking + +commit c8bcd9c5be24fb9e6132e97da5a35e55a83e36b9 upstream. + +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> +--- + drivers/tty/tty_io.c | 7 ++++++- + drivers/tty/tty_jobctrl.c | 44 +++++++++++++++++++++++++++------------ + include/linux/tty.h | 4 ++++ + 3 files changed, 41 insertions(+), 14 deletions(-) + +diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c +index 36c1c59cc72a..642765bf1023 100644 +--- a/drivers/tty/tty_io.c ++++ b/drivers/tty/tty_io.c +@@ -2894,10 +2894,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); + +@@ -2929,6 +2933,7 @@ void __do_SAK(struct tty_struct *tty) + task_unlock(p); + } while_each_thread(g, p); + read_unlock(&tasklist_lock); ++ put_pid(session); + #endif + } + +diff --git a/drivers/tty/tty_jobctrl.c b/drivers/tty/tty_jobctrl.c +index af508957ff05..813be2c05262 100644 +--- a/drivers/tty/tty_jobctrl.c ++++ b/drivers/tty/tty_jobctrl.c +@@ -103,8 +103,8 @@ static void __proc_set_tty(struct tty_struct *tty) + put_pid(tty->session); + put_pid(tty->pgrp); + tty->pgrp = get_pid(task_pgrp(current)); +- spin_unlock_irqrestore(&tty->ctrl_lock, flags); + tty->session = get_pid(task_session(current)); ++ spin_unlock_irqrestore(&tty->ctrl_lock, flags); + if (current->signal->tty) { + tty_debug(tty, "current tty %s not NULL!!\n", + current->signal->tty->name); +@@ -293,20 +293,23 @@ void disassociate_ctty(int on_exit) + spin_lock_irq(¤t->sighand->siglock); + put_pid(current->signal->tty_old_pgrp); + current->signal->tty_old_pgrp = NULL; +- + tty = tty_kref_get(current->signal->tty); ++ spin_unlock_irq(¤t->sighand->siglock); ++ + if (tty) { + unsigned long flags; ++ ++ tty_lock(tty); + spin_lock_irqsave(&tty->ctrl_lock, flags); + put_pid(tty->session); + put_pid(tty->pgrp); + tty->session = NULL; + tty->pgrp = NULL; + spin_unlock_irqrestore(&tty->ctrl_lock, flags); ++ tty_unlock(tty); + tty_kref_put(tty); + } + +- spin_unlock_irq(¤t->sighand->siglock); + /* Now clear signal->tty under the lock */ + read_lock(&tasklist_lock); + session_clear_tty(task_session(current)); +@@ -477,14 +480,19 @@ static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t + return -ENOTTY; + if (retval) + return retval; +- if (!current->signal->tty || +- (current->signal->tty != real_tty) || +- (real_tty->session != task_session(current))) +- return -ENOTTY; ++ + if (get_user(pgrp_nr, p)) + return -EFAULT; + if (pgrp_nr < 0) + return -EINVAL; ++ ++ spin_lock_irq(&real_tty->ctrl_lock); ++ if (!current->signal->tty || ++ (current->signal->tty != real_tty) || ++ (real_tty->session != task_session(current))) { ++ retval = -ENOTTY; ++ goto out_unlock_ctrl; ++ } + rcu_read_lock(); + pgrp = find_vpid(pgrp_nr); + retval = -ESRCH; +@@ -494,12 +502,12 @@ static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t + if (session_of_pgrp(pgrp) != task_session(current)) + goto out_unlock; + retval = 0; +- spin_lock_irq(&real_tty->ctrl_lock); + put_pid(real_tty->pgrp); + real_tty->pgrp = get_pid(pgrp); +- spin_unlock_irq(&real_tty->ctrl_lock); + out_unlock: + rcu_read_unlock(); ++out_unlock_ctrl: ++ spin_unlock_irq(&real_tty->ctrl_lock); + return retval; + } + +@@ -511,20 +519,30 @@ static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t + * + * Obtain the session id of the tty. If there is no session + * return an error. +- * +- * Locking: none. Reference to current->signal->tty is safe. + */ + static int tiocgsid(struct tty_struct *tty, struct tty_struct *real_tty, pid_t __user *p) + { ++ unsigned long flags; ++ pid_t sid; ++ + /* + * (tty == real_tty) is a cheap way of + * testing if the tty is NOT a master pty. + */ + if (tty == real_tty && current->signal->tty != real_tty) + return -ENOTTY; ++ ++ spin_lock_irqsave(&real_tty->ctrl_lock, flags); + if (!real_tty->session) +- return -ENOTTY; +- return put_user(pid_vnr(real_tty->session), p); ++ goto err; ++ sid = pid_vnr(real_tty->session); ++ spin_unlock_irqrestore(&real_tty->ctrl_lock, flags); ++ ++ return put_user(sid, p); ++ ++err: ++ spin_unlock_irqrestore(&real_tty->ctrl_lock, flags); ++ return -ENOTTY; + } + + /* +diff --git a/include/linux/tty.h b/include/linux/tty.h +index a99e9b8e4e31..eb33d948788c 100644 +--- a/include/linux/tty.h ++++ b/include/linux/tty.h +@@ -306,6 +306,10 @@ struct tty_struct { + struct termiox *termiox; /* May be NULL for unsupported */ + char name[64]; + struct pid *pgrp; /* Protected by ctrl lock */ ++ /* ++ * Writes protected by both ctrl lock and legacy mutex, readers must use ++ * at least one of them. ++ */ + struct pid *session; + unsigned long flags; + int count; +-- +2.17.1 + |