kern: tty: recanonicalize the buffer on ICANON/VEOF/VEOL changes

Before this change, we would canonicalize any partial input if the new
local mode is not ICANON, but that's about it.  If we were switching
from -ICANON -> ICANON, or if VEOF/VEOL changes, then our internal canon
accounting would be wrong.

The main consequence of this is that in ICANON mode, we would
potentially hang a read(2) longer if the new VEOF/VEOL appears later in
the buffer, and FIONREAD would be similarly wrong as a result.

Reviewed by:	kib
Differential Revision:	https://reviews.freebsd.org/D43456

5 files changed, 92 insertions, 3 deletions

diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index e4186a67cb31..29bb092a50b0 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -1705,6 +1705,7 @@ tty_generic_ioctl(struct tty *tp, u_long cmd, void *data, int fflag,
 	case TIOCSETAW:
 	case TIOCSETAF: {
 		struct termios *t = data;
+		bool canonicalize = false;
 
 		/*
 		 * Who makes up these funny rules? According to POSIX,
@@ -1754,6 +1755,19 @@ tty_generic_ioctl(struct tty *tp, u_long cmd, void *data, int fflag,
 				return (error);
 		}
 
+		/*
+		 * We'll canonicalize any partial input if we're transitioning
+		 * ICANON one way or the other.  If we're going from -ICANON ->
+		 * ICANON, then in the worst case scenario we're in the middle
+		 * of a line but both ttydisc_read() and FIONREAD will search
+		 * for one of our line terminals.
+		 */
+		if ((t->c_lflag & ICANON) != (tp->t_termios.c_lflag & ICANON))
+			canonicalize = true;
+		else if (tp->t_termios.c_cc[VEOF] != t->c_cc[VEOF] ||
+		    tp->t_termios.c_cc[VEOL] != t->c_cc[VEOL])
+			canonicalize = true;
+
 		/* Copy new non-device driver parameters. */
 		tp->t_termios.c_iflag = t->c_iflag;
 		tp->t_termios.c_oflag = t->c_oflag;
@@ -1762,13 +1776,15 @@ tty_generic_ioctl(struct tty *tp, u_long cmd, void *data, int fflag,
 
 		ttydisc_optimize(tp);
 
+		if (canonicalize)
+			ttydisc_canonicalize(tp);
 		if ((t->c_lflag & ICANON) == 0) {
 			/*
 			 * When in non-canonical mode, wake up all
-			 * readers. Canonicalize any partial input. VMIN
-			 * and VTIME could also be adjusted.
+			 * readers. Any partial input has already been
+			 * canonicalized above if we were in canonical mode.
+			 * VMIN and VTIME could also be adjusted.
 			 */
-			ttyinq_canonicalize(&tp->t_inq);
 			tty_wakeup(tp, FREAD);
 		}
 
diff --git a/sys/kern/tty_inq.c b/sys/kern/tty_inq.c
index 2eed9802a7b7..533fdfd30ce9 100644
--- a/sys/kern/tty_inq.c
+++ b/sys/kern/tty_inq.c
@@ -354,6 +354,55 @@ ttyinq_canonicalize(struct ttyinq *ti)
 	ti->ti_startblock = ti->ti_reprintblock = ti->ti_lastblock;
 }
 
+/*
+ * Canonicalize at one of the break characters; we'll work backwards from the
+ * lastblock to firstblock to try and find the latest one.
+ */
+void
+ttyinq_canonicalize_break(struct ttyinq *ti, const char *breakc)
+{
+	struct ttyinq_block *tib = ti->ti_lastblock;
+	unsigned int canon, off;
+	unsigned int boff;
+
+	/* No block, no change needed. */
+	if (tib == NULL || ti->ti_end == 0)
+		return;
+
+	/* Start just past the end... */
+	off = ti->ti_end;
+	canon = 0;
+
+	while (off > 0) {
+		if ((off % TTYINQ_DATASIZE) == 0)
+			tib = tib->tib_prev;
+
+		off--;
+		boff = off % TTYINQ_DATASIZE;
+
+		if (strchr(breakc, tib->tib_data[boff]) && !GETBIT(tib, boff)) {
+			canon = off + 1;
+			break;
+		}
+	}
+
+	MPASS(canon > 0 || off == 0);
+
+	/*
+	 * We should only be able to hit bcanon == 0 if we walked everything we
+	 * have and didn't find any of the break characters, so if bcanon == 0
+	 * then tib is already the correct block and we should avoid touching
+	 * it.
+	 *
+	 * For all other scenarios, if canon lies on a block boundary then tib
+	 * has already advanced to the previous block.
+	 */
+	if (canon != 0 && (canon % TTYINQ_DATASIZE) == 0)
+		tib = tib->tib_next;
+	ti->ti_linestart = ti->ti_reprint = canon;
+	ti->ti_startblock = ti->ti_reprintblock = tib;
+}
+
 size_t
 ttyinq_findchar(struct ttyinq *ti, const char *breakc, size_t maxlen,
     char *lastc)
diff --git a/sys/kern/tty_ttydisc.c b/sys/kern/tty_ttydisc.c
index f6e9e9869951..2be6b560d4f4 100644
--- a/sys/kern/tty_ttydisc.c
+++ b/sys/kern/tty_ttydisc.c
@@ -166,6 +166,28 @@ ttydisc_bytesavail(struct tty *tp)
 	return (clen);
 }
 
+void
+ttydisc_canonicalize(struct tty *tp)
+{
+	char breakc[4];
+
+	/*
+	 * If we're in non-canonical mode, it's as easy as just canonicalizing
+	 * the current partial line.
+	 */
+	if (!CMP_FLAG(l, ICANON)) {
+		ttyinq_canonicalize(&tp->t_inq);
+		return;
+	}
+
+	/*
+	 * For canonical mode, we need to rescan the buffer for the last EOL
+	 * indicator.
+	 */
+	ttydisc_read_break(tp, &breakc[0], sizeof(breakc));
+	ttyinq_canonicalize_break(&tp->t_inq, breakc);
+}
+
 static int
 ttydisc_read_canonical(struct tty *tp, struct uio *uio, int ioflag)
 {
diff --git a/sys/sys/ttydisc.h b/sys/sys/ttydisc.h
index 81d436139555..cdd3576afedf 100644
--- a/sys/sys/ttydisc.h
+++ b/sys/sys/ttydisc.h
@@ -47,6 +47,7 @@ void	ttydisc_close(struct tty *tp);
 size_t	ttydisc_bytesavail(struct tty *tp);
 int	ttydisc_read(struct tty *tp, struct uio *uio, int ioflag);
 int	ttydisc_write(struct tty *tp, struct uio *uio, int ioflag);
+void	ttydisc_canonicalize(struct tty *tp);
 void	ttydisc_optimize(struct tty *tp);
 
 /* Bottom half routines. */
diff --git a/sys/sys/ttyqueue.h b/sys/sys/ttyqueue.h
index fd5a6bf7719e..89c07b7faa10 100644
--- a/sys/sys/ttyqueue.h
+++ b/sys/sys/ttyqueue.h
@@ -78,6 +78,7 @@ size_t	ttyinq_write(struct ttyinq *ti, const void *buf, size_t len,
 int	ttyinq_write_nofrag(struct ttyinq *ti, const void *buf, size_t len,
     int quote);
 void	ttyinq_canonicalize(struct ttyinq *ti);
+void	ttyinq_canonicalize_break(struct ttyinq *ti, const char *breakc);
 size_t	ttyinq_findchar(struct ttyinq *ti, const char *breakc, size_t maxlen,
     char *lastc);
 void	ttyinq_flush(struct ttyinq *ti);