linux-kernel - [PATCH 5.11 096/104] tty: fix up iterate_tty

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210305120907.879395756@linuxfoundation.org>
Date:   Fri,  5 Mar 2021 13:21:41 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org, Jiri Slaby <jirislaby@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: [PATCH 5.11 096/104] tty: fix up iterate_tty_read() EOVERFLOW handling

From: Linus Torvalds <torvalds@...ux-foundation.org>

commit e71a8d5cf4b4f274740e31b601216071e2a11afa upstream.

When I converted the tty_ldisc_ops 'read()' function to take a kernel
pointer, I was a bit too aggressive about the ldisc returning EOVERFLOW.

Yes, we want to have EOVERFLOW override any partially read data (because
the whole point is that the buffer was too small for the whole packet,
and we don't want to see partial packets), but it shouldn't override a
previous EFAULT.

And in fact, it really is just EOVERFLOW that is special and should
throw away any partially read data, not "any error".  Admittedly
EOVERFLOW is currently the only one that can happen for a continuation
read - and if the first read iteration returns an error we won't have this issue.

So this is more of a technicality, but let's just make the intent very
explicit, and re-organize the error handling a bit so that this is all
clearer.

Reported-by: Jiri Slaby <jirislaby@...nel.org>
Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
Reviewed-by: Jiri Slaby <jirislaby@...nel.org>
Link: https://lore.kernel.org/r/CAHk-=wh+-rGsa=xruEWdg_fJViFG8rN9bpLrfLz=_yBYh2tBhA@mail.gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
 drivers/tty/tty_io.c |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -859,13 +859,20 @@ static int iterate_tty_read(struct tty_l
 		if (!size)
 			break;

-		/*
-		 * A ldisc read error return will override any previously copied
-		 * data (eg -EOVERFLOW from HDLC)
-		 */
 		if (size < 0) {
-			memzero_explicit(kernel_buf, sizeof(kernel_buf));
-			return size;
+			/* Did we have an earlier error (ie -EFAULT)? */
+			if (retval)
+				break;
+			retval = size;
+
+			/*
+			 * -EOVERFLOW means we didn't have enough space
+			 * for a whole packet, and we shouldn't return
+			 * a partial result.
+			 */
+			if (retval == -EOVERFLOW)
+				offset = 0;
+			break;
 		}

 		copied = copy_to_iter(kernel_buf, size, to);