linux-kernel - Re: [PATCH] serial: 8250_omap: Avoid FIFO corruption caused by MDR1 access

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <b6bc434b-d75d-1f04-9bb2-457333c46a92@ti.com>
Date:   Fri, 11 Dec 2020 19:21:50 +0530
From:   Vignesh Raghavendra <vigneshr@...com>
To:     Alexander Sverdlin <alexander.sverdlin@...il.com>,
        <linux-serial@...r.kernel.org>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Peter Ujfalusi <peter.ujfalusi@...com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        <linux-kernel@...r.kernel.org>, <stable@...r.kernel.org>
Subject: Re: [PATCH] serial: 8250_omap: Avoid FIFO corruption caused by MDR1
 access

Hi,

On 12/10/20 11:22 AM, Alexander Sverdlin wrote:
> It has been observed that once per 300-1300 port openings the first
> transmitted byte is being corrupted on AM3352 ("v" written to FIFO appeared
> as "e" on the wire). It only happened if single byte has been transmitted
> right after port open, which means, DMA is not used for this transfer and
> the corruption never happened afterwards.
> 
> Therefore I've carefully re-read the MDR1 errata (link below), which says
> "when accessing the MDR1 registers that causes a dummy under-run condition
> that will freeze the UART in IrDA transmission. In UART mode, this may
> corrupt the transferred data". Strictly speaking,
> omap_8250_mdr1_errataset() performs a read access and if the value is the
> same as should be written, exits without errata-recommended FIFO reset.
> 
> A brief check of the serial_omap_mdr1_errataset() from the competing
> omap-serial driver showed it has no read access of MDR1. After removing the
> read access from omap_8250_mdr1_errataset() the data corruption never
> happened any more.
> 
> Link: https://www.ti.com/lit/er/sprz360i/sprz360i.pdf
> Fixes: 61929cf0169d ("tty: serial: Add 8250-core based omap driver")
> Cc: stable@...r.kernel.org
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...il.com>

Thanks for the fix.

Reviewed-by: Vignesh Raghavendra <vigneshr@...com>

> ---
>  drivers/tty/serial/8250/8250_omap.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 562087df7d33..0cc6d35a0815 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -184,11 +184,6 @@ static void omap_8250_mdr1_errataset(struct uart_8250_port *up,
>  				     struct omap8250_priv *priv)
>  {
>  	u8 timeout = 255;
> -	u8 old_mdr1;
> -
> -	old_mdr1 = serial_in(up, UART_OMAP_MDR1);
> -	if (old_mdr1 == priv->mdr1)
> -		return;
>  
>  	serial_out(up, UART_OMAP_MDR1, priv->mdr1);
>  	udelay(2);
>