commit e218eb32f508c828dc87d0d724c70e2cf9b7866e
author Peter Hurley <peter@hurleysoftware.com> Thu Oct 16 14:59:49 2014 -0400
committer Greg Kroah-Hartman <gregkh@linuxfoundation.org> Wed Nov 05 16:26:14 2014 -0800
tree 52ad8dc062bd5266757924eabef81e225927ec04
parent e1c2296c3485158304bfad5a80e89078463d70c8

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 <peter@hurleysoftware.com>
Reviewed-by: Alan Cox <alan@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

drivers/tty/tty_io.c

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index ae8f53c..07275d0 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -508,7 +508,8 @@
  * 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 @@
 		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(&current->sighand->siglock);
 	if (!noctty &&
@@ -2167,7 +2165,6 @@
 	spin_unlock_irq(&current->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 @@
  *	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 @@
 {
 	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 @@
 	proc_set_tty(tty);
 unlock:
 	read_unlock(&tasklist_lock);
-	mutex_unlock(&tty_mutex);
+	tty_unlock(tty);
 	return ret;
 }