From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Andy Shevchenko <andy.shevchenko@gmail.com>,
Vadym Krevs <vkrevs@yahoo.com>
Cc: Bagas Sanjaya <bagasdotme@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux Regressions <regressions@lists.linux.dev>,
Linux Serial <linux-serial@vger.kernel.org>,
Gilles Buloz <gilles.buloz@kontron.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>
Subject: Re: [regression] [bisected] commit 6bb6fa6908ebd3cb4e14cd4f0ce272ec885d2eb0 corrupts data sent via pseudoterminal device
Date: Tue, 14 May 2024 14:03:05 +0300 (EEST) [thread overview]
Message-ID: <6bfafddc-cf5b-4829-e09b-2622aaf83093@linux.intel.com> (raw)
In-Reply-To: <CAHp75VefMFJMAHrKmDf1joSkKOcaRMJWse8yM63Nt5v2W7w3Fw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3516 bytes --]
On Tue, 14 May 2024, Andy Shevchenko wrote:
> On Tue, May 14, 2024 at 12:28 PM Vadym Krevs <vkrevs@yahoo.com> wrote:
> >
> > It's a standard setup for an out-of-the box default install of openSUSE 15.5 with KDE. All tests done in Konsole with bash as shell.
> >
> > stty -a -F /dev/pts/1
> > speed 38400 baud; rows 57; columns 217; line = 0;
> > intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>; eol2 = <undef>; swtch = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R; werase = ^W; lnext = ^V; discard = ^O; min = 1; time = 0;
> > -parenb -parodd -cmspar cs8 -hupcl -cstopb cread -clocal -crtscts
> > -ignbrk -brkint -ignpar -parmrk -inpck -istrip -inlcr -igncr icrnl ixon ixoff -iuclc -ixany -imaxbel iutf8
> > opost -olcuc -ocrnl onlcr -onocr -onlret -ofill -ofdel nl0 cr0 tab0 bs0 vt0 ff0
> > isig icanon iexten echo echoe echok -echonl -noflsh -xcase -tostop -echoprt echoctl echoke -flusho -extproc
>
> Thank you!
>
> Yeah. SW flow control is enabled, but I don't see which character is
> being used for that. Anyway, let's give Ilpo a chance to look into
> this.
Thanks a lot for pinpointing the commit with bisect. It turns out this
is a quite bad corruption bug and I'm quite surprised I didn't see (or
notice) it while testing the patch.
Could you please test and confirm the patch below fixes the issue?
--
[PATCH] tty: n_tty: Fix buffer offsets when looked ahead is used
When lookahead has "consumed" some characters (la_count > 0),
n_tty_receive_buf_standard() and n_tty_receive_buf_closing() for
characters beyond the la_count are given wrong cp/fp offsets which
leads to duplicating and losing some characters.
If la_count > 0, correct buffer pointers and make count consistency too
(the latter is not strictly necessary to fix the issue but seems more
logical to adjust all variables immediately to keep state consistent).
Reported-by: Vadym Krevs <vkrevs@yahoo.com>
Fixes: 6bb6fa6908eb ("tty: Implement lookahead to process XON/XOFF timely")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218834
Cc: stable@vger.kernel.org
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/tty/n_tty.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index f252d0b5a434..5e9ca4376d68 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1619,15 +1619,25 @@ static void __receive_buf(struct tty_struct *tty, const u8 *cp, const u8 *fp,
else if (ldata->raw || (L_EXTPROC(tty) && !preops))
n_tty_receive_buf_raw(tty, cp, fp, count);
else if (tty->closing && !L_EXTPROC(tty)) {
- if (la_count > 0)
+ if (la_count > 0) {
n_tty_receive_buf_closing(tty, cp, fp, la_count, true);
- if (count > la_count)
- n_tty_receive_buf_closing(tty, cp, fp, count - la_count, false);
+ cp += la_count;
+ if (fp)
+ fp += la_count;
+ count -= la_count;
+ }
+ if (count > 0)
+ n_tty_receive_buf_closing(tty, cp, fp, count, false);
} else {
- if (la_count > 0)
+ if (la_count > 0) {
n_tty_receive_buf_standard(tty, cp, fp, la_count, true);
- if (count > la_count)
- n_tty_receive_buf_standard(tty, cp, fp, count - la_count, false);
+ cp += la_count;
+ if (fp)
+ fp += la_count;
+ count -= la_count;
+ }
+ if (count > 0)
+ n_tty_receive_buf_standard(tty, cp, fp, count, false);
flush_echoes(tty);
if (tty->ops->flush_chars)
--
2.39.2
next prev parent reply other threads:[~2024-05-14 11:03 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 5:00 Fwd: [regression] [bisected] commit 6bb6fa6908ebd3cb4e14cd4f0ce272ec885d2eb0 corrupts data sent via pseudoterminal device Bagas Sanjaya
2024-05-14 9:10 ` Andy Shevchenko
[not found] ` <606328522.1205749.1715678929830@mail.yahoo.com>
2024-05-14 9:33 ` Andy Shevchenko
2024-05-14 11:03 ` Ilpo Järvinen [this message]
2024-05-14 13:21 ` Vadym Krevs
2024-05-14 13:29 ` Andy Shevchenko
2024-05-14 13:30 ` Ilpo Järvinen
2024-05-14 13:51 ` Vadym Krevs
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6bfafddc-cf5b-4829-e09b-2622aaf83093@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=andy.shevchenko@gmail.com \
--cc=bagasdotme@gmail.com \
--cc=gilles.buloz@kontron.com \
--cc=gregkh@linuxfoundation.org \
--cc=jirislaby@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=regressions@lists.linux.dev \
--cc=vkrevs@yahoo.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).