Skip to content

Commit

Permalink
tty: n_tty: Restore EOF push handling behavior
Browse files Browse the repository at this point in the history
[ Upstream commit 65a8b28 ]

TTYs in ICANON mode have a special case that allows "pushing" a line
without a regular EOL character (like newline), by using EOF (the EOT
character - ASCII 0x4) as a pseudo-EOL. It is silently discarded, so
the reader of the PTS will receive the line *without* EOF or any other
terminating character.

This special case has an edge case: What happens if the readers buffer
is the same size as the line (without EOF)? Will they be able to tell
if the whole line is received, i.e. if the next read() will return more
of the same line or the next line?

There are two possibilities,  that both have (dis)advantages:

1. The next read() returns 0. FreeBSD (13.0) and OSX (10.11) do this.
   Advantage: The reader can interpret this as "the line is over".
   Disadvantage: read() returning 0 means EOF, the reader could also
   interpret it as "there's no more data" and stop reading or even
   close the PT.

2. The next read() returns the next line, the EOF is silently discarded.
   Solaris (or at least OpenIndiana 2021.10) does this, Linux has done
   do this since commit 40d5e09 ("n_tty: Fix EOF push handling");
   this behavior was recently broken by commit 3593030 ("tty:
   n_tty: do not look ahead for EOL character past the end of the buffer").
   Advantage: read() won't return 0 (EOF), reader less likely to be
   confused (and things like `while(read(..)>0)` don't break)
   Disadvantage: The reader can't really know if the read() continues
   the last line (that filled the whole read buffer) or starts a
   new line.

As both options are defensible (and are used by other Unix-likes), it's
best to stick to the "old" behavior since "n_tty: Fix EOF push handling"
of 2013, i.e. silently discard that EOF.

This patch - that I actually got from Linus for testing and only
modified slightly - restores that behavior by skipping an EOF
character if it's the next character after reading is done.

Based on a patch from Linus Torvalds.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=215611
Fixes: 3593030 ("tty: n_tty: do not look ahead for EOL character past the end of the buffer")
Cc: Peter Hurley <peter@hurleysoftware.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Reviewed-and-tested-by: Daniel Gibson <daniel@gibson.sh>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Daniel Gibson <daniel@gibson.sh>
Link: https://lore.kernel.org/r/20220329235810.452513-2-daniel@gibson.sh
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Daniel Gibson authored and gregkh committed Jun 14, 2022
1 parent f307bdb commit c84fa72
Showing 1 changed file with 37 additions and 1 deletion.
38 changes: 37 additions & 1 deletion drivers/tty/n_tty.c
Expand Up @@ -2012,6 +2012,35 @@ static bool canon_copy_from_read_buf(struct tty_struct *tty,
return ldata->read_tail != canon_head;
}

/*
* If we finished a read at the exact location of an
* EOF (special EOL character that's a __DISABLED_CHAR)
* in the stream, silently eat the EOF.
*/
static void canon_skip_eof(struct tty_struct *tty)
{
struct n_tty_data *ldata = tty->disc_data;
size_t tail, canon_head;

canon_head = smp_load_acquire(&ldata->canon_head);
tail = ldata->read_tail;

// No data?
if (tail == canon_head)
return;

// See if the tail position is EOF in the circular buffer
tail &= (N_TTY_BUF_SIZE - 1);
if (!test_bit(tail, ldata->read_flags))
return;
if (read_buf(ldata, tail) != __DISABLED_CHAR)
return;

// Clear the EOL bit, skip the EOF char.
clear_bit(tail, ldata->read_flags);
smp_store_release(&ldata->read_tail, ldata->read_tail + 1);
}

/**
* job_control - check job control
* @tty: tty
Expand Down Expand Up @@ -2081,7 +2110,14 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
*/
if (*cookie) {
if (ldata->icanon && !L_EXTPROC(tty)) {
if (canon_copy_from_read_buf(tty, &kb, &nr))
/*
* If we have filled the user buffer, see
* if we should skip an EOF character before
* releasing the lock and returning done.
*/
if (!nr)
canon_skip_eof(tty);
else if (canon_copy_from_read_buf(tty, &kb, &nr))
return kb - kbuf;
} else {
if (copy_from_read_buf(tty, &kb, &nr))
Expand Down

0 comments on commit c84fa72

Please sign in to comment.