From: Peter Hurley Date: Thu, 16 Oct 2014 18:59:49 +0000 (-0400) Subject: tty: Serialize proc_set_tty() with tty_lock X-Git-Tag: v3.19-rc1~79^2~206 X-Git-Url: https://git.karo-electronics.de/?a=commitdiff_plain;h=e218eb32f508c828dc87d0d724c70e2cf9b7866e;p=karo-tx-linux.git tty: Serialize proc_set_tty() with tty_lock Setting the controlling terminal for a session occurs with either the first open of a non-pty master tty or with ioctl(TIOCSCTTY). Since only the session leader can set the controlling terminal for a session (and the session leader cannot change), it is not necessary to prevent a process from attempting to set different ttys as the controlling terminal concurrently. So it's only necessary to prevent the same tty from becoming the controlling terminal for different session leaders. The tty_lock() is sufficient to prevent concurrent proc_set_tty() for the same tty. Remove the tty_mutex lock region; add tty_lock() to tiocsctty(). While this may appear to allow a race condition between opening the controlling tty via tty_open_current_tty() and stealing the controlling tty via ioctl(TIOCSCTTY, 1), that race condition already existed. Even if the tty_mutex prevented stealing the controlling tty while tty_open_current_tty() returned the original controlling tty, it cannot prevent stealing the controlling tty before tty_open() returns. Thus, tty_open() could already return a no-longer-controlling tty when opening /dev/tty. Signed-off-by: Peter Hurley Reviewed-by: Alan Cox Signed-off-by: Greg Kroah-Hartman --- diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c index ae8f53c7972d..07275d0e30a2 100644 --- a/drivers/tty/tty_io.c +++ b/drivers/tty/tty_io.c @@ -508,7 +508,8 @@ void proc_clear_tty(struct task_struct *p) * Only callable by the session leader and only if it does not already have * a controlling terminal. * - * Caller must hold: a readlock on tasklist_lock + * Caller must hold: tty_lock() + * a readlock on tasklist_lock * sighand lock */ static void __proc_set_tty(struct tty_struct *tty) @@ -2152,11 +2153,8 @@ retry_open: goto retry_open; } clear_bit(TTY_HUPPED, &tty->flags); - tty_unlock(tty); - mutex_lock(&tty_mutex); - tty_lock(tty); read_lock(&tasklist_lock); spin_lock_irq(¤t->sighand->siglock); if (!noctty && @@ -2167,7 +2165,6 @@ retry_open: spin_unlock_irq(¤t->sighand->siglock); read_unlock(&tasklist_lock); tty_unlock(tty); - mutex_unlock(&tty_mutex); return 0; err_unlock: mutex_unlock(&tty_mutex); @@ -2448,7 +2445,7 @@ static int fionbio(struct file *file, int __user *p) * leader to set this tty as the controlling tty for the session. * * Locking: - * Takes tty_mutex() to protect tty instance + * Takes tty_lock() to serialize proc_set_tty() for this tty * Takes tasklist_lock internally to walk sessions * Takes ->siglock() when updating signal->tty */ @@ -2457,7 +2454,7 @@ static int tiocsctty(struct tty_struct *tty, int arg) { int ret = 0; - mutex_lock(&tty_mutex); + tty_lock(tty); read_lock(&tasklist_lock); if (current->signal->leader && (task_session(current) == tty->session)) @@ -2490,7 +2487,7 @@ static int tiocsctty(struct tty_struct *tty, int arg) proc_set_tty(tty); unlock: read_unlock(&tasklist_lock); - mutex_unlock(&tty_mutex); + tty_unlock(tty); return ret; }