git: 2a05c2033908 - stable/13 - kern: tty: fix EOF handling for canonical reads

From: Kyle Evans <kevans_at_FreeBSD.org>
Date: Tue, 30 Jan 2024 17:11:25 UTC
The branch stable/13 has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=2a05c20339082c19e64f9027f80706b8d24a082d

commit 2a05c20339082c19e64f9027f80706b8d24a082d
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2024-01-16 02:55:58 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2024-01-30 17:11:09 +0000

    kern: tty: fix EOF handling for canonical reads
    
    If the read(2) buffer is one byte short of an EOF, then we'll end up
    reading the line into the buffer, then re-entering and seeing an EOF at
    the beginning of the inq, assuming it's a zero-length line.
    
    Fix this corner-case by searching one more byte than we have available
    for an EOF.  If we found it, then we'll trim it here; otherwise, we'll
    limit our read to just the space we have in the out buffer and the next
    read(2) will (potentially) read the remainder of the line.
    
    Fix FIONREAD while we're here to match what an application can expect
    read(2) to return -- scan for the first break character in the part of
    the input that's been canonicalized, we'll never return more than that.
    
    PR:     276220
    Reviewed by:    cy, imp (both previous version), kib
    
    (cherry picked from commit d51dac5f1370bdca1ea20c6b48cdea463f6f5dda)
---
 sys/kern/tty.c         |  2 +-
 sys/kern/tty_ttydisc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++------
 sys/sys/ttydisc.h      |  1 +
 3 files changed, 66 insertions(+), 9 deletions(-)

diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index eaf69446afa2..d46462842038 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -1671,7 +1671,7 @@ tty_generic_ioctl(struct tty *tp, u_long cmd, void *data, int fflag,
 		/* This device supports non-blocking operation. */
 		return (0);
 	case FIONREAD:
-		*(int *)data = ttyinq_bytescanonicalized(&tp->t_inq);
+		*(int *)data = ttydisc_bytesavail(tp);
 		return (0);
 	case FIONWRITE:
 	case TIOCOUTQ:
diff --git a/sys/kern/tty_ttydisc.c b/sys/kern/tty_ttydisc.c
index cb5bf672d040..04d99c336438 100644
--- a/sys/kern/tty_ttydisc.c
+++ b/sys/kern/tty_ttydisc.c
@@ -113,15 +113,20 @@ ttydisc_close(struct tty *tp)
 		ttyhook_close(tp);
 }
 
-static int
-ttydisc_read_canonical(struct tty *tp, struct uio *uio, int ioflag)
+/*
+ * Populate our break array; it should likely be at least 4 bytes in size to
+ * allow for \n, VEOF, and VEOL.
+ */
+static void
+ttydisc_read_break(struct tty *tp, char *breakc, size_t breaksz)
 {
-	char breakc[4] = { CNL }; /* enough to hold \n, VEOF and VEOL. */
-	int error;
-	size_t clen, flen = 0, n = 1;
-	unsigned char lastc = _POSIX_VDISABLE;
+	size_t n = 0;
 
+	MPASS(breaksz != 0);
+
+	breakc[n++] = CNL;
 #define BREAK_ADD(c) do { \
+	MPASS(n < breaksz - 1);	/* NUL terminated */	\
 	if (tp->t_termios.c_cc[c] != _POSIX_VDISABLE)	\
 		breakc[n++] = tp->t_termios.c_cc[c];	\
 } while (0)
@@ -129,7 +134,48 @@ ttydisc_read_canonical(struct tty *tp, struct uio *uio, int ioflag)
 	BREAK_ADD(VEOF);
 	BREAK_ADD(VEOL);
 #undef BREAK_ADD
+
 	breakc[n] = '\0';
+}
+
+size_t
+ttydisc_bytesavail(struct tty *tp)
+{
+	size_t clen;
+	char breakc[4];
+	unsigned char lastc = _POSIX_VDISABLE;
+
+	clen = ttyinq_bytescanonicalized(&tp->t_inq);
+	if (!CMP_FLAG(l, ICANON) || clen == 0)
+		return (clen);
+
+	ttydisc_read_break(tp, &breakc[0], sizeof(breakc));
+	clen = ttyinq_findchar(&tp->t_inq, breakc, clen, &lastc);
+
+	/*
+	 * We might have a partial line canonicalized in the input queue if we,
+	 * for instance, switched to ICANON after taking some input in raw mode.
+	 * In this case, read(2) will block because we only have a partial line.
+	 */
+	if (lastc == _POSIX_VDISABLE)
+		return (0);
+
+	/* If VEOF was our terminal, it must be discarded (not counted). */
+	if (CMP_CC(VEOF, lastc))
+		clen--;
+
+	return (clen);
+}
+
+static int
+ttydisc_read_canonical(struct tty *tp, struct uio *uio, int ioflag)
+{
+	char breakc[4]; /* enough to hold \n, VEOF and VEOL. */
+	int error;
+	size_t clen, flen = 0;
+	unsigned char lastc = _POSIX_VDISABLE;
+
+	ttydisc_read_break(tp, &breakc[0], sizeof(breakc));
 
 	do {
 		error = tty_wait_background(tp, curthread, SIGTTIN);
@@ -154,7 +200,7 @@ ttydisc_read_canonical(struct tty *tp, struct uio *uio, int ioflag)
 		 * cause the TTY layer to return data in chunks using
 		 * the blocksize (except the first and last blocks).
 		 */
-		clen = ttyinq_findchar(&tp->t_inq, breakc, uio->uio_resid,
+		clen = ttyinq_findchar(&tp->t_inq, breakc, uio->uio_resid + 1,
 		    &lastc);
 
 		/* No more data. */
@@ -170,10 +216,20 @@ ttydisc_read_canonical(struct tty *tp, struct uio *uio, int ioflag)
 			continue;
 		}
 
-		/* Don't send the EOF char back to userspace. */
+		/*
+		 * Don't send the EOF char back to userspace.  Our above call to
+		 * ttyinq_findchar overreads by 1 character in case we would
+		 * otherwise be leaving an EOF for the next read().  We'll trim
+		 * clen back down to uio_resid whether we find our EOF or not.
+		 */
 		if (CMP_CC(VEOF, lastc))
 			flen = 1;
 
+		/*
+		 * Trim clen back down to the buffer size, since we had
+		 * intentionally over-read.
+		 */
+		clen = MIN(uio->uio_resid + flen, clen);
 		MPASS(flen <= clen);
 
 		/* Read and throw away the EOF character. */
diff --git a/sys/sys/ttydisc.h b/sys/sys/ttydisc.h
index 0458fae6e34b..81d436139555 100644
--- a/sys/sys/ttydisc.h
+++ b/sys/sys/ttydisc.h
@@ -44,6 +44,7 @@ struct uio;
 /* Top half routines. */
 void	ttydisc_open(struct tty *tp);
 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_optimize(struct tty *tp);