From 522083ffbd1ab9b485861750e889d606dc75ed0a Mon Sep 17 00:00:00 2001 From: Kyle Evans Date: Mon, 15 Jan 2024 20:55:59 -0600 Subject: [PATCH] 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 --- 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 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);