linux-kernel - Re: [PATCH v6 1/6] serial: 8250: make saved LSR larger

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: <670010a1-7727-f2d9-87ad-18ddbeb0cbef@kernel.org>
Date:   Mon, 13 Jun 2022 10:26:49 +0200
From:   Jiri Slaby <jirislaby@...nel.org>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        linux-serial@...r.kernel.org, Greg KH <gregkh@...uxfoundation.org>,
        Paul Cercueil <paul@...pouillou.net>,
        linux-kernel@...r.kernel.org, linux-mips@...r.kernel.org
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Lukas Wunner <lukas.wunner@...el.com>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Subject: Re: [PATCH v6 1/6] serial: 8250: make saved LSR larger

On 13. 06. 22, 9:52, Ilpo Järvinen wrote:
> DW flags address received as BIT(8) in LSR. In order to not lose that
> on read, enlarge lsr_saved_flags to u16.
> 
> Adjust lsr/status variables and related call chains which used unsigned
> char type previously to unsigned int. Technically, some of these type
> conversion would not be needed but it doesn't hurt to be consistent.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
...
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -119,7 +119,7 @@ struct uart_8250_port {
>   	 * be immediately processed.
>   	 */
>   #define LSR_SAVE_FLAGS UART_LSR_BRK_ERROR_BITS
> -	unsigned char		lsr_saved_flags;
> +	u16			lsr_saved_flags;
>   #define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
>   	unsigned char		msr_saved_flags;
>   
> @@ -170,8 +170,8 @@ extern void serial8250_do_set_divisor(struct uart_port *port, unsigned int baud,
>   				      unsigned int quot_frac);
>   extern int fsl8250_handle_irq(struct uart_port *port);
>   int serial8250_handle_irq(struct uart_port *port, unsigned int iir);
> -unsigned char serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr);
> -void serial8250_read_char(struct uart_8250_port *up, unsigned char lsr);
> +unsigned int serial8250_rx_chars(struct uart_8250_port *up, unsigned int lsr);
> +void serial8250_read_char(struct uart_8250_port *up, unsigned int lsr);

It looks odd to have
   u16 lsr_saved_flags
in the struct and
   unsigned int lsr
here and there. You wrote:
   Technically, some of these type conversion would not be needed
   but it doesn't hurt to be consistent
But it looks like you actually made them a bit inconsistent. So why not 
use u16 for everyone?

thanks,
-- 
js
suse labs