linux-kernel - Re: [PATCH RFC] pty: Add parity enabling routine

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: <2024041840-roundworm-sedate-9167@gregkh>
Date: Thu, 18 Apr 2024 14:53:43 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Esa Laakso <fidelix.laakso@...il.com>
Cc: jirislaby@...nel.org, linux-serial@...r.kernel.org,
	kari.argillander@...il.com, linux-kernel@...r.kernel.org,
	Esa Laakso <esa.laakso@...elix.com>
Subject: Re: [PATCH RFC] pty: Add parity enabling routine

On Thu, Apr 18, 2024 at 03:43:49PM +0300, Esa Laakso wrote:
> There are some cases where parity selection is required for passing
> it forward to a virtualized terminal. In this sepcific use-case, we
> want to use pty to send and receive serial data to a serial
> multiplexer. By using a pty, we avoid writing a custom tty driver.
> 
> There is very little evidence on the reasoning on why this option is
> hard-coded to be disabled. AFAIK it has been as such since 1996. With
> the lack of information about why this is, and based on the fact there
> are other similar fields that are not hard-coded, it is considered safe
> to enable this option.
> 
> Still, in order not to be too intrusive about the change, add it only on
> the condition that the termios flag `EXTPROC` is turned on. This way
> there is very little chance it will cause any unintended problems in any
> other implementation.

You need to document that EXTPROC thing somewhere, otherwise someone is
going to ask about this in 20 years and be confused :)

> Signed-off-by: Esa Laakso <esa.laakso@...elix.com>
> Signed-off-by: Esa Laakso <fidelix.laakso@...il.com>

Do not sign off on something twice, use your real email address only
once, that's all that is needed.

> ---
> 
> We are looking for some assistance on this patch, or just a green light
> to submit it, if it is good to go.
> 
> We would need to know if the change is valid in context of pty, as
> there seems to be fairly little information about the reasoning behind
> the hard-coded values.
> 
> Our guess is that they have been as such forever and nobody has had a
> reason to change them. We have a reason to change them, and we would
> like to know if it is safe to do so and possibly contribute in the
> process. Either by just using the patch for ourselves or submitting
> it here.
> 
> For the record, this is my first patch submitted upstream. Please let
> me know if something is incorrect or missing.
> ---
> 
>  drivers/tty/pty.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index 07394fdaf522..e2d9718dcea0 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -267,7 +267,9 @@ static void pty_set_termios(struct tty_struct *tty,
>  		}
>  	}
>  
> -	tty->termios.c_cflag &= ~(CSIZE | PARENB);
> +	tty->termios.c_cflag &= ~(CSIZE);
> +	if (!L_EXTPROC(tty))
> +		tty->termios.c_cflag &= ~(PARENB);

Some description of how you tested this in the changelog would also be
good, and a comment here too as to what you are doing.

thanks,

greg k-h