tty: Fix the ldisc hangup race

This was noticed by Matthias Urlichs and he proposed a fix. This patch does the fixing a different way to avoid introducing several new race conditions into the code. The problem case is TTY_DRIVER_RESET_TERMIOS = 0. In that case while we abort the ldisc change, the hangup processing has not cleaned up and restarted the ldisc either. We can't restart the ldisc stuff in the set_ldisc as we don't know what the hangup did and may touch stuff we shouldn't as we are no longer supposed to influence the tty at that point in case it has been re-opened before we get rescheduled. Instead do it the simple way. Always re-init the ldisc on the hangup, but use TTY_DRIVER_RESET_TERMIOS to indicate that we should force N_TTY. Signed-off-by: Alan Cox <alan@linux.intel.com> Cc: stable <stable@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

tty: Fix the ldisc hangup race
This was noticed by Matthias Urlichs and he proposed a fix. This patch does the fixing a different way to avoid introducing several new race conditions into the code. The problem case is TTY_DRIVER_RESET_TERMIOS = 0. In that case while we abort the ldisc change, the hangup processing has not cleaned up and restarted the ldisc either. We can't restart the ldisc stuff in the set_ldisc as we don't know what the hangup did and may touch stuff we shouldn't as we are no longer supposed to influence the tty at that point in case it has been re-opened before we get rescheduled. Instead do it the simple way. Always re-init the ldisc on the hangup, but use TTY_DRIVER_RESET_TERMIOS to indicate that we should force N_TTY. Signed-off-by: Alan Cox <alan@linux.intel.com> Cc: stable <stable@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Alan Cox · Greg Kroah-Hartman
1 parent 97c22394bb
Showing 1 changed file with 30 additions and 20 deletions Side-by-side Diff
drivers/char/tty_ldisc.c
@@ -706,12 +706,13 @@
 /**
  *	tty_ldisc_reinit	-	reinitialise the tty ldisc
  *	@tty: tty to reinit
+ *	@ldisc: line discipline to reinitialize
  *
- *	Switch the tty back to N_TTY line discipline and leave the
- *	ldisc state closed
+ *	Switch the tty to a line discipline and leave the ldisc
+ *	state closed
  */
  
-static void tty_ldisc_reinit(struct tty_struct *tty)
+static void tty_ldisc_reinit(struct tty_struct *tty, int ldisc)
 {
 	struct tty_ldisc *ld;
  
  
@@ -721,10 +722,10 @@
 	/*
 	 *	Switch the line discipline back
 	 */
-	ld = tty_ldisc_get(N_TTY);
+	ld = tty_ldisc_get(ldisc);
 	BUG_ON(IS_ERR(ld));
 	tty_ldisc_assign(tty, ld);
-	tty_set_termios_ldisc(tty, N_TTY);
+	tty_set_termios_ldisc(tty, ldisc);
 }
  
 /**
@@ -745,6 +746,8 @@
 void tty_ldisc_hangup(struct tty_struct *tty)
 {
 	struct tty_ldisc *ld;
+	int reset = tty->driver->flags & TTY_DRIVER_RESET_TERMIOS;
+	int err = 0;
  
 	/*
 	 * FIXME! What are the locking issues here? This may me overdoing
  
  
  
  
@@ -772,25 +775,32 @@
 	wake_up_interruptible_poll(&tty->read_wait, POLLIN);
 	/*
 	 * Shutdown the current line discipline, and reset it to
-	 * N_TTY.
+	 * N_TTY if need be.
+	 *
+	 * Avoid racing set_ldisc or tty_ldisc_release
 	 */
-	if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) {
-		/* Avoid racing set_ldisc or tty_ldisc_release */
-		mutex_lock(&tty->ldisc_mutex);
-		tty_ldisc_halt(tty);
-		if (tty->ldisc) {	/* Not yet closed */
-			/* Switch back to N_TTY */
-			tty_ldisc_reinit(tty);
-			/* At this point we have a closed ldisc and we want to
-			   reopen it. We could defer this to the next open but
-			   it means auditing a lot of other paths so this is
-			   a FIXME */
+	mutex_lock(&tty->ldisc_mutex);
+	tty_ldisc_halt(tty);
+	/* At this point we have a closed ldisc and we want to
+	   reopen it. We could defer this to the next open but
+	   it means auditing a lot of other paths so this is
+	   a FIXME */
+	if (tty->ldisc) {	/* Not yet closed */
+		if (reset == 0) {
+			tty_ldisc_reinit(tty, tty->termios->c_line);
+			err = tty_ldisc_open(tty, tty->ldisc);
+		}
+		/* If the re-open fails or we reset then go to N_TTY. The
+		   N_TTY open cannot fail */
+		if (reset || err) {
+			tty_ldisc_reinit(tty, N_TTY);
 			WARN_ON(tty_ldisc_open(tty, tty->ldisc));
-			tty_ldisc_enable(tty);
 		}
-		mutex_unlock(&tty->ldisc_mutex);
-		tty_reset_termios(tty);
+		tty_ldisc_enable(tty);
 	}
+	mutex_unlock(&tty->ldisc_mutex);
+	if (reset)
+		tty_reset_termios(tty);
 }
  
 /**
...	...	@@ -706,12 +706,13 @@
706	706	/**
707	707	* tty_ldisc_reinit - reinitialise the tty ldisc
708	708	* @tty: tty to reinit
	709	+ * @ldisc: line discipline to reinitialize
709	710	*
710		- * Switch the tty back to N_TTY line discipline and leave the
711		- * ldisc state closed
	711	+ * Switch the tty to a line discipline and leave the ldisc
	712	+ * state closed
712	713	*/
713	714
714		-static void tty_ldisc_reinit(struct tty_struct *tty)
	715	+static void tty_ldisc_reinit(struct tty_struct *tty, int ldisc)
715	716	{
716	717	struct tty_ldisc *ld;
717	718
718	719
...	...	@@ -721,10 +722,10 @@
721	722	/*
722	723	* Switch the line discipline back
723	724	*/
724		- ld = tty_ldisc_get(N_TTY);
	725	+ ld = tty_ldisc_get(ldisc);
725	726	BUG_ON(IS_ERR(ld));
726	727	tty_ldisc_assign(tty, ld);
727		- tty_set_termios_ldisc(tty, N_TTY);
	728	+ tty_set_termios_ldisc(tty, ldisc);
728	729	}
729	730
730	731	/**
...	...	@@ -745,6 +746,8 @@
745	746	void tty_ldisc_hangup(struct tty_struct *tty)
746	747	{
747	748	struct tty_ldisc *ld;
	749	+ int reset = tty->driver->flags & TTY_DRIVER_RESET_TERMIOS;
	750	+ int err = 0;
748	751
749	752	/*
750	753	* FIXME! What are the locking issues here? This may me overdoing
751	754
752	755
753	756
754	757
...	...	@@ -772,25 +775,32 @@
772	775	wake_up_interruptible_poll(&tty->read_wait, POLLIN);
773	776	/*
774	777	* Shutdown the current line discipline, and reset it to
775		- * N_TTY.
	778	+ * N_TTY if need be.
	779	+ *
	780	+ * Avoid racing set_ldisc or tty_ldisc_release
776	781	*/
777		- if (tty->driver->flags & TTY_DRIVER_RESET_TERMIOS) {
778		- /* Avoid racing set_ldisc or tty_ldisc_release */
779		- mutex_lock(&tty->ldisc_mutex);
780		- tty_ldisc_halt(tty);
781		- if (tty->ldisc) { /* Not yet closed */
782		- /* Switch back to N_TTY */
783		- tty_ldisc_reinit(tty);
784		- /* At this point we have a closed ldisc and we want to
785		- reopen it. We could defer this to the next open but
786		- it means auditing a lot of other paths so this is
787		- a FIXME */
	782	+ mutex_lock(&tty->ldisc_mutex);
	783	+ tty_ldisc_halt(tty);
	784	+ /* At this point we have a closed ldisc and we want to
	785	+ reopen it. We could defer this to the next open but
	786	+ it means auditing a lot of other paths so this is
	787	+ a FIXME */
	788	+ if (tty->ldisc) { /* Not yet closed */
	789	+ if (reset == 0) {
	790	+ tty_ldisc_reinit(tty, tty->termios->c_line);
	791	+ err = tty_ldisc_open(tty, tty->ldisc);
	792	+ }
	793	+ /* If the re-open fails or we reset then go to N_TTY. The
	794	+ N_TTY open cannot fail */
	795	+ if (reset \|\| err) {
	796	+ tty_ldisc_reinit(tty, N_TTY);
788	797	WARN_ON(tty_ldisc_open(tty, tty->ldisc));
789		- tty_ldisc_enable(tty);
790	798	}
791		- mutex_unlock(&tty->ldisc_mutex);
792		- tty_reset_termios(tty);
	799	+ tty_ldisc_enable(tty);
793	800	}
	801	+ mutex_unlock(&tty->ldisc_mutex);
	802	+ if (reset)
	803	+ tty_reset_termios(tty);
794	804	}
795	805
796	806	/**