git: 5738d741fb79 - main - kern: tty: fix recanonicalization

From: Kyle Evans <kevans_at_FreeBSD.org>
Date: Wed, 24 Jan 2024 19:48:40 UTC
The branch main has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=5738d741fb796c1f0a6b5c2157af7de58104ac97

commit 5738d741fb796c1f0a6b5c2157af7de58104ac97
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2024-01-24 19:36:26 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-01-24 19:48:31 +0000

    kern: tty: fix recanonicalization
    
    `ti->ti_begin` is actually the offset within the first block that is
    unread, so we must use that for our lower bound.
    
    Moving to the previous block has to be done at the end of the loop in
    order to correctly handle the case of ti_begin == TTYINQ_DATASIZE.  At
    that point, lastblock is still the last one with data written and the
    next write into the queue would advance lastblock.  If we move to the
    previous block at the beginning, then we're essentially off by one block
    for the entire scan and run the risk of running off the end of the block
    queue.
    
    The ti_begin == 0 case is still handled correctly, as we skip the loop
    entirely and the linestart gets recorded as the first byte available for
    writing.  The bit after the loop about moving to the next block is also
    still correct, even with both previous fixes in mind: we skipped moving
    to the previous block if we hit ti_begin, and `off + 1` would in-fact be
    a member of the next block from where we're reading if it falls on a
    block boundary.
    
    Reported by:    dim
    Fixes:  522083ffbd1ab ("kern: tty: recanonicalize the buffer on [...]")
---
 sys/kern/tty_inq.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/sys/kern/tty_inq.c b/sys/kern/tty_inq.c
index 533fdfd30ce9..589d958e59fa 100644
--- a/sys/kern/tty_inq.c
+++ b/sys/kern/tty_inq.c
@@ -371,12 +371,9 @@ ttyinq_canonicalize_break(struct ttyinq *ti, const char *breakc)
 
 	/* Start just past the end... */
 	off = ti->ti_end;
-	canon = 0;
-
-	while (off > 0) {
-		if ((off % TTYINQ_DATASIZE) == 0)
-			tib = tib->tib_prev;
+	canon = ti->ti_begin;
 
+	while (off > ti->ti_begin) {
 		off--;
 		boff = off % TTYINQ_DATASIZE;
 
@@ -384,20 +381,23 @@ ttyinq_canonicalize_break(struct ttyinq *ti, const char *breakc)
 			canon = off + 1;
 			break;
 		}
+
+		if (off != ti->ti_begin && boff == 0)
+			tib = tib->tib_prev;
 	}
 
-	MPASS(canon > 0 || off == 0);
+	MPASS(canon > ti->ti_begin || off == ti->ti_begin);
 
 	/*
-	 * 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.
+	 * 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 != 0 && (canon % TTYINQ_DATASIZE) == 0)
+	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;