[PATCH] n_tty: Remove LINEMODE support

[PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

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.

Cc: Howard Chu <hyc@symas.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 22 ++++++----------------
 drivers/tty/pty.c   | 24 +-----------------------
 2 files changed, 7 insertions(+), 39 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index eb9f114..5120d2b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1562,10 +1562,6 @@ n_tty_receive_buf_standard(struct tty_struct *tty, const unsigned char *cp,
 				c &= 0x7f;
 			if (I_IUCLC(tty) && L_IEXTEN(tty))
 				c = tolower(c);
-			if (L_EXTPROC(tty)) {
-				put_tty_queue(c, ldata);
-				continue;
-			}
 			if (!test_bit(c, ldata->char_map))
 				n_tty_receive_char_inline(tty, c);
 			else if (n_tty_receive_char_special(tty, c) && count) {
@@ -1613,9 +1609,9 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 
 	if (ldata->real_raw)
 		n_tty_receive_buf_real_raw(tty, cp, fp, count);
-	else if (ldata->raw || (L_EXTPROC(tty) && !preops))
+	else if (ldata->raw)
 		n_tty_receive_buf_raw(tty, cp, fp, count);
-	else if (tty->closing && !L_EXTPROC(tty))
+	else if (tty->closing)
 		n_tty_receive_buf_closing(tty, cp, fp, count);
 	else {
 		if (ldata->lnext) {
@@ -1637,13 +1633,13 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 			tty->ops->flush_chars(tty);
 	}
 
-	if (ldata->icanon && !L_EXTPROC(tty))
+	if (ldata->icanon)
 		return;
 
 	/* publish read_head to consumer */
 	smp_store_release(&ldata->commit_head, ldata->read_head);
 
-	if ((read_cnt(ldata) >= ldata->minimum_to_wake) || L_EXTPROC(tty)) {
+	if (read_cnt(ldata) >= ldata->minimum_to_wake) {
 		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
 		if (waitqueue_active(&tty->read_wait))
 			wake_up_interruptible_poll(&tty->read_wait, POLLIN);
@@ -1939,7 +1935,7 @@ static inline int input_available_p(struct tty_struct *tty, int poll)
 	struct n_tty_data *ldata = tty->disc_data;
 	int amt = poll && !TIME_CHAR(tty) && MIN_CHAR(tty) ? MIN_CHAR(tty) : 1;
 
-	if (ldata->icanon && !L_EXTPROC(tty))
+	if (ldata->icanon)
 		return ldata->canon_head != ldata->read_tail;
 	else
 		return ldata->commit_head - ldata->read_tail >= amt;
@@ -1973,7 +1969,6 @@ static int copy_from_read_buf(struct tty_struct *tty,
 	struct n_tty_data *ldata = tty->disc_data;
 	int retval;
 	size_t n;
-	bool is_eof;
 	size_t head = smp_load_acquire(&ldata->commit_head);
 	size_t tail = ldata->read_tail & (N_TTY_BUF_SIZE - 1);
 
@@ -1983,14 +1978,9 @@ static int copy_from_read_buf(struct tty_struct *tty,
 	if (n) {
 		retval = copy_to_user(*b, read_buf_addr(ldata, tail), n);
 		n -= retval;
-		is_eof = n == 1 && read_buf(ldata, tail) == EOF_CHAR(tty);
 		tty_audit_add_data(tty, read_buf_addr(ldata, tail), n,
 				ldata->icanon);
 		smp_store_release(&ldata->read_tail, ldata->read_tail + n);
-		/* Turn single EOF into zero-length read */
-		if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
-		    (head == ldata->read_tail))
-			n = 0;
 		*b += n;
 		*nr -= n;
 	}
@@ -2257,7 +2247,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 			continue;
 		}
 
-		if (ldata->icanon && !L_EXTPROC(tty)) {
+		if (ldata->icanon) {
 			retval = canon_copy_from_read_buf(tty, &b, &nr);
 			if (retval == -EAGAIN) {
 				retval = 0;
diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index ee06b77..01ac182 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -192,20 +192,6 @@ static int pty_get_pktmode(struct tty_struct *tty, int __user *arg)
 	return put_user(pktmode, arg);
 }
 
-/* Send a signal to the slave */
-static int pty_signal(struct tty_struct *tty, int sig)
-{
-	struct pid *pgrp;
-
-	if (tty->link) {
-		pgrp = tty_get_pgrp(tty->link);
-		if (pgrp)
-			kill_pgrp(pgrp, sig, 1);
-		put_pid(pgrp);
-	}
-	return 0;
-}
-
 static void pty_flush_buffer(struct tty_struct *tty)
 {
 	struct tty_struct *to = tty->link;
@@ -254,15 +240,13 @@ static void pty_set_termios(struct tty_struct *tty,
 {
 	/* See if packet mode change of state. */
 	if (tty->link && tty->link->packet) {
-		int extproc = (old_termios->c_lflag & EXTPROC) |
-				(tty->termios.c_lflag & EXTPROC);
 		int old_flow = ((old_termios->c_iflag & IXON) &&
 				(old_termios->c_cc[VSTOP] == '\023') &&
 				(old_termios->c_cc[VSTART] == '\021'));
 		int new_flow = (I_IXON(tty) &&
 				STOP_CHAR(tty) == '\023' &&
 				START_CHAR(tty) == '\021');
-		if ((old_flow != new_flow) || extproc) {
+		if (old_flow != new_flow) {
 			spin_lock_irq(&tty->ctrl_lock);
 			if (old_flow != new_flow) {
 				tty->ctrl_status &= ~(TIOCPKT_DOSTOP | TIOCPKT_NOSTOP);
@@ -271,8 +255,6 @@ static void pty_set_termios(struct tty_struct *tty,
 				else
 					tty->ctrl_status |= TIOCPKT_NOSTOP;
 			}
-			if (extproc)
-				tty->ctrl_status |= TIOCPKT_IOCTL;
 			spin_unlock_irq(&tty->ctrl_lock);
 			wake_up_interruptible(&tty->link->read_wait);
 		}
@@ -482,8 +464,6 @@ static int pty_bsd_ioctl(struct tty_struct *tty,
 		return pty_set_pktmode(tty, (int __user *)arg);
 	case TIOCGPKT: /* Get PT packet mode */
 		return pty_get_pktmode(tty, (int __user *)arg);
-	case TIOCSIG:    /* Send signal to other side of pty */
-		return pty_signal(tty, (int) arg);
 	case TIOCGPTN: /* TTY returns ENOTTY, but glibc expects EINVAL here */
 		return -EINVAL;
 	}
@@ -607,8 +587,6 @@ static int pty_unix98_ioctl(struct tty_struct *tty,
 		return pty_get_pktmode(tty, (int __user *)arg);
 	case TIOCGPTN: /* Get PT Number */
 		return put_user(tty->index, (unsigned int __user *)arg);
-	case TIOCSIG:    /* Send signal to other side of pty */
-		return pty_signal(tty, (int) arg);
 	}
 
 	return -ENOIOCTLCMD;
-- 
2.2.2

Re: [PATCH] n_tty: Remove LINEMODE support

[Howard Chu]

Peter Hurley wrote:
> 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

>
> Cc: Howard Chu <hyc@symas.com>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>   drivers/tty/n_tty.c | 22 ++++++----------------
>   drivers/tty/pty.c   | 24 +-----------------------
>   2 files changed, 7 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index eb9f114..5120d2b 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1562,10 +1562,6 @@ n_tty_receive_buf_standard(struct tty_struct *tty, const unsigned char *cp,
>   				c &= 0x7f;
>   			if (I_IUCLC(tty) && L_IEXTEN(tty))
>   				c = tolower(c);
> -			if (L_EXTPROC(tty)) {
> -				put_tty_queue(c, ldata);
> -				continue;
> -			}
>   			if (!test_bit(c, ldata->char_map))
>   				n_tty_receive_char_inline(tty, c);
>   			else if (n_tty_receive_char_special(tty, c) && count) {
> @@ -1613,9 +1609,9 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
>
>   	if (ldata->real_raw)
>   		n_tty_receive_buf_real_raw(tty, cp, fp, count);
> -	else if (ldata->raw || (L_EXTPROC(tty) && !preops))
> +	else if (ldata->raw)
>   		n_tty_receive_buf_raw(tty, cp, fp, count);
> -	else if (tty->closing && !L_EXTPROC(tty))
> +	else if (tty->closing)
>   		n_tty_receive_buf_closing(tty, cp, fp, count);
>   	else {
>   		if (ldata->lnext) {
> @@ -1637,13 +1633,13 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
>   			tty->ops->flush_chars(tty);
>   	}
>
> -	if (ldata->icanon && !L_EXTPROC(tty))
> +	if (ldata->icanon)
>   		return;
>
>   	/* publish read_head to consumer */
>   	smp_store_release(&ldata->commit_head, ldata->read_head);
>
> -	if ((read_cnt(ldata) >= ldata->minimum_to_wake) || L_EXTPROC(tty)) {
> +	if (read_cnt(ldata) >= ldata->minimum_to_wake) {
>   		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
>   		if (waitqueue_active(&tty->read_wait))
>   			wake_up_interruptible_poll(&tty->read_wait, POLLIN);
> @@ -1939,7 +1935,7 @@ static inline int input_available_p(struct tty_struct *tty, int poll)
>   	struct n_tty_data *ldata = tty->disc_data;
>   	int amt = poll && !TIME_CHAR(tty) && MIN_CHAR(tty) ? MIN_CHAR(tty) : 1;
>
> -	if (ldata->icanon && !L_EXTPROC(tty))
> +	if (ldata->icanon)
>   		return ldata->canon_head != ldata->read_tail;
>   	else
>   		return ldata->commit_head - ldata->read_tail >= amt;
> @@ -1973,7 +1969,6 @@ static int copy_from_read_buf(struct tty_struct *tty,
>   	struct n_tty_data *ldata = tty->disc_data;
>   	int retval;
>   	size_t n;
> -	bool is_eof;
>   	size_t head = smp_load_acquire(&ldata->commit_head);
>   	size_t tail = ldata->read_tail & (N_TTY_BUF_SIZE - 1);
>
> @@ -1983,14 +1978,9 @@ static int copy_from_read_buf(struct tty_struct *tty,
>   	if (n) {
>   		retval = copy_to_user(*b, read_buf_addr(ldata, tail), n);
>   		n -= retval;
> -		is_eof = n == 1 && read_buf(ldata, tail) == EOF_CHAR(tty);
>   		tty_audit_add_data(tty, read_buf_addr(ldata, tail), n,
>   				ldata->icanon);
>   		smp_store_release(&ldata->read_tail, ldata->read_tail + n);
> -		/* Turn single EOF into zero-length read */
> -		if (L_EXTPROC(tty) && ldata->icanon && is_eof &&
> -		    (head == ldata->read_tail))
> -			n = 0;
>   		*b += n;
>   		*nr -= n;
>   	}
> @@ -2257,7 +2247,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
>   			continue;
>   		}
>
> -		if (ldata->icanon && !L_EXTPROC(tty)) {
> +		if (ldata->icanon) {
>   			retval = canon_copy_from_read_buf(tty, &b, &nr);
>   			if (retval == -EAGAIN) {
>   				retval = 0;
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index ee06b77..01ac182 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -192,20 +192,6 @@ static int pty_get_pktmode(struct tty_struct *tty, int __user *arg)
>   	return put_user(pktmode, arg);
>   }
>
> -/* Send a signal to the slave */
> -static int pty_signal(struct tty_struct *tty, int sig)
> -{
> -	struct pid *pgrp;
> -
> -	if (tty->link) {
> -		pgrp = tty_get_pgrp(tty->link);
> -		if (pgrp)
> -			kill_pgrp(pgrp, sig, 1);
> -		put_pid(pgrp);
> -	}
> -	return 0;
> -}
> -
>   static void pty_flush_buffer(struct tty_struct *tty)
>   {
>   	struct tty_struct *to = tty->link;
> @@ -254,15 +240,13 @@ static void pty_set_termios(struct tty_struct *tty,
>   {
>   	/* See if packet mode change of state. */
>   	if (tty->link && tty->link->packet) {
> -		int extproc = (old_termios->c_lflag & EXTPROC) |
> -				(tty->termios.c_lflag & EXTPROC);
>   		int old_flow = ((old_termios->c_iflag & IXON) &&
>   				(old_termios->c_cc[VSTOP] == '\023') &&
>   				(old_termios->c_cc[VSTART] == '\021'));
>   		int new_flow = (I_IXON(tty) &&
>   				STOP_CHAR(tty) == '\023' &&
>   				START_CHAR(tty) == '\021');
> -		if ((old_flow != new_flow) || extproc) {
> +		if (old_flow != new_flow) {
>   			spin_lock_irq(&tty->ctrl_lock);
>   			if (old_flow != new_flow) {
>   				tty->ctrl_status &= ~(TIOCPKT_DOSTOP | TIOCPKT_NOSTOP);
> @@ -271,8 +255,6 @@ static void pty_set_termios(struct tty_struct *tty,
>   				else
>   					tty->ctrl_status |= TIOCPKT_NOSTOP;
>   			}
> -			if (extproc)
> -				tty->ctrl_status |= TIOCPKT_IOCTL;
>   			spin_unlock_irq(&tty->ctrl_lock);
>   			wake_up_interruptible(&tty->link->read_wait);
>   		}
> @@ -482,8 +464,6 @@ static int pty_bsd_ioctl(struct tty_struct *tty,
>   		return pty_set_pktmode(tty, (int __user *)arg);
>   	case TIOCGPKT: /* Get PT packet mode */
>   		return pty_get_pktmode(tty, (int __user *)arg);
> -	case TIOCSIG:    /* Send signal to other side of pty */
> -		return pty_signal(tty, (int) arg);
>   	case TIOCGPTN: /* TTY returns ENOTTY, but glibc expects EINVAL here */
>   		return -EINVAL;
>   	}
> @@ -607,8 +587,6 @@ static int pty_unix98_ioctl(struct tty_struct *tty,
>   		return pty_get_pktmode(tty, (int __user *)arg);
>   	case TIOCGPTN: /* Get PT Number */
>   		return put_user(tty->index, (unsigned int __user *)arg);
> -	case TIOCSIG:    /* Send signal to other side of pty */
> -		return pty_signal(tty, (int) arg);
>   	}
>
>   	return -ENOIOCTLCMD;
>


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

Hi Howard,

On 01/18/2015 05:09 PM, Howard Chu wrote:
> Peter Hurley wrote:
>> 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.

Where is a working userspace consumer of this interface?

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.

Also, ioctl(FIONREAD) doesn't match what read() returns, nor that poll()/select()
indicated input was available.

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Howard Chu]

Peter Hurley wrote:
> Hi Howard,
>
> On 01/18/2015 05:09 PM, Howard Chu wrote:
>> Peter Hurley wrote:
>>> 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.

> Where is a working userspace consumer of this interface?

The OpenSSH fork on github is a full working client and server using 
this interface.

> 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.

> 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);
     }


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/18/2015 05:44 PM, Howard Chu wrote:
> Peter Hurley wrote:
>> Hi Howard,
>>
>> On 01/18/2015 05:09 PM, Howard Chu wrote:
>>> Peter Hurley wrote:
>>>> 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.


>> 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.


>> 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.


>> 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

Re: [PATCH] n_tty: Remove LINEMODE support

[Theodore Ts'o]

Line mode goes back to BSD 4.x days, and it's useful over high latency
links --- i.e., the sort of thing that you get with stone age cellular
data networks (i.e., the sort of thing that we still have the US),
amateur packet radio links, etc.  So it's certianly a nice to have, if
someone is willing to support it and the maintenance burden on the
rest of the tty stack isn't too onerous.

OTOH, one could argue that with something like mosh, if you can get
that working across a particular firewall, it solves the probably much
more portably without requiring kernel hacks.  Mosh couldn't be used
on amateur packet links, since it uses encryption, which is verboten
for ham radio.  But for other high latency links, mosh does work
fairly well, without requiring EXTPROC in the kernel.

       	     	     	       	       	      - Ted

Re: [PATCH] n_tty: Remove LINEMODE support

[Howard Chu]

Peter Hurley wrote:
> On 01/18/2015 05:45 PM, Howard Chu wrote:
>> Peter Hurley wrote:
>>> 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.
>>
>> What's the motivation to remove this code, rather than improve it if it
>> needs fixing? It has been removed from the Linux kernel at least once
>> before already, and that was a mistake back then too.
>
> It is a significant maintenance burden, and I have concerns about the
> level of support it's receiving. Here's some outstanding issues:
>
> 1. No man page documentation. At a minimum, tty_ioctl(4) and termios(3)
>     need the userspace visible definitions and behavior documented. Better
>     would be a LINEMODE (7) description of how this implementation works
>     wrt supporting RFC 1116.

That can be added easily enough. Historically EXTPROC has had very 
little documentation of its own. E.g. the FreeBSD manpage only shows 
that the flag exists.

https://www.freebsd.org/cgi/man.cgi?query=termios&sektion=4

> 2. read(), poll()/select() and ioctl() with and without EXTPROC need to
>     have _identical_ userspace behavior.

OK, this can be fixed.

> 3. Does the local edit guarantee canon lines <= 4096 chars? What happens
>     if pty slave reader does this?
>
> 	char buffer[4096];
> 	char *p = buffer;
>
> 	n = read(tty, buffer, sizeof(buffer));
> 	if (n <= 0)
> 		goto done;
> 	while (*p++ != '\n')
> 		;

This reader is broken, since the tty driver supports EOL and EOL2 and 
this code doesn't account for it.

In practice your concern is misdirected - it's the job of whatever code 
is talking to the pty master to send valid data to the pty slave. 
There's no reason for the tty driver to second-guess the app here.

> 4. ioctl(TIOCSIG) can send _any_ signal to a different process without
>     permission checks. That's not good.

It can only send to the pty slave. Permissions were already checked when 
the pty master was opened. What further checks do you think are needed? 
You think it should be limited to tty-specific signals: INT, QUIT, CONT, 
TSTP, TTIN, TTOU, WINCH?

> 5. This needs to work with readline(). Right now, I don't see how this
>     won't have worst-case behavior, constantly sending termios changes,
>     with scripted input where the reader switches back-and-forth between
>     canonical and non-canonical mode (like readline() does). Database
>     shells behave like this, but you can do a 20-line shell mockup with
>     just readline().

readline() patched accordingly 
https://groups.google.com/forum/#!topic/gnu.bash.bug/o0UA55AhADs will 
cooperate. Sending one or two termios changes per input line is still 
far better than one character-at-a-time packets back and forth.

> 6. EXTPROC still does some input processing on the server. For example,
>     7-bit mode (ISTRIP), tolower mode (IUCLC) and processing while
>     closing; if input processing is being done on the local/client side,
>     why the extra work here?

That's defensive, on the assumption that something else might break if 
e.g. the tty expected only 7-bit input but 8-bit characters were sent to it.

> 7. This needs a reference userspace implementation which for the moment
>     could double as regression testing. A library with unit tests would
>     be ideal.

telnet/telnetd can probably used as a starting point for this.

> ISTM the right implementation, if there is one, is for EXTPROC to process
> input exactly like raw mode except that line termination wakes up read_wait
> and there is no special casing in read/poll.

I agree that this sounds simpler but I feel there's a reason (which I 
can't remember at the moment) that it doesn't work out that way.

> Does SLC_FORW1 & SLC_FORW2
> map directly to termios.c_cc[] line termination values?

Maps exactly to VEOL / VEOL2.

> I'd like to do away with the signalling part; just turn off EXTPROC
> and send the appropriate signalling char from the pty master, like telnetd
> does now. Same for EOF.

That introduces additional mode changes, which you were just worrying 
about above, re: readline. It would make the traffic stream less 
efficient overall.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

Thanks, Howard.

[ adding Ted too ]

On 01/19/2015 07:46 AM, Howard Chu wrote:
> Peter Hurley wrote:
>> On 01/18/2015 05:45 PM, Howard Chu wrote:
>>> Peter Hurley wrote:
>>>> 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.
>>>
>>> What's the motivation to remove this code, rather than improve it if it
>>> needs fixing? It has been removed from the Linux kernel at least once
>>> before already, and that was a mistake back then too.
>>
>> It is a significant maintenance burden, and I have concerns about the
>> level of support it's receiving. Here's some outstanding issues:
>>
>> 1. No man page documentation. At a minimum, tty_ioctl(4) and termios(3)
>>     need the userspace visible definitions and behavior documented. Better
>>     would be a LINEMODE (7) description of how this implementation works
>>     wrt supporting RFC 1116.
> 
> That can be added easily enough.

Is this you signing up?  :)

> Historically EXTPROC has had very little documentation of its own. E.g. the FreeBSD manpage only shows that the flag exists.
> 
> https://www.freebsd.org/cgi/man.cgi?query=termios&sektion=4

A description of how the pty master uses EXTPROC to implement line mode would
be very helpful, especially to people working in the tty code (eg., me, although
I don't need it now).


>> 2. read(), poll()/select() and ioctl() with and without EXTPROC need to
>>     have _identical_ userspace behavior.
> 
> OK, this can be fixed.

Ok.

>> 3. Does the local edit guarantee canon lines <= 4096 chars? What happens
>>     if pty slave reader does this?
>>
>>     char buffer[4096];
>>     char *p = buffer;
>>
>>     n = read(tty, buffer, sizeof(buffer));
>>     if (n <= 0)
>>         goto done;
>>     while (*p++ != '\n')
>>         ;
> 
> This reader is broken, since the tty driver supports EOL and EOL2 and this code doesn't account for it.

This reader set EOL2 to DISABLED_CHAR earlier, and left EOL unchanged.
I have seen userspace code that expects a line to be no longer than 4096 chars.

> In practice your concern is misdirected - it's the job of whatever code is talking to the pty master to send valid data to the pty slave. There's no reason for the tty driver to second-guess the app here.

What about a malicious client?

>> 4. ioctl(TIOCSIG) can send _any_ signal to a different process without
>>     permission checks. That's not good.
> 
> It can only send to the pty slave. Permissions were already checked when the pty master was opened.

Still not ok.

> What further checks do you think are needed? You think it should be limited to tty-specific signals: INT, QUIT, CONT, TSTP, TTIN, TTOU, WINCH?

My first choice is to do away with TIOCSIG ioctl completely; see ending comments.
My second would be masked to only those already used: INT, QUIT, TSTP.

>> 5. This needs to work with readline(). Right now, I don't see how this
>>     won't have worst-case behavior, constantly sending termios changes,
>>     with scripted input where the reader switches back-and-forth between
>>     canonical and non-canonical mode (like readline() does). Database
>>     shells behave like this, but you can do a 20-line shell mockup with
>>     just readline().
> 
> readline() patched accordingly https://groups.google.com/forum/#!topic/gnu.bash.bug/o0UA55AhADs will cooperate. Sending one or two termios changes per input line is still far better than one character-at-a-time packets back and forth.

Ok, I misunderstood: the only incompatibility here is that readline()
simply doesn't use line mode. That's fine from the kernel perspective.

Although it does seem odd that the command shell doesn't support the
input mode :)

>> 6. EXTPROC still does some input processing on the server. For example,
>>     7-bit mode (ISTRIP), tolower mode (IUCLC) and processing while
>>     closing; if input processing is being done on the local/client side,
>>     why the extra work here?
> 
> That's defensive, on the assumption that something else might break if e.g. the tty expected only 7-bit input but 8-bit characters were sent to it.

Ok, is that because RFC 1116 doesn't specify ISTRIP and IUCLC handling so
the server can't be sure the client did it? If so, that should be documented
so that refactors don't remove that handling.

Regular ptys dump input while closing so this should too.

>> 7. This needs a reference userspace implementation which for the moment
>>     could double as regression testing. A library with unit tests would
>>     be ideal.
> 
> telnet/telnetd can probably used as a starting point for this.

Except telnet is unmaintained except by each distro. Your patches don't apply
to my distro telnet; they had been applied but are now ifdef'd out. I messed
around with rebuilding it but never got telnetd to actually set EXTPROC.

My point is there is no straightforward way to test this, and that's a problem.

In fact, this is my main concern: no matter how useful this might be, it's
pointless if the complementary userspace puzzle piece is unsupported.
I know you have written patches but that's not the same as support; for example
your OpenSSH fork is 4 years old, which is a non-starter because I'm sure
there's known exploits which have been fixed in those 4 years.

Why doesn't Ubuntu/Debian telnet even compile for line mode support? Did the
package maintainer shut it off because it was causing bug reports?

What more needs to be done to get line mode working 100% in bash?

>> ISTM the right implementation, if there is one, is for EXTPROC to process
>> input exactly like raw mode except that line termination wakes up read_wait
>> and there is no special casing in read/poll.
> 
> I agree that this sounds simpler but I feel there's a reason (which I can't remember at the moment) that it doesn't work out that way.
> 
>> Does SLC_FORW1 & SLC_FORW2
>> map directly to termios.c_cc[] line termination values?
> 
> Maps exactly to VEOL / VEOL2.
> 
>> I'd like to do away with the signalling part; just turn off EXTPROC
>> and send the appropriate signalling char from the pty master, like telnetd
>> does now. Same for EOF.
> 
> That introduces additional mode changes, which you were just worrying about above, re: readline. It would make the traffic stream less efficient overall.

Maybe you misunderstood. If local signalling is being performed by the client,
the over-the-wire traffic is the same. The pty master would turn off EXTPROC to
write the signalling char but would not reflect the termios to the client (because
it contains no relevant changes).

And the signalling char is isolated and unique: it's not as if it's being stuffed
within an existing stream of i/o, because the client should have already ceased
input if it's handling signals locally.

Which brings up another point: only a pty master should be able to set EXTPROC
mode. Right now, any tty can be set to EXTPROC and the pty slave can even
accidentally unset it. This argues for a new pty ioctl() to set EXTPROC in
termios. pty_set_termios() can silently merge the bit.

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

Hi Ted,

On 01/18/2015 11:55 PM, Theodore Ts'o wrote:
> Line mode goes back to BSD 4.x days, and it's useful over high latency
> links --- i.e., the sort of thing that you get with stone age cellular
> data networks (i.e., the sort of thing that we still have the US),
> amateur packet radio links, etc.  So it's certianly a nice to have, if
> someone is willing to support it and the maintenance burden on the
> rest of the tty stack isn't too onerous.

I'm not philosophically opposed to Line mode support, but I am
concerned about providing a userspace interface that's not getting tested
or used or otherwise not fit for purpose. I copied you on the other email
regarding the specific deficiencies so I won't reiterate them here.

Perhaps it's fortunate that userspace didn't pick this up; otherwise,
this interface would be cast in stone.

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Howard Chu]

Peter Hurley wrote:
> Thanks, Howard.
>
> [ adding Ted too ]
>
> On 01/19/2015 07:46 AM, Howard Chu wrote:
>> Peter Hurley wrote:
>>> On 01/18/2015 05:45 PM, Howard Chu wrote:
>>>> Peter Hurley wrote:
>>>>> 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.
>>>>
>>>> What's the motivation to remove this code, rather than improve it if it
>>>> needs fixing? It has been removed from the Linux kernel at least once
>>>> before already, and that was a mistake back then too.
>>>
>>> It is a significant maintenance burden, and I have concerns about the
>>> level of support it's receiving. Here's some outstanding issues:
>>>
>>> 1. No man page documentation. At a minimum, tty_ioctl(4) and termios(3)
>>>      need the userspace visible definitions and behavior documented. Better
>>>      would be a LINEMODE (7) description of how this implementation works
>>>      wrt supporting RFC 1116.
>>
>> That can be added easily enough.
>
> Is this you signing up?  :)

Sure. I suppose the obvious place to add it is near the TIOCPKT doc in 
tty_ioctl(4). But, about your further comment:

 > Which brings up another point: only a pty master should be able to 
set EXTPROC
 > mode. Right now, any tty can be set to EXTPROC and the pty slave can even
 > accidentally unset it. This argues for a new pty ioctl() to set 
EXTPROC in
 > termios. pty_set_termios() can silently merge the bit.

Historically EXTPROC could also be set on regular ttys, for lines that 
were hooked up to special terminal interfaces. E.g. X.29 PADs that did 
their own input processing. I'm not sure how relevant this is now, but 
this is the reason EXTPROC is not a pty-specific ioctl.

> A description of how the pty master uses EXTPROC to implement line mode would
> be very helpful, especially to people working in the tty code (eg., me, although
> I don't need it now).

The original patch submission carried this description
http://lkml.iu.edu/hypermail/linux/kernel/1006.2/02428.html

>>> 3. Does the local edit guarantee canon lines <= 4096 chars? What happens
>>>      if pty slave reader does this?
>>>
>>>      char buffer[4096];
>>>      char *p = buffer;
>>>
>>>      n = read(tty, buffer, sizeof(buffer));
>>>      if (n <= 0)
>>>          goto done;
>>>      while (*p++ != '\n')
>>>          ;
>>
>> This reader is broken, since the tty driver supports EOL and EOL2 and this code doesn't account for it.
>
> This reader set EOL2 to DISABLED_CHAR earlier, and left EOL unchanged.
> I have seen userspace code that expects a line to be no longer than 4096 chars.
>
>> In practice your concern is misdirected - it's the job of whatever code is talking to the pty master to send valid data to the pty slave. There's no reason for the tty driver to second-guess the app here.
>
> What about a malicious client?

This is still a broken reader. It's as bad as using strchr() without 
knowing if the string is NUL-terminated, if 4096 bytes are read the 
thing will go off into the weeds.
>
>>> 4. ioctl(TIOCSIG) can send _any_ signal to a different process without
>>>      permission checks. That's not good.
>>
>> It can only send to the pty slave. Permissions were already checked when the pty master was opened.
>
> Still not ok.
>
>> What further checks do you think are needed? You think it should be limited to tty-specific signals: INT, QUIT, CONT, TSTP, TTIN, TTOU, WINCH?
>
> My first choice is to do away with TIOCSIG ioctl completely; see ending comments.
> My second would be masked to only those already used: INT, QUIT, TSTP.

Perhaps your way is better. This patch simply duplicated the BSD 
functionality, to maintain compatibility.

>>> 5. This needs to work with readline(). Right now, I don't see how this
>>>      won't have worst-case behavior, constantly sending termios changes,
>>>      with scripted input where the reader switches back-and-forth between
>>>      canonical and non-canonical mode (like readline() does). Database
>>>      shells behave like this, but you can do a 20-line shell mockup with
>>>      just readline().
>>
>> readline() patched accordingly https://groups.google.com/forum/#!topic/gnu.bash.bug/o0UA55AhADs will cooperate. Sending one or two termios changes per input line is still far better than one character-at-a-time packets back and forth.
>
> Ok, I misunderstood: the only incompatibility here is that readline()
> simply doesn't use line mode. That's fine from the kernel perspective.
>
> Although it does seem odd that the command shell doesn't support the
> input mode :)
>
>>> 6. EXTPROC still does some input processing on the server. For example,
>>>      7-bit mode (ISTRIP), tolower mode (IUCLC) and processing while
>>>      closing; if input processing is being done on the local/client side,
>>>      why the extra work here?
>>
>> That's defensive, on the assumption that something else might break if e.g. the tty expected only 7-bit input but 8-bit characters were sent to it.
>
> Ok, is that because RFC 1116 doesn't specify ISTRIP and IUCLC handling so
> the server can't be sure the client did it? If so, that should be documented
> so that refactors don't remove that handling.
>
> Regular ptys dump input while closing so this should too.
>
>>> 7. This needs a reference userspace implementation which for the moment
>>>      could double as regression testing. A library with unit tests would
>>>      be ideal.
>>
>> telnet/telnetd can probably used as a starting point for this.
>
> Except telnet is unmaintained except by each distro. Your patches don't apply
> to my distro telnet; they had been applied but are now ifdef'd out. I messed
> around with rebuilding it but never got telnetd to actually set EXTPROC.
>
> My point is there is no straightforward way to test this, and that's a problem.
>
> In fact, this is my main concern: no matter how useful this might be, it's
> pointless if the complementary userspace puzzle piece is unsupported.
> I know you have written patches but that's not the same as support; for example
> your OpenSSH fork is 4 years old, which is a non-starter because I'm sure
> there's known exploits which have been fixed in those 4 years.
>
> Why doesn't Ubuntu/Debian telnet even compile for line mode support? Did the
> package maintainer shut it off because it was causing bug reports?

I don't know. Looking at the package history it appears they have never 
enabled it. https://launchpad.net/ubuntu/+source/netkit-telnet/+changelog

I also don't know where the actual VCS repo is for this code.

> What more needs to be done to get line mode working 100% in bash?

TAB-completion was messy. Although editing and scrolling through command 
history can be done completely locally, TAB-completion required sending 
a partial line to the server and feeding the completion results back, 
and then possibly flushing the input buffer if the results weren't 
actually used.

With a few years of hindsight, this can probably be cleaned up better now.

>>> I'd like to do away with the signalling part; just turn off EXTPROC
>>> and send the appropriate signalling char from the pty master, like telnetd
>>> does now. Same for EOF.
>>
>> That introduces additional mode changes, which you were just worrying about above, re: readline. It would make the traffic stream less efficient overall.
>
> Maybe you misunderstood. If local signalling is being performed by the client,
> the over-the-wire traffic is the same. The pty master would turn off EXTPROC to
> write the signalling char but would not reflect the termios to the client (because
> it contains no relevant changes).
>
> And the signalling char is isolated and unique: it's not as if it's being stuffed
> within an existing stream of i/o, because the client should have already ceased
> input if it's handling signals locally.
>
Makes sense. Don't know why the original telnet patch didn't do this.
-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] n_tty: Remove LINEMODE support

[Theodore Ts'o]

On Mon, Jan 19, 2015 at 09:57:23AM -0500, Peter Hurley wrote:
> 
> This reader set EOL2 to DISABLED_CHAR earlier, and left EOL unchanged.
> I have seen userspace code that expects a line to be no longer than 4096 chars.

Userspace code that does this is going to be very fragile.  Input from
the tty really should be considered completely untrustworthy.  For
example, what if, while the tty is in canonical (ICANON) mode, an
attacker sets PARMRK and clears IGNPAR, IGNBRK, and BRKINT on the tty
and then proceeds to send a large number of breaks or parity errors
down the tty?  That will certainly cause a buffer overflow of said
userspace process.  So if there is *any* setuid program which isn't
defensively checking for buffer overruns it's kind of screwed anyway.

> >> 4. ioctl(TIOCSIG) can send _any_ signal to a different process without
> >>     permission checks. That's not good.
> > 
> > It can only send to the pty slave. Permissions were already checked when the pty master was opened.
> 
> Still not ok.

Interestingly, TIOCSIG is supported by FreeBSD *and* OpenBSD, and
without any kind of checks.  That being said, I can certainly think of
potential security threats that could happen if a malicious program
were to run a setuid program (say, /bin/passwd) under a pty, and then
proceeded to send it a one or more of non-trappable signals -- for
example, either SIGKILL or SIGSTOP -- to either widen the window for a
race condition, or to kill a setuid program at a particularly
inconvenient time.

It might be interesting to see if someone could figure out an attack
that utilized this ioctl, and then demonstrated it on OpenBSD.  :-)

> > What further checks do you think are needed? You think it should
> > be limited to tty-specific signals: INT, QUIT, CONT, TSTP, TTIN,
> > TTOU, WINCH?

Definitely, and we should do this sooner rather than later IMHO.
Preferably before someone figures out whether or not it's possible to
attack OpenBSD and FreeBSD using this ioctl, and requests a CVE.  :-)

	    	   	     	  	 	     - Ted

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/19/2015 11:37 AM, Theodore Ts'o wrote:
> On Mon, Jan 19, 2015 at 09:57:23AM -0500, Peter Hurley wrote:
>>
>> This reader set EOL2 to DISABLED_CHAR earlier, and left EOL unchanged.
>> I have seen userspace code that expects a line to be no longer than 4096 chars.
> 
> Userspace code that does this is going to be very fragile.  Input from
> the tty really should be considered completely untrustworthy.

Should and do are different modal verbs.

> For example, what if, while the tty is in canonical (ICANON) mode, an
> attacker sets PARMRK and clears IGNPAR, IGNBRK, and BRKINT on the tty
> and then proceeds to send a large number of breaks or parity errors
> down the tty?

The input path limits _all_ canonical lines to 4096 chars. Input beyond that
is discarded until a newline is received. I know this because I broke it
and just recently fixed it.

> That will certainly cause a buffer overflow of said
> userspace process.  So if there is *any* setuid program which isn't
> defensively checking for buffer overruns it's kind of screwed anyway.
> 
>>>> 4. ioctl(TIOCSIG) can send _any_ signal to a different process without
>>>>     permission checks. That's not good.
>>>
>>> It can only send to the pty slave. Permissions were already checked when the pty master was opened.
>>
>> Still not ok.
> 
> Interestingly, TIOCSIG is supported by FreeBSD *and* OpenBSD, and
> without any kind of checks.  That being said, I can certainly think of
> potential security threats that could happen if a malicious program
> were to run a setuid program (say, /bin/passwd) under a pty, and then
> proceeded to send it a one or more of non-trappable signals -- for
> example, either SIGKILL or SIGSTOP -- to either widen the window for a
> race condition, or to kill a setuid program at a particularly
> inconvenient time.

I hadn't considered that possibility.

> It might be interesting to see if someone could figure out an attack
> that utilized this ioctl, and then demonstrated it on OpenBSD.  :-)

I don't need that reputation :)

>>> What further checks do you think are needed? You think it should
>>> be limited to tty-specific signals: INT, QUIT, CONT, TSTP, TTIN,
>>> TTOU, WINCH?
> 
> Definitely, and we should do this sooner rather than later IMHO.
> Preferably before someone figures out whether or not it's possible to
> attack OpenBSD and FreeBSD using this ioctl, and requests a CVE.  :-)

I'm on it.

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/19/2015 11:36 AM, Howard Chu wrote:
> Peter Hurley wrote:
>> Thanks, Howard.
>>
>> [ adding Ted too ]
>>
>> On 01/19/2015 07:46 AM, Howard Chu wrote:
>>> Peter Hurley wrote:
>>>> On 01/18/2015 05:45 PM, Howard Chu wrote:
>>>>> Peter Hurley wrote:
>>>>>> 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.
>>>>>
>>>>> What's the motivation to remove this code, rather than improve it if it
>>>>> needs fixing? It has been removed from the Linux kernel at least once
>>>>> before already, and that was a mistake back then too.
>>>>
>>>> It is a significant maintenance burden, and I have concerns about the
>>>> level of support it's receiving. Here's some outstanding issues:
>>>>
>>>> 1. No man page documentation. At a minimum, tty_ioctl(4) and termios(3)
>>>>      need the userspace visible definitions and behavior documented. Better
>>>>      would be a LINEMODE (7) description of how this implementation works
>>>>      wrt supporting RFC 1116.
>>>
>>> That can be added easily enough.
>>
>> Is this you signing up?  :)
> 
> Sure. I suppose the obvious place to add it is near the TIOCPKT doc in tty_ioctl(4). But, about your further comment:

Thanks.

>> Which brings up another point: only a pty master should be able to set EXTPROC
>> mode. Right now, any tty can be set to EXTPROC and the pty slave can even
>> accidentally unset it. This argues for a new pty ioctl() to set EXTPROC in
>> termios. pty_set_termios() can silently merge the bit.
> 
> Historically EXTPROC could also be set on regular ttys, for lines that were hooked up to special terminal interfaces. E.g. X.29 PADs that did their own input processing. I'm not sure how relevant this is now, but this is the reason EXTPROC is not a pty-specific ioctl.

How does the remote end find out when non-canon mode is selected?
And who does the EXTPROC setting, because if it's the program that opened the
tty, then it can just select raw mode instead.


>> A description of how the pty master uses EXTPROC to implement line mode would
>> be very helpful, especially to people working in the tty code (eg., me, although
>> I don't need it now).
> 
> The original patch submission carried this description
> http://lkml.iu.edu/hypermail/linux/kernel/1006.2/02428.html
> 
>>>> 3. Does the local edit guarantee canon lines <= 4096 chars? What happens
>>>>      if pty slave reader does this?
>>>>
>>>>      char buffer[4096];
>>>>      char *p = buffer;
>>>>
>>>>      n = read(tty, buffer, sizeof(buffer));
>>>>      if (n <= 0)
>>>>          goto done;
>>>>      while (*p++ != '\n')
>>>>          ;
>>>
>>> This reader is broken, since the tty driver supports EOL and EOL2 and this code doesn't account for it.
>>
>> This reader set EOL2 to DISABLED_CHAR earlier, and left EOL unchanged.
>> I have seen userspace code that expects a line to be no longer than 4096 chars.
>>
>>> In practice your concern is misdirected - it's the job of whatever code is talking to the pty master to send valid data to the pty slave. There's no reason for the tty driver to second-guess the app here.
>>
>> What about a malicious client?
> 
> This is still a broken reader. It's as bad as using strchr() without knowing if the string is NUL-terminated, if 4096 bytes are read the thing will go off into the weeds.

There's lots of broken userspace programs that do all kinds of stupid stuff.
Want to find out which ones they are? Just declare N_TTY_BUF_SIZE to be 8192
and start allowing that canon line size through.


>>>> 4. ioctl(TIOCSIG) can send _any_ signal to a different process without
>>>>      permission checks. That's not good.
>>>
>>> It can only send to the pty slave. Permissions were already checked when the pty master was opened.
>>
>> Still not ok.
>>
>>> What further checks do you think are needed? You think it should be limited to tty-specific signals: INT, QUIT, CONT, TSTP, TTIN, TTOU, WINCH?
>>
>> My first choice is to do away with TIOCSIG ioctl completely; see ending comments.
>> My second would be masked to only those already used: INT, QUIT, TSTP.
> 
> Perhaps your way is better. This patch simply duplicated the BSD functionality, to maintain compatibility.
> 
>>>> 5. This needs to work with readline(). Right now, I don't see how this
>>>>      won't have worst-case behavior, constantly sending termios changes,
>>>>      with scripted input where the reader switches back-and-forth between
>>>>      canonical and non-canonical mode (like readline() does). Database
>>>>      shells behave like this, but you can do a 20-line shell mockup with
>>>>      just readline().
>>>
>>> readline() patched accordingly https://groups.google.com/forum/#!topic/gnu.bash.bug/o0UA55AhADs will cooperate. Sending one or two termios changes per input line is still far better than one character-at-a-time packets back and forth.
>>
>> Ok, I misunderstood: the only incompatibility here is that readline()
>> simply doesn't use line mode. That's fine from the kernel perspective.
>>
>> Although it does seem odd that the command shell doesn't support the
>> input mode :)
>>
>>>> 6. EXTPROC still does some input processing on the server. For example,
>>>>      7-bit mode (ISTRIP), tolower mode (IUCLC) and processing while
>>>>      closing; if input processing is being done on the local/client side,
>>>>      why the extra work here?
>>>
>>> That's defensive, on the assumption that something else might break if e.g. the tty expected only 7-bit input but 8-bit characters were sent to it.
>>
>> Ok, is that because RFC 1116 doesn't specify ISTRIP and IUCLC handling so
>> the server can't be sure the client did it? If so, that should be documented
>> so that refactors don't remove that handling.

Could you get back to me about this, as well?

>> Regular ptys dump input while closing so this should too.
>>
>>>> 7. This needs a reference userspace implementation which for the moment
>>>>      could double as regression testing. A library with unit tests would
>>>>      be ideal.
>>>
>>> telnet/telnetd can probably used as a starting point for this.
>>
>> Except telnet is unmaintained except by each distro. Your patches don't apply
>> to my distro telnet; they had been applied but are now ifdef'd out. I messed
>> around with rebuilding it but never got telnetd to actually set EXTPROC.
>>
>> My point is there is no straightforward way to test this, and that's a problem.
>>
>> In fact, this is my main concern: no matter how useful this might be, it's
>> pointless if the complementary userspace puzzle piece is unsupported.
>> I know you have written patches but that's not the same as support; for example
>> your OpenSSH fork is 4 years old, which is a non-starter because I'm sure
>> there's known exploits which have been fixed in those 4 years.
>>
>> Why doesn't Ubuntu/Debian telnet even compile for line mode support? Did the
>> package maintainer shut it off because it was causing bug reports?
> 
> I don't know. Looking at the package history it appears they have never enabled it. https://launchpad.net/ubuntu/+source/netkit-telnet/+changelog
> 
> I also don't know where the actual VCS repo is for this code.

There isn't one. The base code is from downloaded tarball.

I don't know what distro you use (or if you even do), but for Ubuntu/Debian
the base package information is here https://packages.debian.org/wheezy/telnetd
including maintainer email, etc.

Typically, to get changes merged requires making a patch(es) against the package
source and sending/uploading those.

Another alternative is to setup your own repository from that packaging;
sometimes I do that on Launchpad from the packaging source so that
the maintainer can easily see the difference. Ubuntu calls these PPAs.

>> What more needs to be done to get line mode working 100% in bash?
> 
> TAB-completion was messy. Although editing and scrolling through command history can be done completely locally, TAB-completion required sending a partial line to the server and feeding the completion results back, and then possibly flushing the input buffer if the results weren't actually used.
> 
> With a few years of hindsight, this can probably be cleaned up better now.

Ok.

>>>> I'd like to do away with the signalling part; just turn off EXTPROC
>>>> and send the appropriate signalling char from the pty master, like telnetd
>>>> does now. Same for EOF.
>>>
>>> That introduces additional mode changes, which you were just worrying about above, re: readline. It would make the traffic stream less efficient overall.
>>
>> Maybe you misunderstood. If local signalling is being performed by the client,
>> the over-the-wire traffic is the same. The pty master would turn off EXTPROC to
>> write the signalling char but would not reflect the termios to the client (because
>> it contains no relevant changes).
>>
>> And the signalling char is isolated and unique: it's not as if it's being stuffed
>> within an existing stream of i/o, because the client should have already ceased
>> input if it's handling signals locally.
>>
> Makes sense. Don't know why the original telnet patch didn't do this.

Ok.
I might see if I can get telnet + a revised kernel interface to work together; I'll
let you know if this works out.

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/19/2015 11:36 AM, Howard Chu wrote:
> Peter Hurley wrote:

[...]

>> A description of how the pty master uses EXTPROC to implement line mode would
>> be very helpful, especially to people working in the tty code (eg., me, although
>> I don't need it now).
> 
> The original patch submission carried this description
> http://lkml.iu.edu/hypermail/linux/kernel/1006.2/02428.html

[from linked source]

> Paraphrased from the 1989 BSD patch by David Borman @ cray.com:
> 
> These are the changes needed for the kernel to support
> LINEMODE in the server.
> 
> There is a new bit in the termios local flag word, EXTPROC.
> When this bit is set, several aspects of the terminal driver
> are disabled. Input line editing, character echo, and mapping
> of signals are all disabled. This allows the telnetd to turn
> off these functions when in linemode, but still keep track of
> what state the user wants the terminal to be in.

Ok, but this should go on to describe at least the basics of
transparent local/remote line editing. Even mentioning that
it should be transparent is important.

> New ioctl:
> TIOCSIG Generate a signal to processes in the
> current process group of the pty.

Yes, but documentation should describe why this is important/required/useful.
That is, how this supports client signal handling.

> There is a new mode for packet driver, the TIOCPKT_IOCTL bit.
> When packet mode is turned on in the pty, and the EXTPROC bit
> is set, then whenever the state of the pty is changed, the
> next read on the master side of the pty will have the TIOCPKT_IOCTL
> bit set. This allows the process on the server side of the pty
> to know when the state of the terminal has changed; it can then
> issue the appropriate ioctl to retrieve the new state.

... and go on to outline in a sentence or two what the pty master
process does with this information.

IOW, _how_ does this kernel interface support LINEMODE?

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Howard Chu]

Peter Hurley wrote:
> On 01/19/2015 11:36 AM, Howard Chu wrote:
>> Peter Hurley wrote:
>>> Thanks, Howard.
>>>
>>> [ adding Ted too ]
>>>
>>> On 01/19/2015 07:46 AM, Howard Chu wrote:
>>>> Peter Hurley wrote:
>>>>> On 01/18/2015 05:45 PM, Howard Chu wrote:
>>>>>> Peter Hurley wrote:
>>>>>>> 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.
>>>>>>
>>>>>> What's the motivation to remove this code, rather than improve it if it
>>>>>> needs fixing? It has been removed from the Linux kernel at least once
>>>>>> before already, and that was a mistake back then too.
>>>>>
>>>>> It is a significant maintenance burden, and I have concerns about the
>>>>> level of support it's receiving. Here's some outstanding issues:
>>>>>
>>>>> 1. No man page documentation. At a minimum, tty_ioctl(4) and termios(3)
>>>>>       need the userspace visible definitions and behavior documented. Better
>>>>>       would be a LINEMODE (7) description of how this implementation works
>>>>>       wrt supporting RFC 1116.
>>>>
>>>> That can be added easily enough.
>>>
>>> Is this you signing up?  :)
>>
>> Sure. I suppose the obvious place to add it is near the TIOCPKT doc in tty_ioctl(4). But, about your further comment:
>
> Thanks.
>
>>> Which brings up another point: only a pty master should be able to set EXTPROC
>>> mode. Right now, any tty can be set to EXTPROC and the pty slave can even
>>> accidentally unset it. This argues for a new pty ioctl() to set EXTPROC in
>>> termios. pty_set_termios() can silently merge the bit.
>>
>> Historically EXTPROC could also be set on regular ttys, for lines that were hooked up to special terminal interfaces. E.g. X.29 PADs that did their own input processing. I'm not sure how relevant this is now, but this is the reason EXTPROC is not a pty-specific ioctl.
>
> How does the remote end find out when non-canon mode is selected?
> And who does the EXTPROC setting, because if it's the program that opened the
> tty, then it can just select raw mode instead.

There's a few significant differences in semantics between raw mode and 
EXTPROC mode. The point of EXTPROC is to maintain the tty driver state 
as if cooked mode were still in effect. One obvious difference is that 
VMIN/VTIME are overlaid on VEOL/VEOF in raw mode, so the two are not 
directly interchangeable.

The fact that EXTPROC can be manually unset is by design. Quoting from 
the original again:

> stty.diff:
>     This file contains the changes needed for the stty(1) program
>     to report on the current status of the TS_EXTPROC bit.  It also
>     allows the user to turn on/off the TS_EXTPROC bit.  This is useful
>     because it allows the user to say "stty -extproc", and the
>     LINEMODE option will be automatically disabled, and saying "stty
>     extproc" will re-enable the LINEMODE option.


>>>>> 6. EXTPROC still does some input processing on the server. For example,
>>>>>       7-bit mode (ISTRIP), tolower mode (IUCLC) and processing while
>>>>>       closing; if input processing is being done on the local/client side,
>>>>>       why the extra work here?
>>>>
>>>> That's defensive, on the assumption that something else might break if e.g. the tty expected only 7-bit input but 8-bit characters were sent to it.
>>>
>>> Ok, is that because RFC 1116 doesn't specify ISTRIP and IUCLC handling so
>>> the server can't be sure the client did it? If so, that should be documented
>>> so that refactors don't remove that handling.
>
> Could you get back to me about this, as well?

The telnet protocol (RFC854) defines a Network Virtual Terminal (NVT) as 
using 7-bit USASCII in an 8-bit field. As such, it expects the client to 
be able to generate both upper and lower case itself, so there's no 
analogue to IUCLC, and there would be no reason to use ISTRIP.

RFC5198 updates the protocols to use UTF8. So again, it assumes full 
octets are being transmitted.

Perhaps we can drop these special cases from the driver.

>>> Regular ptys dump input while closing so this should too.
>>>
>>>>> 7. This needs a reference userspace implementation which for the moment
>>>>>       could double as regression testing. A library with unit tests would
>>>>>       be ideal.
>>>>
>>>> telnet/telnetd can probably used as a starting point for this.
>>>
>>> Except telnet is unmaintained except by each distro. Your patches don't apply
>>> to my distro telnet; they had been applied but are now ifdef'd out. I messed
>>> around with rebuilding it but never got telnetd to actually set EXTPROC.
>>>
>>> My point is there is no straightforward way to test this, and that's a problem.
>>>
>>> In fact, this is my main concern: no matter how useful this might be, it's
>>> pointless if the complementary userspace puzzle piece is unsupported.
>>> I know you have written patches but that's not the same as support; for example
>>> your OpenSSH fork is 4 years old, which is a non-starter because I'm sure
>>> there's known exploits which have been fixed in those 4 years.
>>>
>>> Why doesn't Ubuntu/Debian telnet even compile for line mode support? Did the
>>> package maintainer shut it off because it was causing bug reports?
>>
>> I don't know. Looking at the package history it appears they have never enabled it. https://launchpad.net/ubuntu/+source/netkit-telnet/+changelog
>>
>> I also don't know where the actual VCS repo is for this code.
>
> There isn't one. The base code is from downloaded tarball.
>
> I don't know what distro you use (or if you even do), but for Ubuntu/Debian
> the base package information is here https://packages.debian.org/wheezy/telnetd
> including maintainer email, etc.
>
> Typically, to get changes merged requires making a patch(es) against the package
> source and sending/uploading those.

This was already done, and the package maintainer said he was going to 
integrate it. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585527

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] n_tty: Remove LINEMODE support

[Howard Chu]

Peter Hurley wrote:
> Ok.
> I might see if I can get telnet + a revised kernel interface to work together; I'll
> let you know if this works out.

The patch here 
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585527#20 still 
applies cleanly to current Ubuntu netkit-telnet source and works.

You'll have to use a dumb shell to see any effect.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/19/2015 03:31 PM, Howard Chu wrote:
> Peter Hurley wrote:
>> Ok.
>> I might see if I can get telnet + a revised kernel interface to work together; I'll
>> let you know if this works out.
> 
> The patch here https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585527#20 still applies cleanly to current Ubuntu netkit-telnet source and works.

Aw snap. I just quickly glanced at the patch and source, and assumed
that the 4 year old patch had been applied.

> You'll have to use a dumb shell to see any effect.

I'm testing it now.

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/20/2015 09:53 AM, Peter Hurley wrote:
> On 01/19/2015 03:31 PM, Howard Chu wrote:
>> Peter Hurley wrote:
>>> Ok.
>>> I might see if I can get telnet + a revised kernel interface to work together; I'll
>>> let you know if this works out.
>>
>> The patch here https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=585527#20 still applies cleanly to current Ubuntu netkit-telnet source and works.
> 
> Aw snap. I just quickly glanced at the patch and source, and assumed
> that the 4 year old patch had been applied.
> 
>> You'll have to use a dumb shell to see any effect.
> 
> I'm testing it now.

Yeah, canonical read() does not behave as it should if EXTPROC
is enabled; it's returning both multiple lines and unterminated lines.

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/19/2015 02:43 PM, Howard Chu wrote:
> Peter Hurley wrote:
>> On 01/19/2015 11:36 AM, Howard Chu wrote:
>>> Peter Hurley wrote:
>>>> On 01/19/2015 07:46 AM, Howard Chu wrote:
>>>>> Peter Hurley wrote:

[...]

>>>> Which brings up another point: only a pty master should be able to set EXTPROC
>>>> mode. Right now, any tty can be set to EXTPROC and the pty slave can even
>>>> accidentally unset it. This argues for a new pty ioctl() to set EXTPROC in
>>>> termios. pty_set_termios() can silently merge the bit.
>>>
>>> Historically EXTPROC could also be set on regular ttys, for lines that were hooked up to special terminal interfaces. E.g. X.29 PADs that did their own input processing. I'm not sure how relevant this is now, but this is the reason EXTPROC is not a pty-specific ioctl.
>>
>> How does the remote end find out when non-canon mode is selected?
>> And who does the EXTPROC setting, because if it's the program that opened the
>> tty, then it can just select raw mode instead.
> 
> There's a few significant differences in semantics between raw mode and EXTPROC mode.

Sorry, I have this unfortunate habit of referring to non-canonical modes
as 'raw' (which stems from the way the terminal driver handles input).

> The point of EXTPROC is to maintain the tty driver state as if cooked mode were still in effect. One obvious difference is that VMIN/VTIME are overlaid on VEOL/VEOF in raw mode

Not in Linux; these are different c_cc[] slots.

> , so the two are not directly interchangeable.
> 
> The fact that EXTPROC can be manually unset is by design. Quoting from the original again:
> 
>> stty.diff:
>>     This file contains the changes needed for the stty(1) program
>>     to report on the current status of the TS_EXTPROC bit.  It also
>>     allows the user to turn on/off the TS_EXTPROC bit.  This is useful
>>     because it allows the user to say "stty -extproc", and the
>>     LINEMODE option will be automatically disabled, and saying "stty
>>     extproc" will re-enable the LINEMODE option.

This option is not supported by gnu coreutils.

So it's really back to the question of, does allowing EXTPROC for regular
ttys have _value_?


>>>>>> 6. EXTPROC still does some input processing on the server. For example,
>>>>>>       7-bit mode (ISTRIP), tolower mode (IUCLC) and processing while
>>>>>>       closing; if input processing is being done on the local/client side,
>>>>>>       why the extra work here?
>>>>>
>>>>> That's defensive, on the assumption that something else might break if e.g. the tty expected only 7-bit input but 8-bit characters were sent to it.
>>>>
>>>> Ok, is that because RFC 1116 doesn't specify ISTRIP and IUCLC handling so
>>>> the server can't be sure the client did it? If so, that should be documented
>>>> so that refactors don't remove that handling.
>>
>> Could you get back to me about this, as well?
> 
> The telnet protocol (RFC854) defines a Network Virtual Terminal (NVT) as using 7-bit USASCII in an 8-bit field. As such, it expects the client to be able to generate both upper and lower case itself, so there's no analogue to IUCLC, and there would be no reason to use ISTRIP.
> 
> RFC5198 updates the protocols to use UTF8. So again, it assumes full octets are being transmitted.
> 
> Perhaps we can drop these special cases from the driver.

I don't mind leaving it in, but without comments it looks like a
refactoring error.

Regards,
Peter Hurley

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/19/2015 02:43 PM, Howard Chu wrote:
> Peter Hurley wrote:
>> On 01/19/2015 11:36 AM, Howard Chu wrote:
>>> Peter Hurley wrote:

[...]

>>>> Which brings up another point: only a pty master should be able to set EXTPROC
>>>> mode. Right now, any tty can be set to EXTPROC and the pty slave can even
>>>> accidentally unset it. This argues for a new pty ioctl() to set EXTPROC in
>>>> termios. pty_set_termios() can silently merge the bit.

Another major problem with supporting EXTPROC through termios directly is
that Linux does not mask off unsupported termios bits, so the process can't
find out directly if the the kernel has this support or doesn't.

Re: [PATCH] n_tty: Remove LINEMODE support

[Howard Chu]

Peter Hurley wrote:
> On 01/19/2015 02:43 PM, Howard Chu wrote:
>> The fact that EXTPROC can be manually unset is by design. Quoting from the original again:
>>
>>> stty.diff:
>>>      This file contains the changes needed for the stty(1) program
>>>      to report on the current status of the TS_EXTPROC bit.  It also
>>>      allows the user to turn on/off the TS_EXTPROC bit.  This is useful
>>>      because it allows the user to say "stty -extproc", and the
>>>      LINEMODE option will be automatically disabled, and saying "stty
>>>      extproc" will re-enable the LINEMODE option.
>
> This option is not supported by gnu coreutils.

OK. It's in *BSD and Minix. Looks like I never wrote a patch for 
coreutils for this last time around.
>
> So it's really back to the question of, does allowing EXTPROC for regular
> ttys have _value_?

Does preventing it have value? I like having the option of turning 
linemode on and off in a session, for debugging purposes if nothing else.
>
>
>>>>>>> 6. EXTPROC still does some input processing on the server. For example,
>>>>>>>        7-bit mode (ISTRIP), tolower mode (IUCLC) and processing while
>>>>>>>        closing; if input processing is being done on the local/client side,
>>>>>>>        why the extra work here?
>>>>>>
>>>>>> That's defensive, on the assumption that something else might break if e.g. the tty expected only 7-bit input but 8-bit characters were sent to it.
>>>>>
>>>>> Ok, is that because RFC 1116 doesn't specify ISTRIP and IUCLC handling so
>>>>> the server can't be sure the client did it? If so, that should be documented
>>>>> so that refactors don't remove that handling.
>>>
>>> Could you get back to me about this, as well?
>>
>> The telnet protocol (RFC854) defines a Network Virtual Terminal (NVT) as using 7-bit USASCII in an 8-bit field. As such, it expects the client to be able to generate both upper and lower case itself, so there's no analogue to IUCLC, and there would be no reason to use ISTRIP.
>>
>> RFC5198 updates the protocols to use UTF8. So again, it assumes full octets are being transmitted.
>>
>> Perhaps we can drop these special cases from the driver.
>
> I don't mind leaving it in, but without comments it looks like a
> refactoring error.

I'm working up revisions for the patch.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Re: [PATCH] n_tty: Remove LINEMODE support

[Howard Chu]

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

Howard Chu wrote:
> Peter Hurley wrote:
>> On 01/19/2015 02:43 PM, Howard Chu wrote:
>>> The fact that EXTPROC can be manually unset is by design. Quoting
>>> from the original again:
>>>
>>>> stty.diff:
>>>>      This file contains the changes needed for the stty(1) program
>>>>      to report on the current status of the TS_EXTPROC bit.  It also
>>>>      allows the user to turn on/off the TS_EXTPROC bit.  This is useful
>>>>      because it allows the user to say "stty -extproc", and the
>>>>      LINEMODE option will be automatically disabled, and saying "stty
>>>>      extproc" will re-enable the LINEMODE option.
>>
>> This option is not supported by gnu coreutils.
>
> OK. It's in *BSD and Minix. Looks like I never wrote a patch for
> coreutils for this last time around.

Wrote it, just never submitted it.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

[-- Attachment #2: dif.txt --]
[-- Type: text/plain, Size: 414 bytes --]

--- stty.c	2009-04-24 13:41:19.000000000 +0100
+++ ../../../coreutils-7.4/src/stty.c	2010-06-17 06:48:42.000000000 +0100
@@ -330,6 +330,9 @@
   {"echoke", local, SANE_SET | REV, ECHOKE, 0},
   {"crtkill", local, REV | OMIT, ECHOKE, 0},
 #endif
+#ifdef EXTPROC
+  {"extproc", local, SANE_UNSET | REV, EXTPROC, 0},
+#endif
 
   {"evenp", combination, REV | OMIT, 0, 0},
   {"parity", combination, REV | OMIT, 0, 0},

Re: [PATCH] n_tty: Remove LINEMODE support

[Peter Hurley]

On 01/20/2015 01:39 PM, Howard Chu wrote:
> Peter Hurley wrote:
>> On 01/19/2015 02:43 PM, Howard Chu wrote:

[...]

>> So it's really back to the question of, does allowing EXTPROC for regular
>> ttys have _value_?
> 
> Does preventing it have value?

Not that that is the appropriate metric for kernel behavior,
but yes, preventing it has value: it significantly limits
the matrix required for testing.


> I like having the option of turning linemode on and off in a session, for debugging purposes if nothing else.

... which is just as achievable with a pty-specific ioctl.

Regards,
Peter Hurley