linux-kernel - Re: [PATCH 1/1] pty: BREAK for pseudoterminals

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: <54E219B0.2070206@hurleysoftware.com>
Date:	Mon, 16 Feb 2015 11:24:16 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	Petr Tesarik <ptesarik@...e.cz>
CC:	Nan Li <nli@...e.com>, gregkh@...uxfoundation.org, jslaby@...e.cz,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] pty: BREAK for pseudoterminals

Hi Petr,

On 02/16/2015 08:22 AM, Petr Tesarik wrote:
> On Mon, 16 Feb 2015 08:04:02 -0500
> Peter Hurley <peter@...leysoftware.com> wrote:
> 
>> On 02/05/2015 02:11 PM, Nan Li wrote:
>>> This will greatly enhance the usefulness of QEMU virtual serial ports, because the Linux kernel interprets a break on the serial console as a SysRq, but there is currently no way to pass this signal over a pseudo-terminal. This patch will work for transmitting BREAK from master to slave through pseudo-terminal.
>>
>> pty is an IPC mechanism, not a virtualization driver.
> 
> No, but it can be used as a TTY. Teletypes have always had the capacity
> to send and receive BREAK.

In some general-purpose but restricted capacity, the *slave* end _mimics_
a tty. That doesn't mean that it is suitable for every conceivable
use as a tty, nor should it.

If BREAK was actually useful for real terminal i/o, the pty driver would
already support this. Note the pty driver doesn't emulate modem signals
either, for the same reason.

>> Those programs that know they're consuming from a slave pty may not
>> have bothered to set termios for BREAKs, and may not be prepared
>> for NULLs in the input stream.
> 
> Well, the default termios includes IGNBRK, so unless they bothered to
> do anything about BREAKs, they won't see any change.

Userspace programs are sloppy, especially with terminal i/o and settings.
Unlikely is not the same as not possible.

> Anyway, the current kernel behaviour is clearly suboptimal. Calling
> tcsendbreak() on a pty descriptor does nothing but reports success.
> There are obviously two ways to fix it: either report an error, or
> deliver the BREAK for real.

The pty master end is even less of a tty than the slave end, but this
isn't really about errno. This patch doesn't address either of your
points wrt tcsendbreak() on the slave descriptor which is the actual
terminal end.

> This patch implements the latter, adding at least one valid use case
> to explain why it is better than the former.

I disagree that this is a valid use case for the _pty driver_.

AFAICT this is simply for convenience, as sysrq functionality is
already available via sendkey.

Regards,
Peter Hurley


>>> Signed-off-by: Nan Li <nli@...e.com>
>>> ---
>>>  drivers/tty/pty.c | 19 +++++++++++++++++++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
>>> index e72ee62..ac8893a 100644
>>> --- a/drivers/tty/pty.c
>>> +++ b/drivers/tty/pty.c
>>> @@ -31,6 +31,7 @@
>>>  static struct tty_driver *ptm_driver;
>>>  static struct tty_driver *pts_driver;
>>>  static DEFINE_MUTEX(devpts_mutex);
>>> +#define BREAK_STRING '\0'
>>>  #endif
>>>  
>>>  static void pty_close(struct tty_struct *tty, struct file *filp)
>>> @@ -674,6 +675,23 @@ static void pty_unix98_shutdown(struct
>>> tty_struct *tty) devpts_kill_index(tty->driver_data, tty->index);
>>>  }
>>>  
>>> +static int pty_unix98_break_ctl(struct tty_struct *tty, int
>>> break_state) +{
>>> +	int c;
>>> +	struct tty_struct *to = tty->link;
>>> +
>>> +	if (break_state) {
>>> +		/* Stuff the break data into the input queue of
>>> the other end */
>>> +		c = tty_insert_flip_char(to->port, BREAK_STRING,
>>> TTY_BREAK);
>>> +		/* And shovel */
>>> +		if (c)
>>> +			tty_flip_buffer_push(to->port);
>>> +		else
>>> +			return -ENOMEM;
>>> +	}
>>> +	return 0;
>>> +}
>>> +
>>>  static const struct tty_operations ptm_unix98_ops = {
>>>  	.lookup = ptm_unix98_lookup,
>>>  	.install = pty_unix98_install,
>>> @@ -686,6 +704,7 @@ static const struct tty_operations
>>> ptm_unix98_ops = { .chars_in_buffer = pty_chars_in_buffer,
>>>  	.unthrottle = pty_unthrottle,
>>>  	.ioctl = pty_unix98_ioctl,
>>> +	.break_ctl = pty_unix98_break_ctl,
>>>  	.resize = pty_resize,
>>>  	.shutdown = pty_unix98_shutdown,
>>>  	.cleanup = pty_cleanup
>>>
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/