[LTP] [PATCH 1/2] pty07: Resize console to the same size as before

[LTP] [PATCH 1/2] pty07: Resize console to the same size as before

[Martin Doucha]

Resizing console to 1x1 breaks some QA automation systems. Keep the original
console dimensions throughout the test.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/pty/pty07.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/pty/pty07.c b/testcases/kernel/pty/pty07.c
index c63b71c89..ae600b692 100644
--- a/testcases/kernel/pty/pty07.c
+++ b/testcases/kernel/pty/pty07.c
@@ -40,6 +40,7 @@ static int test_tty_port = 8;
 static int fd = -1;
 static struct tst_fzsync_pair fzp;
 
+static struct vt_consize consize;
 static unsigned short vt_active;
 
 static void *open_close(void *unused)
@@ -60,13 +61,12 @@ static void *open_close(void *unused)
 
 static void do_test(void)
 {
-	struct vt_consize sz = { 0x1, 0x1, 0x1, 0x1, 0x1, 0x1 };
 
 	tst_fzsync_pair_reset(&fzp, open_close);
 
 	while (tst_fzsync_run_a(&fzp)) {
 		tst_fzsync_start_race_a(&fzp);
-		ioctl(fd, VT_RESIZEX, &sz);
+		ioctl(fd, VT_RESIZEX, &consize);
 		tst_fzsync_end_race_a(&fzp);
 		if (tst_taint_check()) {
 			tst_res(TFAIL, "Kernel is buggy");
@@ -79,6 +79,7 @@ static void do_test(void)
 static void setup(void)
 {
 	struct vt_stat stat;
+	struct winsize wsize;
 
 	sprintf(tty_path, "/dev/tty%d", test_tty_port);
 	if (access(tty_path, F_OK))
@@ -90,6 +91,9 @@ static void setup(void)
 
 	tst_res(TINFO, "Saving active console %i", vt_active);
 
+	SAFE_IOCTL(fd, TIOCGWINSZ, &wsize);
+	consize.v_rows = wsize.ws_row;
+	consize.v_cols = wsize.ws_col;
 	tst_fzsync_pair_init(&fzp);
 }
 
-- 
2.36.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/2] pty04: Fix cleanup

[Martin Doucha]

If pty04 gets executed with -i0 argument, the sk file descriptor will
stay zero and cleanup() will close stdin, which can result in the parent
shell logging out. Initialize global file descriptors to -1 and only
clean up those that were left open by the test.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/pty/pty04.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/pty/pty04.c b/testcases/kernel/pty/pty04.c
index 00b714c82..c2510d8fe 100644
--- a/testcases/kernel/pty/pty04.c
+++ b/testcases/kernel/pty/pty04.c
@@ -92,7 +92,7 @@ static struct ldisc_info ldiscs[] = {
 	{N_SLCAN, "N_SLCAN", CAN_MTU},
 };
 
-static int ptmx, pts, sk, mtu, no_check;
+static int ptmx = -1, pts = -1, sk = -1, mtu, no_check;
 
 static int set_ldisc(int tty, const struct ldisc_info *ldisc)
 {
@@ -455,9 +455,14 @@ static void do_test(unsigned int n)
 
 static void cleanup(void)
 {
-	ioctl(pts, TIOCVHANGUP);
-	ioctl(ptmx, TIOCVHANGUP);
-	close(sk);
+	if (pts >= 0)
+		ioctl(pts, TIOCVHANGUP);
+
+	if (ptmx >= 0)
+		ioctl(ptmx, TIOCVHANGUP);
+
+	if (sk >= 0)
+		close(sk);
 
 	tst_reap_children();
 }
-- 
2.36.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] pty04: Fix cleanup

[Petr Vorel]

Hi Martin, Li, Cyril,

> If pty04 gets executed with -i0 argument, the sk file descriptor will
> stay zero and cleanup() will close stdin, which can result in the parent
> shell logging out. Initialize global file descriptors to -1 and only
> clean up those that were left open by the test.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Thanks a lot. As and obvious fix I'm going to merge it later today.

BTW C API would deserve (after the release) TBROK on -i0, which is already
implemented in shell API:
df01 1 TBROK: Number of iterations (-i) must be > 0

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] pty04: Fix cleanup

[Martin Doucha]

On 20. 05. 22 11:01, Petr Vorel wrote:
> BTW C API would deserve (after the release) TBROK on -i0, which is already
> implemented in shell API:
> df01 1 TBROK: Number of iterations (-i) must be > 0

I don't see why. Running pty07 with -i0 was actually quite helpful in
debugging this issue. If a test fails when executed with -i0, it needs
to be fixed.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] pty04: Fix cleanup

[Petr Vorel]

> On 20. 05. 22 11:01, Petr Vorel wrote:
> > BTW C API would deserve (after the release) TBROK on -i0, which is already
> > implemented in shell API:
> > df01 1 TBROK: Number of iterations (-i) must be > 0

> I don't see why. Running pty07 with -i0 was actually quite helpful in
> debugging this issue. If a test fails when executed with -i0, it needs
> to be fixed.
Do you mean that -i0 is useful to test only setup and cleanup in C API?
If we agree we want this (not against it if it's useful), shell API should
be unified with it (my concern was that both APIs should behave the same on
the same getopt option).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] pty04: Fix cleanup

[Cyril Hrubis]

Hi!
> > I don't see why. Running pty07 with -i0 was actually quite helpful in
> > debugging this issue. If a test fails when executed with -i0, it needs
> > to be fixed.
> Do you mean that -i0 is useful to test only setup and cleanup in C API?
> If we agree we want this (not against it if it's useful), shell API should
> be unified with it (my concern was that both APIs should behave the same on
> the same getopt option).

It never occured to me that -i0 can be used to test that setup and
cleanup are coded properly but it makes a lot of sense.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] pty04: Fix cleanup

[Petr Vorel]

> Hi!
> > > I don't see why. Running pty07 with -i0 was actually quite helpful in
> > > debugging this issue. If a test fails when executed with -i0, it needs
> > > to be fixed.
> > Do you mean that -i0 is useful to test only setup and cleanup in C API?
> > If we agree we want this (not against it if it's useful), shell API should
> > be unified with it (my concern was that both APIs should behave the same on
> > the same getopt option).

> It never occured to me that -i0 can be used to test that setup and
> cleanup are coded properly but it makes a lot of sense.

OK, I might send a patch for shell implementing it, but it has a lower priority
(there are other patches which should make it to get into the release than
this).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] pty04: Fix cleanup

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/2] pty04: Fix cleanup

[Petr Vorel]

Hi all,

merged this one, thanks Martin!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] pty07: Resize console to the same size as before

[Petr Vorel]

Hi all,

> Resizing console to 1x1 breaks some QA automation systems. Keep the original
> console dimensions throughout the test.

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Martin, thanks a lot for fixing.

LGTM. If you don't mind, I'd merge this patchset later today to get into the
release.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] pty07: Resize console to the same size as before

[Cyril Hrubis]

Hi!
> Resizing console to 1x1 breaks some QA automation systems. Keep the original
> console dimensions throughout the test.

Good catch.

I guess that the actual size we pass to VT_RESIZEX does not matter when
we attempt to trigger the race.

Reviwed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] pty07: Resize console to the same size as before

[Martin Doucha]

On 20. 05. 22 11:26, Cyril Hrubis wrote:
> I guess that the actual size we pass to VT_RESIZEX does not matter when
> we attempt to trigger the race.

I've tested this on SLE-15SP1 GM kernel which is older than the fix and
this version still triggers the crash. Technically we could even pass
all zeroes which would simply leave all console parameters unchanged and
it'd still work because the ioctl implementation sets a flag in the
freed memory anyway. But that behavior might change in the future so
I've decided to just set the rows and cols to the values queried from
kernel.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/2] pty07: Resize console to the same size as before

[Petr Vorel]

> On 20. 05. 22 11:26, Cyril Hrubis wrote:
> > I guess that the actual size we pass to VT_RESIZEX does not matter when
> > we attempt to trigger the race.

> I've tested this on SLE-15SP1 GM kernel which is older than the fix and
> this version still triggers the crash. Technically we could even pass
> all zeroes which would simply leave all console parameters unchanged and
> it'd still work because the ioctl implementation sets a flag in the
> freed memory anyway. But that behavior might change in the future so
> I've decided to just set the rows and cols to the values queried from
> kernel.

+1. Merged this one, thank you!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp