[PATCH 1/1] TTY: forbid setting ldisc on HUPPED tty, step 1

[Jiri Slaby]

Commit c65c9bc3e (tty: rewrite the ldisc locking) introduced a check
into tty_set_ldisc whether the terminal is already hung. In that case
it returns -EIO.

Later, commit aa3c8af86 (tty ldisc: Close/Reopen race prevention
should check the proper flag) effectively disabled the check by
replacing HUPPED by HUPPING flag. Those are two different flags and
the former check was actually correct. In principle we revert here to
the previous behavior plus we add a check if we are HUPPING. Because
it makes no sense to change ldisc when some other thread is making a
HUP but it had to unlock BTM temporarily.

The bug which commit aa3c8af86 above tried to avoid lays in fact in
login(1). It was fixed by util-linux commit 2e7035646 (login: close
tty before vhangup()) already. It was reproducible for example by the
following setup:
1) add a user ppp with shell set to /usr/bin/pppd
2) run agetty on ttyS0 to wait for a user
3) user connects via modem on ttyS0, types ppp to getty
4) 'login ppp' is executed
5) login performs hangup without properly closing ttyS0
6) login executes pppd
7) pppd does TIOCSETD attempt to N_PPP
8) kernel denies step 7) as ttyS0 is HUPPED
9) pppd calls some N_PPP ldisc's ioctls regardless -- uninterruptible
   sleep in tty_ioctl, trying to have a ref on tty->ldisc

Now to sum up the fixes:
* The fix in util-linux is that 5 is changed to properly close all
  ttyS0 instances.
* The workaround from commit aa3c8af86 was that 8) succeeds.
* This patch is the same as aa3c8af86 except we still return -EIO and
  warn the user this will not be supported in the future. This is
  because we do not want to have a user-visible regression here.

"Step 1" because step two would be to immediatelly return -EIO like in
the attached false branch in the patch.

Signed-off-by: Jiri Slaby <jslaby@xxxxxxx>
Cc: Shachar Shemesh <shachar@xxxxxxxx>
Cc: Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>
---
 drivers/tty/tty_ldisc.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c
index f4e6754..7cca3fd 100644
--- a/drivers/tty/tty_ldisc.c
+++ b/drivers/tty/tty_ldisc.c
@@ -563,6 +563,7 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 	struct tty_ldisc *o_ldisc, *new_ldisc;
 	int work, o_work = 0;
 	struct tty_struct *o_tty;
+	int retval_hupped = 0;
 
 	new_ldisc = tty_ldisc_get(ldisc);
 	if (IS_ERR(new_ldisc))
@@ -659,14 +660,32 @@ int tty_set_ldisc(struct tty_struct *tty, int ldisc)
 		goto enable;
 	}
 
-	if (test_bit(TTY_HUPPING, &tty->flags)) {
-		/* We were raced by the hangup method. It will have stomped
-		   the ldisc data and closed the ldisc down */
-		clear_bit(TTY_LDISC_CHANGING, &tty->flags);
-		mutex_unlock(&tty->ldisc_mutex);
-		tty_ldisc_put(new_ldisc);
-		tty_unlock(tty);
-		return -EIO;
+	if (test_bit(TTY_HUPPED, &tty->flags) ||
+			test_bit(TTY_HUPPING, &tty->flags)) {
+		/*
+		 * Until login(1) is fixed everywhere, let's fall through.
+		 * Change to goto with -EIO after 3.10.
+		 */
+		if (1) {
+			char ttyn[64], comm[TASK_COMM_LEN];
+			printk_ratelimited(KERN_WARNING "%s: %s calls TIOCSETD on a hung TTY (%s). This is deprecated and will stop working soon.\n",
+					__func__, get_task_comm(comm, current),
+					tty_name(tty, ttyn));
+			retval_hupped = -EIO;
+		} else {
+			/*
+			 * We were raced by the hangup method. It will have
+			 * stomped the ldisc data and closed the ldisc down
+			 */
+			tty_ldisc_put(new_ldisc);
+			retval = -EIO;
+			/*
+			 * We have to enable the original ldisc so that
+			 * processes see N_TTY and fail instead of waiting
+			 * infinitely.
+			 */
+			goto enable;
+		}
 	}
 
 	/* Shutdown the current discipline. */
@@ -709,7 +728,7 @@ enable:
 		schedule_work(&o_tty->port->buf.work);
 	mutex_unlock(&tty->ldisc_mutex);
 	tty_unlock(tty);
-	return retval;
+	return retval ? : retval_hupped;
 }
 
 /**
-- 
1.7.12


--------------040306050204040008060706--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/