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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YgBPNYhs/H0DwePH@cello>
Date:   Mon, 7 Feb 2022 01:44:05 +0300
From:   Arseny Maslennikov <ar@...msu.ru>
To:     Walt Drummond <walt@...mmond.us>
Cc:     agordeev@...ux.ibm.com, arnd@...db.de, benh@...nel.crashing.org,
        borntraeger@...ibm.com, chris@...kel.net, davem@...emloft.net,
        gregkh@...uxfoundation.org, hca@...ux.ibm.com, deller@....de,
        ink@...assic.park.msu.ru, James.Bottomley@...senpartnership.com,
        jirislaby@...nel.org, mattst88@...il.com, jcmvbkbc@...il.com,
        mpe@...erman.id.au, paulus@...ba.org, rth@...ddle.net,
        dalias@...c.org, tsbogend@...ha.franken.de, gor@...ux.ibm.com,
        ysato@...rs.osdn.me, linux-kernel@...r.kernel.org,
        linux-alpha@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-ia64@...r.kernel.org, linux-mips@...r.kernel.org,
        linux-parisc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
        linux-s390@...r.kernel.org, linux-sh@...r.kernel.org,
        linux-xtensa@...ux-xtensa.org, sparclinux@...r.kernel.org
Subject: Re: [PATCH v2 3/3] vstatus: Display an informational message when
 the VSTATUS character is pressed or TIOCSTAT ioctl is called.

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?

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)