NetBSD Problem Report #56173

From www@netbsd.org  Sat May 15 04:10:58 2021
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 7AEED1A9287
	for <gnats-bugs@gnats.NetBSD.org>; Sat, 15 May 2021 04:10:58 +0000 (UTC)
Message-Id: <20210515041056.EAD881A9288@mollari.NetBSD.org>
Date: Sat, 15 May 2021 04:10:56 +0000 (UTC)
From: mforney@mforney.org
Reply-To: mforney@mforney.org
To: gnats-bugs@NetBSD.org
Subject: libcurses: reset_prog_mode() after reset_shell_mode() overwrites original TTY state
X-Send-Pr-Version: www-1.0

>Number:         56173
>Category:       lib
>Synopsis:       libcurses: reset_prog_mode() after reset_shell_mode() overwrites original TTY state
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat May 15 04:15:00 +0000 2021
>Last-Modified:  Tue Jun 01 01:05:01 +0000 2021
>Originator:     Michael Forney
>Release:        trunk
>Organization:
>Environment:
>Description:
If a curses program calls reset_shell_mode() and then reset_prog_mode() to temporarily switch to shell mode and then back to curses mode, the original TTY settings are overwritten. This results in incorrect TTY settings after the application exits.

This pattern is used in the text editor vis when you read output from a shell command.

The problem seems to be that reset_shell_mode() calls __stopwin() to save the curses mode settings and restore the shell mode settings, but reset_prog_mode() just restores the saved curses mode settings, leaving the screen in the "endwin" state. Then, when the application performs a refresh(), __restartwin() is called which saves the current (already restored) settings as the original settings, and then restores the curses mode settings again. At this point, the original settings have been overwritten with the curses mode settings, which persist even after the application calls endwin() and exits.

In other words, the following sequence of events occurs:

	reset_shell_mode()
		__stopwin()
			saved settings = current settings
			current settings = original settings
			endwin = 1
	reset_prog_mode()
		current settings = saved settings
	refresh()
		__restartwin()
			original settings = current settings
			current settings = saved settings

At this point we have original settings == current settings == curses mode settings. The original settings have been overwritten.

It appears that this behavior dates back to this commit from 2003:
https://github.com/NetBSD/src/commit/880234af7eb36a86d6adfa775327ccec592d519d#diff-3825aa201b54dbfe36d66fe3f650737571d39e3459b936691d1c955f37871d2dL199-R204

Previously, reset_shell_mode() and reset_prog_mode() were symmetrical, calling __endwin() and __restartwin() respectively. After the change, reset_shell_mode() still calls __endwin(), but reset_prog_mode() just changes the TTY settings leaving the screen in the endwin state.
>How-To-Repeat:
1. Build vis (https://github.com/martanne/vis) against NetBSD libcurses, then run it.
2. Type ":<echo hello" to read the output of a command into the buffer.
3. Exit vis with ":q!".
4. Terminal settings are now messed up.
>Fix:
I don't quite understand what problem the commit referenced above was attempting to fix, but if I revert the highlighted hunk, the problem is fixed.

diff --git a/lib/libcurses/tstp.c b/lib/libcurses/tstp.c
index f0baa999120b..ef54143a4a54 100644
--- a/lib/libcurses/tstp.c
+++ b/lib/libcurses/tstp.c
@@ -348,8 +348,8 @@ int
 reset_prog_mode(void)
 {

-	return tcsetattr(fileno(_cursesi_screen->infd), TCSASOFT | TCSADRAIN,
-			 &_cursesi_screen->save_termios) ? ERR : OK;
+	__restartwin();
+	return OK;
 }

 int

>Audit-Trail:
From: Christos Zoulas <christos@zoulas.com>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org,
 gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org
Subject: Re: lib/56173: libcurses: reset_prog_mode() after reset_shell_mode()
 overwrites original TTY state
Date: Mon, 31 May 2021 13:07:09 -0400

 --Apple-Mail=_6F304CAD-4C8E-4B9A-8419-26EA703F25C3
 Content-Transfer-Encoding: quoted-printable
 Content-Type: text/plain;
 	charset=us-ascii

 This program behaves very differently under ncurses and curses:

 include <stdio.h>
 #include <curses.h>
 #include <stdlib.h>

 int
 main(void)
 {
         printf("\r\ninitially\n");
         system("stty -a");
         initscr();
         printf("\r\nafter initscr\n");
         system("stty -a");
         reset_shell_mode();
         printf("\r\nafter reset_shell_mode\n");
         system("stty -a");
         reset_prog_mode();
         printf("\r\nafter reset_prog_mode\n");
         system("stty -a");
         endwin();
         printf("\r\nafter endwin\n");
         system("stty -a");
         return 0;
 }

 The question is who is right?

 christos

 > On May 15, 2021, at 12:15 AM, mforney@mforney.org wrote:
 >=20
 >> Number:         56173
 >> Category:       lib
 >> Synopsis:       libcurses: reset_prog_mode() after reset_shell_mode() =
 overwrites original TTY state
 >> Confidential:   no
 >> Severity:       serious
 >> Priority:       medium
 >> Responsible:    lib-bug-people
 >> State:          open
 >> Class:          sw-bug
 >> Submitter-Id:   net
 >> Arrival-Date:   Sat May 15 04:15:00 +0000 2021
 >> Originator:     Michael Forney
 >> Release:        trunk
 >> Organization:
 >> Environment:
 >> Description:
 > If a curses program calls reset_shell_mode() and then =
 reset_prog_mode() to temporarily switch to shell mode and then back to =
 curses mode, the original TTY settings are overwritten. This results in =
 incorrect TTY settings after the application exits.
 >=20
 > This pattern is used in the text editor vis when you read output from =
 a shell command.
 >=20
 > The problem seems to be that reset_shell_mode() calls __stopwin() to =
 save the curses mode settings and restore the shell mode settings, but =
 reset_prog_mode() just restores the saved curses mode settings, leaving =
 the screen in the "endwin" state. Then, when the application performs a =
 refresh(), __restartwin() is called which saves the current (already =
 restored) settings as the original settings, and then restores the =
 curses mode settings again. At this point, the original settings have =
 been overwritten with the curses mode settings, which persist even after =
 the application calls endwin() and exits.
 >=20
 > In other words, the following sequence of events occurs:
 >=20
 > 	reset_shell_mode()
 > 		__stopwin()
 > 			saved settings =3D current settings
 > 			current settings =3D original settings
 > 			endwin =3D 1
 > 	reset_prog_mode()
 > 		current settings =3D saved settings
 > 	refresh()
 > 		__restartwin()
 > 			original settings =3D current settings
 > 			current settings =3D saved settings
 >=20
 > At this point we have original settings =3D=3D current settings =3D=3D =
 curses mode settings. The original settings have been overwritten.
 >=20
 > It appears that this behavior dates back to this commit from 2003:
 > =
 https://github.com/NetBSD/src/commit/880234af7eb36a86d6adfa775327ccec592d5=
 19d#diff-3825aa201b54dbfe36d66fe3f650737571d39e3459b936691d1c955f37871d2dL=
 199-R204
 >=20
 > Previously, reset_shell_mode() and reset_prog_mode() were symmetrical, =
 calling __endwin() and __restartwin() respectively. After the change, =
 reset_shell_mode() still calls __endwin(), but reset_prog_mode() just =
 changes the TTY settings leaving the screen in the endwin state.
 >> How-To-Repeat:
 > 1. Build vis (https://github.com/martanne/vis) against NetBSD =
 libcurses, then run it.
 > 2. Type ":<echo hello" to read the output of a command into the =
 buffer.
 > 3. Exit vis with ":q!".
 > 4. Terminal settings are now messed up.
 >> Fix:
 > I don't quite understand what problem the commit referenced above was =
 attempting to fix, but if I revert the highlighted hunk, the problem is =
 fixed.
 >=20
 > diff --git a/lib/libcurses/tstp.c b/lib/libcurses/tstp.c
 > index f0baa999120b..ef54143a4a54 100644
 > --- a/lib/libcurses/tstp.c
 > +++ b/lib/libcurses/tstp.c
 > @@ -348,8 +348,8 @@ int
 > reset_prog_mode(void)
 > {
 >=20
 > -	return tcsetattr(fileno(_cursesi_screen->infd), TCSASOFT | =
 TCSADRAIN,
 > -			 &_cursesi_screen->save_termios) ? ERR : OK;
 > +	__restartwin();
 > +	return OK;
 > }
 >=20
 > int


 --Apple-Mail=_6F304CAD-4C8E-4B9A-8419-26EA703F25C3
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
 	filename=signature.asc
 Content-Type: application/pgp-signature;
 	name=signature.asc
 Content-Description: Message signed with OpenPGP

 -----BEGIN PGP SIGNATURE-----
 Comment: GPGTools - http://gpgtools.org

 iF0EARECAB0WIQS+BJlbqPkO0MDBdsRxESqxbLM7OgUCYLUXvQAKCRBxESqxbLM7
 OgDzAJ4ulhf4OsPZOcIHyjSKH7O1HGY2bACg4cqhYJXyUIGVEA8bACdHAZpFM70=
 =od53
 -----END PGP SIGNATURE-----

 --Apple-Mail=_6F304CAD-4C8E-4B9A-8419-26EA703F25C3--

From: Brett Lymn <blymn@internode.on.net>
To: gnats-bugs@netbsd.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
        mforney@mforney.org
Subject: Re: lib/56173: libcurses: reset_prog_mode() after reset_shell_mode()
 overwrites original TTY state
Date: Tue, 1 Jun 2021 10:30:56 +0930

 On Mon, May 31, 2021 at 05:10:02PM +0000, Christos Zoulas wrote:
 >  
 >  The question is who is right?
 >  

 Unfortunately, I don't think it matters who is more correct - if the
 behaviour is not defined clearly in the standard then we probably should
 just follow ncurses because that is what most code expects.

 -- 
 Brett Lymn
 --
 Sent from my NetBSD device.

 "We are were wolves",
 "You mean werewolves?",
 "No we were wolves, now we are something else entirely",
 "Oh"

Home	PR Database Search