lkml.org 
[lkml]   [2022]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.18 011/339] tty: n_tty: Restore EOF push handling behavior
    Date
    From: Daniel Gibson <daniel@gibson.sh>

    [ Upstream commit 65a8b287023da68c4550deab5c764e6891cf1caf ]

    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 40d5e0905a03 ("n_tty: Fix EOF push handling");
    this behavior was recently broken by commit 359303076163 ("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: 359303076163 ("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>
    ---
    drivers/tty/n_tty.c | 38 +++++++++++++++++++++++++++++++++++++-
    1 file changed, 37 insertions(+), 1 deletion(-)

    diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
    index efc72104c840..bdc314aeab88 100644
    --- a/drivers/tty/n_tty.c
    +++ b/drivers/tty/n_tty.c
    @@ -1975,6 +1975,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
    @@ -2045,7 +2074,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))
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-06-13 15:26    [W:4.096 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site