Re: [PATCH v2 3/3] vstatus: Display an informational message when the VSTATUS character is pressed or TIOCSTAT ioctl is called.

[Arseny Maslennikov <ar@cs.msu.ru>]

[-- Attachment #1: Type: text/plain, Size: 3100 bytes --]

On Mon, Feb 07, 2022 at 12:25:21AM +0300, Arseny Maslennikov wrote:
> On Sun, Feb 06, 2022 at 07:48:54AM -0800, Walt Drummond wrote:
> > @@ -2430,6 +2459,11 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
> >  			retval = read_cnt(ldata);
> >  		up_write(&tty->termios_rwsem);
> >  		return put_user(retval, (unsigned int __user *) arg);
> > +	case TIOCSTAT:
> 
> Perhaps we want to guard this (example pseudocode follows):
> 
> 		if (*our ldisc is not n_tty*)
> 			return an error like -ENOTTY;
> 
> ...since kerninfo is useless for non-UI ttys, e. g. serial device
> drivers, and this ioctl could mess them up if this code path can be
> taken. (I have not verified this kind of breakage is possible.) Please
> see the complete rationale below, this paragraph is an illustrational
> note for it.

Oh wait, this *is* n_tty_ioctl(), so the ioctl is n_tty-specific. This
makes the case below even clearer.
I've been clumsy, sorry about that.

> > diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
> > index 6616d4a0d41d..f2f4f48ea502 100644
> > --- a/drivers/tty/tty_io.c
> > +++ b/drivers/tty/tty_io.c
> > @@ -125,7 +125,7 @@ struct ktermios tty_std_termios = {	/* for the benefit of tty drivers  */
> >  	.c_oflag = OPOST | ONLCR,
> >  	.c_cflag = B38400 | CS8 | CREAD | HUPCL,
> >  	.c_lflag = ISIG | ICANON | ECHO | ECHOE | ECHOK |
> > -		   ECHOCTL | ECHOKE | IEXTEN,
> > +		   ECHOCTL | ECHOKE | IEXTEN | NOKERNINFO,
> 
> Does this mean that nokerninfo is on by default? Do we have a reason to
> do that?
> 
> As of this patch we require icanon and iexten to be set for the message
> to be composed and printed. An experiment shows PTY encapsulation
> programs like openssh turn off both those flags on the tty they run on
> before they take control (contrary to what has been said in LWN), so
> they are unimpacted.
> 
> The termios(3) page from man-pages states:
>    Raw mode
>        cfmakeraw() sets the terminal to something like the "raw"  mode
>        of  the old Version 7 terminal driver: input is available char‐
>        acter by character, echoing is disabled, and all  special  pro‐
>        cessing  of  terminal  input and output characters is disabled.
>        The terminal attributes are set as follows:
> 
>            termios_p->c_iflag &= ~(IGNBRK | BRKINT | PARMRK | ISTRIP
>                            | INLCR | IGNCR | ICRNL | IXON);
>            termios_p->c_oflag &= ~OPOST;
>            termios_p->c_lflag &= ~(ECHO | ECHONL | ICANON | ISIG | IEXTEN);
>            termios_p->c_cflag &= ~(CSIZE | PARENB);
>            termios_p->c_cflag |= CS8;
> 
> So any program which uses this API effectively turns off kerninfo as
> implemented here.
> 
> There are 2 ways n_tty_status() can be called as of this patch: either
> from inside n_tty or via TIOCSTAT. The first path can't be taken on ttys
> whose ldisc is not N_TTY, ...

The second path is OK as well.

> Given all this, is there any other reason to enable nokerninfo (i. e.
> disable status message) by default?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]