linux-kernel - Re: [BACKPORT 4.14.y v2 6/6] serial: sprd: Modify the baud rate calculation formula

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: <20190905090130.GF1701@localhost>
Date:   Thu, 5 Sep 2019 11:01:30 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Baolin Wang <baolin.wang@...aro.org>
Cc:     stable@...r.kernel.org, gregkh@...uxfoundation.org,
        lanqing.liu@...soc.com, linux-serial@...r.kernel.org,
        arnd@...db.de, orsonzhai@...il.com, vincent.guittot@...aro.org,
        linux-kernel@...r.kernel.org
Subject: Re: [BACKPORT 4.14.y v2 6/6] serial: sprd: Modify the baud rate
 calculation formula

On Thu, Sep 05, 2019 at 11:11:26AM +0800, Baolin Wang wrote:
> From: Lanqing Liu <lanqing.liu@...soc.com>
> 
> [Upstream commit 5b9cea15a3de5d65000d49f626b71b00d42a0577]
> 
> When the source clock is not divisible by the expected baud rate and
> the remainder is not less than half of the expected baud rate, the old
> formular will round up the frequency division coefficient. This will
> make the actual baud rate less than the expected value and can not meet
> the external transmission requirements.
> 
> Thus this patch modifies the baud rate calculation formula to support
> the serial controller output the maximum baud rate.
> 
> Signed-off-by: Lanqing Liu <lanqing.liu@...soc.com>
> Signed-off-by: Baolin Wang <baolin.wang@...aro.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Baolin Wang <baolin.wang@...aro.org>
> ---
>  drivers/tty/serial/sprd_serial.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
> index e902494..72e96ab8 100644
> --- a/drivers/tty/serial/sprd_serial.c
> +++ b/drivers/tty/serial/sprd_serial.c
> @@ -380,7 +380,7 @@ static void sprd_set_termios(struct uart_port *port,
>  	/* ask the core to calculate the divisor for us */
>  	baud = uart_get_baud_rate(port, termios, old, 0, SPRD_BAUD_IO_LIMIT);
>  
> -	quot = (unsigned int)((port->uartclk + baud / 2) / baud);
> +	quot = port->uartclk / baud;

Are you sure the original patch is even correct?

By replacing the divisor rounding with truncation you are introducing
larger errors for some baud rates, something which could possibly even
break working systems.

Perhaps the original patch should even be reverted, but in any case
backporting this to stable looks questionable.

>  
>  	/* set data length */
>  	switch (termios->c_cflag & CSIZE) {

Johan