Thread (7 messages) 7 messages, 3 authors, 2015-01-19

Re: [PATCH] n_tty: Remove LINEMODE support

From: Peter Hurley <hidden>
Date: 2015-01-18 23:06:53
Also in: lkml

On 01/18/2015 05:44 PM, Howard Chu wrote:
Peter Hurley wrote:
quoted
Hi Howard,

On 01/18/2015 05:09 PM, Howard Chu wrote:
quoted
Peter Hurley wrote:
quoted
Commit 26df6d13406d1 ("tty: Add EXTPROC support for LINEMODE") added
the undocumented EXTPROC input processing mode, which ignores the ICANON
setting and forces pty slave input to be processed in non-canonical
mode.

Although intended to provide a transparent mechanism for local line
edit with telnetd (and other remote shell protocols), the transparency
is limited.

Userspace usage is abandoned; telnetd does not even compile with
LINEMODE support. readline/bash and sshd never supported this.
I object to this. Code for all of the above exists and works. I use this code daily.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585527
http://lists.gnu.org/archive/html/bug-readline/2011-01/msg00004.html
https://github.com/hyc/OpenSSH-LINEMODE

The lack of LINEMODE support in upstream sshd can only be considered a security hole.

http://www.metzdowd.com/pipermail/cryptography/2015-January/024288.html
These are all bug reports about userspace _not_ supporting this extension.
Bug reports *with working patches* attached. And the fact remains that not supporting this feature *is* a security liability.
Sure, I get that, but mainline kernel is not for one-off features.
I have huge patchsets for debugging kernel code that I don't foist
off on mainline, and that I constantly have to rebase.

quoted
Where is a working userspace consumer of this interface?
The OpenSSH fork on github is a full working client and server using this interface.
Ok, I can look into that.

But I'm concerned the expectation is that your personal fork of OpenSSH is
basically defining the terminal driver requirements right now.

Missing documentation really hurts too, because I can't even write
unit tests to this. Pointing at userspace patches is not documentation.
Look at man termios and man tty_ioctl.

quoted
I seriously doubt this works reliably.
What happens when the pty slave reader is in canonical mode and gets unterminated
input because only a portion of the input is available yet? The way this is
coded does _not_ require line termination before returning data to userspace.
Userspace already has to deal with incomplete lines if the input line is longer than the input buffer.
That's what I mean about not reliable:

	int n;
	char buffer[8192];

	n = read(tty, buffer, sizeof(buffer));

This had better contain a newline in canonical mode: in EXTPROC mode
the reader does not get that guarantee.

quoted
Also, ioctl(FIONREAD) doesn't match what read() returns, nor that poll()/select()
indicated input was available.
Hm, I think you're mistaken about poll/select.

    if ((!ldata->icanon && (ldata->read_cnt >= tty->minimum_to_wake)) ||
        L_EXTPROC(tty)) {
        kill_fasync(&tty->fasync, SIGIO, POLL_IN);
        if (waitqueue_active(&tty->read_wait))
            wake_up_interruptible(&tty->read_wait);
    }
That's not the code for poll()/select(): that's the input worker which wakes up
waiters on the read_wait queue, which is n_tty_read() and/or n_tty_poll().
Compare the input_available_p() logic with n_tty_ioctl(TIOCINQ).

Regards,
Peter Hurley
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help