[LTP] [PATCH] pty/pty02: new test for hang involving EXTPROC|ICANON terminal mode

[LTP] [PATCH] pty/pty02: new test for hang involving EXTPROC|ICANON terminal mode

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Test for a bug in the kernel's tty layer where an infinite loop could
occur if the EXTPROC and ICANON terminal modes were combined.  This bug
was found by the syzkaller fuzzer and was fixed in v4.15.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 runtest/pty                     |  1 +
 testcases/kernel/pty/.gitignore |  1 +
 testcases/kernel/pty/pty02.c    | 58 +++++++++++++++++++++++++++++++++
 3 files changed, 60 insertions(+)
 create mode 100644 testcases/kernel/pty/pty02.c

diff --git a/runtest/pty b/runtest/pty
index 92c89ab31..52e2c07f1 100644
--- a/runtest/pty
+++ b/runtest/pty
@@ -1,5 +1,6 @@
 #DESCRIPTION:Terminal type stress
 pty01 pty01
+pty02 pty02
 ptem01 ptem01
 hangup01 hangup01
 
diff --git a/testcases/kernel/pty/.gitignore b/testcases/kernel/pty/.gitignore
index db16401dc..131b8a077 100644
--- a/testcases/kernel/pty/.gitignore
+++ b/testcases/kernel/pty/.gitignore
@@ -1,3 +1,4 @@
 /hangup01
 /ptem01
 /pty01
+/pty02
diff --git a/testcases/kernel/pty/pty02.c b/testcases/kernel/pty/pty02.c
new file mode 100644
index 000000000..d96eec211
--- /dev/null
+++ b/testcases/kernel/pty/pty02.c
@@ -0,0 +1,58 @@
+/*
+ * Copyright (c) 2018 Google, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program, if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Regression test for commit 966031f340185 ("n_tty: fix EXTPROC vs ICANON
+ * interaction with TIOCINQ (aka FIONREAD)").  The test reproduces a hang
+ * (infinite loop in the kernel) after a pseudoterminal is put in both canonical
+ * (ICANON) and external processing (EXTPROC) mode, some data is written to the
+ * master and read from the slave, and the FIONREAD ioctl is called on the
+ * slave.  This is simplified from a syzkaller-generated reproducer.
+ */
+
+#include <stdlib.h>
+#include <termio.h>
+
+#include "tst_test.h"
+
+static void do_test(void)
+{
+	struct termios io = { .c_lflag = EXTPROC | ICANON };
+	int ptmx, pts;
+	char c = 'A';
+	int nbytes;
+
+	ptmx = SAFE_OPEN("/dev/ptmx", O_WRONLY);
+
+	if (tcsetattr(ptmx, TCSANOW, &io) != 0)
+		tst_brk(TBROK | TERRNO, "tcsetattr() failed");
+
+	if (unlockpt(ptmx) != 0)
+		tst_brk(TBROK | TERRNO, "unlockpt() failed");
+
+	pts = SAFE_OPEN(ptsname(ptmx), O_RDONLY);
+	SAFE_WRITE(1, ptmx, &c, 1);
+	SAFE_READ(1, pts, &c, 1);
+
+	tst_res(TINFO, "Calling FIONREAD, this will hang in n_tty_ioctl() if the bug is present...");
+	SAFE_IOCTL(pts, FIONREAD, &nbytes);
+	tst_res(TPASS, "Got to the end without hanging");
+}
+
+static struct tst_test test = {
+	.test_all = do_test,
+};
-- 
2.17.0

[LTP] [PATCH] pty/pty02: new test for hang involving EXTPROC|ICANON terminal mode

[Cyril Hrubis]

Hi!
> +	tst_res(TINFO, "Calling FIONREAD, this will hang in n_tty_ioctl() if the bug is present...");
> +	SAFE_IOCTL(pts, FIONREAD, &nbytes);

I've added two SAFE_CLOSE() here so that the test works with the -i 1000
as well and pushed, thanks.

> +	tst_res(TPASS, "Got to the end without hanging");
> +}

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] pty/pty02: new test for hang involving EXTPROC|ICANON terminal mode

[Jan Stancek]

----- Original Message -----
> + * Regression test for commit 966031f340185 ("n_tty: fix EXTPROC vs ICANON
> + * interaction with TIOCINQ (aka FIONREAD)").  The test reproduces a hang
> + * (infinite loop in the kernel) after a pseudoterminal is put in both
> canonical
> + * (ICANON) and external processing (EXTPROC) mode, some data is written to
> the
> + * master and read from the slave, and the FIONREAD ioctl is called on the
> + * slave.  This is simplified from a syzkaller-generated reproducer.
> + */
> +
> +#include <stdlib.h>
> +#include <termio.h>
> +
> +#include "tst_test.h"
> +
> +static void do_test(void)
> +{
> +	struct termios io = { .c_lflag = EXTPROC | ICANON };

Hi,

I'm running into compilation errors on older distros (RHEL5/6)
with this test:

pty02.c: In function ‘do_test’:
pty02.c:34: error: ‘EXTPROC’ undeclared (first use in this function)
pty02.c:34: error: (Each undeclared identifier is reported only once
pty02.c:34: error: for each function it appears in.)
make: *** [pty02] Error 1

We should probably ifdef the test, because adding define to LAPI
still makes it fail:

tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s
pty02.c:44: BROK: tcsetattr() failed: EINVAL

Regards,
Jan

[LTP] [PATCH] pty/pty02: new test for hang involving EXTPROC|ICANON terminal mode

[Li Wang]

Hi Jan,

On Thu, May 31, 2018 at 9:35 PM, Jan Stancek <jstancek@redhat.com> wrote:

>
>
> ----- Original Message -----
> > + * Regression test for commit 966031f340185 ("n_tty: fix EXTPROC vs
> ICANON
> > + * interaction with TIOCINQ (aka FIONREAD)").  The test reproduces a
> hang
> > + * (infinite loop in the kernel) after a pseudoterminal is put in both
> > canonical
> > + * (ICANON) and external processing (EXTPROC) mode, some data is
> written to
> > the
> > + * master and read from the slave, and the FIONREAD ioctl is called on
> the
> > + * slave.  This is simplified from a syzkaller-generated reproducer.
> > + */
> > +
> > +#include <stdlib.h>
> > +#include <termio.h>
> > +
> > +#include "tst_test.h"
> > +
> > +static void do_test(void)
> > +{
> > +     struct termios io = { .c_lflag = EXTPROC | ICANON };
>
> Hi,
>
> I'm running into compilation errors on older distros (RHEL5/6)
> with this test:
>
> pty02.c: In function ‘do_test’:
> pty02.c:34: error: ‘EXTPROC’ undeclared (first use in this function)
> pty02.c:34: error: (Each undeclared identifier is reported only once
> pty02.c:34: error: for each function it appears in.)
> make: *** [pty02] Error 1
>
> We should probably ifdef the test, because adding define to LAPI
> still makes it fail:
>


​Xiao and I have also noticed this, ​beside the undefine issue, there are
still
other problems in pty02 test.

The discuss is here:
http://lists.linux.it/pipermail/ltp/2018-May/008253.html



>
> tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s
> pty02.c:44: BROK: tcsetattr() failed: EINVAL
>
> Regards,
> Jan
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>



-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180601/bb2bbfb8/attachment-0001.html>

[LTP] [PATCH] pty/pty02: new test for hang involving EXTPROC|ICANON terminal mode

[Cyril Hrubis]

Hi!
> I'm running into compilation errors on older distros (RHEL5/6)
> with this test:
> 
> pty02.c: In function ???do_test???:
> pty02.c:34: error: ???EXTPROC??? undeclared (first use in this function)
> pty02.c:34: error: (Each undeclared identifier is reported only once
> pty02.c:34: error: for each function it appears in.)
> make: *** [pty02] Error 1
> 
> We should probably ifdef the test, because adding define to LAPI
> still makes it fail:
> 
> tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s
> pty02.c:44: BROK: tcsetattr() failed: EINVAL

Or we can return TCONF on EINVAL since EINVAL means unsupported value
anyway...

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] pty/pty02: new test for hang involving EXTPROC|ICANON terminal mode

[Li Wang]

On Mon, Jun 4, 2018 at 7:40 PM, Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > I'm running into compilation errors on older distros (RHEL5/6)
> > with this test:
> >
> > pty02.c: In function ???do_test???:
> > pty02.c:34: error: ???EXTPROC??? undeclared (first use in this function)
> > pty02.c:34: error: (Each undeclared identifier is reported only once
> > pty02.c:34: error: for each function it appears in.)
> > make: *** [pty02] Error 1
> >
> > We should probably ifdef the test, because adding define to LAPI
> > still makes it fail:
> >
> > tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s
> > pty02.c:44: BROK: tcsetattr() failed: EINVAL
>
> Or we can return TCONF on EINVAL since EINVAL means unsupported value
> anyway...
>

​Maybe at least we should do more try before skipping it when getting
EINVAL. Because sometimes it also fails with EINVAL when using a termios
structure that was not obtained using tcgeaatr(). tcsetattr() should use
only a termios structure that was obtained by tcgetattr().

 pty02.c works fine on mainline kernel-v4.17 after applying this:
  http://lists.linux.it/pipermail/ltp/2018-May/008253.html

What do you think?

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20180605/936fccfa/attachment.html>