Received-SPF: pass (google.com: domain of linux-kernel+bounces-178756-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Tue, 14 May 2024 16:30:41 +0300 (EEST)
To: Vadym Krevs <vkrevs@yahoo.com>
cc: Andy Shevchenko <andy.shevchenko@gmail.com>, 
    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
In-Reply-To: <645803059.1237606.1715692880298@mail.yahoo.com>
Message-ID: <386fae3e-5286-e13d-4171-3a6e87cb1567@linux.intel.com>
References: <ZkLv3wn62jqMVpSF@archie.me> <CAHp75VcjeJ5oZy2JjbyrKn+GVvixJKmfh3Yj1p-HxcJHrm0Wzg@mail.gmail.com> <606328522.1205749.1715678929830@mail.yahoo.com> <CAHp75VefMFJMAHrKmDf1joSkKOcaRMJWse8yM63Nt5v2W7w3Fw@mail.gmail.com> <6bfafddc-cf5b-4829-e09b-2622aaf83093@linux.intel.com>
 <645803059.1237606.1715692880298@mail.yahoo.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323328-191811837-1715693441=:6875"

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-191811837-1715693441=:6875
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Tue, 14 May 2024, Vadym Krevs wrote:

> On Tuesday, 14 May 2024 at 12:03:25 BST, Ilpo J=C3=A4rvinen <ilpo.jarvine=
n@linux.intel.com> wrote:
> =C2=A0
> > On Tue, 14 May 2024, Andy Shevchenko wrote:
> >=C2=A0
> > > On Tue, May 14, 2024 at 12:28=E2=80=AFPM Vadym Krevs <vkrevs@yahoo.co=
m> wrote:
> > > >
> > > > It's a standard setup for an out-of-the box default install of open=
SUSE 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 =3D 0;
> > > > intr =3D ^C; quit =3D ^; erase =3D ^?; kill =3D ^U; eof =3D ^D; eol=
 =3D <undef>; eol2 =3D <undef>; swtch =3D <undef>; start =3D ^Q; stop =3D ^=
S; susp =3D ^Z; rprnt =3D ^R; werase =3D ^W; lnext =3D ^V; discard =3D ^O; =
min =3D 1; time =3D 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.
> >=C2=A0
> > 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.
> >=C2=A0
> > Could you please test and confirm the patch below fixes the issue?
> > --
> > [PATCH] tty: n_tty: Fix buffer offsets when looked ahead is used
> >=C2=A0
> > 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.
> >=C2=A0
> > 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).
> >=C2=A0
> > Reported-by: Vadym Krevs <vkrevs@yahoo.com>
> > Fixes: 6bb6fa6908eb ("tty: Implement lookahead to process XON/XOFF time=
ly")
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=3D218834
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Ilpo J=C3=A4rvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> > drivers/tty/n_tty.c | 22 ++++++++++++++++------
> > 1 file changed, 16 insertions(+), 6 deletions(-)
> >=C2=A0
> > 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 *tt=
y, 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)) {
> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (la_count > 0)
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (la_count > 0) {
> > n_tty_receive_buf_closing(tty, cp, fp, la_count, true);
> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (count > la_count)
> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 n_tty_receive_buf_closing(tt=
y, cp, fp, count - la_count, false);
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 cp +=3D la_count;
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (fp)
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fp +=3D la_cou=
nt;
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 count -=3D la_count;
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (count > 0)
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 n_tty_receive_buf_closing(tt=
y, cp, fp, count, false);
> > } else {
> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (la_count > 0)
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (la_count > 0) {
> > n_tty_receive_buf_standard(tty, cp, fp, la_count, true);
> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (count > la_count)
> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 n_tty_receive_buf_standard(t=
ty, cp, fp, count - la_count, false);
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 cp +=3D la_count;
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (fp)
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fp +=3D la_cou=
nt;
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 count -=3D la_count;
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (count > 0)
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 n_tty_receive_buf_standard(t=
ty, cp, fp, count, false);
> >=C2=A0
> > flush_echoes(tty);
> > if (tty->ops->flush_chars)
> > --
> > 2.39.2
>=20
> Yes, I've tested the patch against the 6.9.0-rc7-local-00012-gdccb07f2914=
c kernel (last commit=C2=A045db3ab70092637967967bfd8e6144017638563c from Ma=
y 8th)=C2=A0and it works just fine.=C2=A0
>=20
> Thank you very much for fixing the problem so quicky.
>=20
> Kind regards,
> Vadym
>=20
> P.S.: Hopefully, Yahoo mail has actually sent this reply as plain text.

Thanks for testing.

Can I put your Tested-by tag into the fix?

--=20
 i.

--8323328-191811837-1715693441=:6875--