linux/drivers/tty
Linus Torvalds 3593030761 tty: n_tty: do not look ahead for EOL character past the end of the buffer
Daniel Gibson reports that the n_tty code gets line termination wrong in
very specific cases:

 "If you feed a line with exactly 64 chars + terminating newline, and
  directly afterwards (without reading) another line into a pseudo
  terminal, the the first read() on the other side will return the 64
  char line *without* terminating newline, and the next read() will
  return the missing terminating newline AND the complete next line (if
  it fits in the buffer)"

and bisected the behavior to commit 3b830a9c34 ("tty: convert
tty_ldisc_ops 'read()' function to take a kernel pointer").

Now, digging deeper, it turns out that the behavior isn't exactly new:
what changed in commit 3b830a9c34 was that the tty line discipline
.read() function is now passed an intermediate kernel buffer rather than
the final user space buffer.

And that intermediate kernel buffer is 64 bytes in size - thus that
special case with exactly 64 bytes plus terminating newline.

The same problem did exist before, but historically the boundary was not
the 64-byte chunk, but the user-supplied buffer size, which is obviously
generally bigger (and potentially bigger than N_TTY_BUF_SIZE, which
would hide the issue entirely).

The reason is that the n_tty canon_copy_from_read_buf() code would look
ahead for the EOL character one byte further than it would actually
copy.  It would then decide that it had found the terminator, and unmark
it as an EOL character - which in turn explains why the next read
wouldn't then be terminated by it.

Now, the reason it did all this in the first place is related to some
historical and pretty obscure EOF behavior, see commit ac8f3bf883
("n_tty: Fix poll() after buffer-limited eof push read") and commit
40d5e0905a ("n_tty: Fix EOF push handling").

And the reason for the EOL confusion is that we treat EOF as a special
EOL condition, with the EOL character being NUL (aka "__DISABLED_CHAR"
in the kernel sources).

So that EOF look-ahead also affects the normal EOL handling.

This patch just removes the look-ahead that causes problems, because EOL
is much more critical than the historical "EOF in the middle of a line
that coincides with the end of the buffer" handling ever was.

Now, it is possible that we should indeed re-introduce the "look at next
character to see if it's a EOF" behavior, but if so, that should be done
not at the kernel buffer chunk boundary in canon_copy_from_read_buf(),
but at a higher level, when we run out of the user buffer.

In particular, the place to do that would be at the top of
'n_tty_read()', where we check if it's a continuation of a previously
started read, and there is no more buffer space left, we could decide to
just eat the __DISABLED_CHAR at that point.

But that would be a separate patch, because I suspect nobody actually
cares, and I'd like to get a report about it before bothering.

Fixes: 3b830a9c34 ("tty: convert tty_ldisc_ops 'read()' function to take a kernel pointer")
Fixes: ac8f3bf883 ("n_tty: Fix  poll() after buffer-limited eof push read")
Fixes: 40d5e0905a ("n_tty: Fix EOF push handling")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215611
Reported-and-tested-by: Daniel Gibson <metalcaedes@gmail.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2022-02-16 10:13:23 -08:00
..
hvc xen/console: harden hvc_xen against event channel storms 2021-12-16 08:24:08 +01:00
ipwireless tty: drop put_tty_driver 2021-07-27 12:17:21 +02:00
serdev Revert "serdev: BREAK/FRAME/PARITY/OVERRUN notification prototype V2" 2021-12-31 13:42:30 +01:00
serial parisc architecture fixes for kernel v5.17-rc5 2022-02-15 09:10:09 -08:00
vt vt_ioctl: add array_index_nospec to VT_ACTIVATE 2022-02-04 15:36:02 +01:00
amiserial.c tty: drop put_tty_driver 2021-07-27 12:17:21 +02:00
ehv_bytechan.c tty: drop put_tty_driver 2021-07-27 12:17:21 +02:00
goldfish.c tty: goldfish: Use platform_get_irq() to get the interrupt 2021-12-30 13:50:05 +01:00
Kconfig tty: add rpmsg driver 2021-10-21 12:35:35 +02:00
Makefile tty: add rpmsg driver 2021-10-21 12:35:35 +02:00
mips_ejtag_fdc.c tty: mips_ejtag_fdc: Make use of the helper function kthread_run_on_cpu() 2021-12-03 16:00:41 +01:00
moxa.c tty: drivers/tty/, stop using tty_schedule_flip() 2021-11-25 18:35:23 +01:00
mxser.c mxser: use PCI_DEVICE_DATA 2021-11-25 18:33:22 +01:00
n_gsm.c tty: n_gsm: fix SW flow control encoding/handling 2022-01-26 14:51:42 +01:00
n_hdlc.c Linux 5.16-rc6 2021-12-20 10:00:30 +01:00
n_null.c
n_tty.c tty: n_tty: do not look ahead for EOL character past the end of the buffer 2022-02-16 10:13:23 -08:00
nozomi.c tty: drop put_tty_driver 2021-07-27 12:17:21 +02:00
pty.c
rpmsg_tty.c tty: rpmsg: Fix race condition releasing tty port 2022-01-26 14:50:26 +01:00
synclink_gt.c tty: synclink_gt: rename a conflicting function name 2021-09-14 10:51:37 +02:00
sysrq.c TTY / Serial driver update for 5.16-rc1 2021-11-04 09:09:37 -07:00
tty_audit.c
tty_baudrate.c tty: baudrate: Explicit usage of B0 for encoding input baudrate 2021-10-05 15:00:56 +02:00
tty_buffer.c tty: reformat kernel-doc in tty_buffer.c 2021-11-26 16:27:43 +01:00
tty_io.c tty: tty_io: Switch to vmalloc() fallback in case of TTY_NO_WRITE_SPLIT 2021-12-21 09:18:44 +01:00
tty_ioctl.c tty: remove file from n_tty_ioctl_helper 2021-09-22 16:59:13 +02:00
tty_jobctrl.c
tty_ldisc.c tty: reformat kernel-doc in tty_ldisc.c 2021-11-26 16:27:43 +01:00
tty_ldsem.c tty/ldsem: Fix syntax errors in comments 2021-12-21 09:15:49 +01:00
tty_mutex.c
tty_port.c tty: add kernel-doc for more tty_port functions 2021-11-26 16:27:43 +01:00
tty.h
ttynull.c tty: drop put_tty_driver 2021-07-27 12:17:21 +02:00
vcc.c tty: drop put_tty_driver 2021-07-27 12:17:21 +02:00