Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 30 Jan 2024 17:11:27 GMT
From:      Kyle Evans <kevans@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 050b4db5a8c1 - stable/13 - kern: tty: recanonicalize the buffer on ICANON/VEOF/VEOL changes
Message-ID:  <202401301711.40UHBReA093659@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=050b4db5a8c1a32a0c77391b7dc81ef5a2ef93f7

commit 050b4db5a8c1a32a0c77391b7dc81ef5a2ef93f7
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2024-01-16 02:55:59 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-01-30 17:11:10 +0000

    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
    
    (cherry picked from commit 522083ffbd1ab9b485861750e889d606dc75ed0a)
    (cherry picked from commit 5738d741fb796c1f0a6b5c2157af7de58104ac97)
---
 sys/kern/tty.c         | 22 +++++++++++++++++++---
 sys/kern/tty_inq.c     | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 sys/kern/tty_ttydisc.c | 22 ++++++++++++++++++++++
 sys/sys/ttydisc.h      |  1 +
 sys/sys/ttyqueue.h     |  1 +
 5 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index d46462842038..d7f9a914166a 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -1703,6 +1703,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,
@@ -1752,6 +1753,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;
@@ -1760,13 +1774,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 291a815fa2db..daf3bde77712 100644
--- a/sys/kern/tty_inq.c
+++ b/sys/kern/tty_inq.c
@@ -346,6 +346,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 = ti->ti_begin;
+
+	while (off > ti->ti_begin) {
+		off--;
+		boff = off % TTYINQ_DATASIZE;
+
+		if (strchr(breakc, tib->tib_data[boff]) && !GETBIT(tib, boff)) {
+			canon = off + 1;
+			break;
+		}
+
+		if (off != ti->ti_begin && boff == 0)
+			tib = tib->tib_prev;
+	}
+
+	MPASS(canon > ti->ti_begin || off == ti->ti_begin);
+
+	/*
+	 * We should only be able to hit canon == ti_begin if we walked
+	 * everything we have and didn't find any of the break characters, so
+	 * if canon == ti_begin 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 != ti->ti_begin && (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 04d99c336438..92f75d672269 100644
--- a/sys/kern/tty_ttydisc.c
+++ b/sys/kern/tty_ttydisc.c
@@ -167,6 +167,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);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202401301711.40UHBReA093659>