linux-kernel - Re: 8250_omap: incorrect RS485 RTS on close during transmission

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: <bce6407bc42eeec8f459f09bc1326761327c98ec.camel@ew.tq-group.com>
Date:   Tue, 22 Nov 2022 09:52:40 +0100
From:   Matthias Schiffer <matthias.schiffer@...tq-group.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        Lukas Wunner <lukas@...ner.de>
Cc:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: 8250_omap: incorrect RS485 RTS on close during transmission

On Fri, 2022-11-18 at 12:57 +0100, Matthias Schiffer wrote:
> Hi Lukas,
> 
> I've discovered a new issue with EM485 in 8250_omap (and probably other
> serial drivers as well?): When closing the TTY device while there is
> still an ongoing transmission, the RTS pin will get stuck active until
> the TTY is opened again. This can be easily reproduced by running `cat
> /dev/zero > /dev/ttySx` and then stopping it using Ctrl-C.
> 
> The issue exists in current mainline (6.1-rc5+), and applying "serial:
> 8250: 8250_omap: Avoid RS485 RTS glitch on ->set_termios()" from next
> doesn't solve it. I think it should work with "serial: 8250: 8250_omap:
> Support native RS485", but we also need RS485 on AM57xx CPUs, which
> don't have native RS485.
> 
> I intend to look into this myself next week, but as you've worked on
> this code a lot lately, maybe you already have an idea how to fix it?
> 
> Regards,
> Matthias
> 

Okay, I've narrowed down the issue to uart_shutdown() ->
uart_port_shutdown() - here, ops->shutdown() is called, which clears
the FIFOs (in the case of the default 8250_port and 8250_omap)
implementations, but doesn't wait for the clear to finish, and doesn't
call ops->stop_tx().

I believe a similar problem can occur in uart_suspend_port(). Here,
ops->stop_tx() is called before ops->shutdown(), but stop_tx() is a no-
op when the TX FIFO is not empty (again, for the 8250 case - I haven't
really looked at other drivers).

Given that EM485 is handled in 8250_port by __stop_tx(), which is
called both internally in the 8250 driver and from the serial core
through stop_tx(), it is not clear to me what the correct layer to fix
this is. Two ideas:

(1) Have ops->shutdown() wait until the TX FIFO is empty. Call
ops->stop_tx() after ops->shutdown() in serial core. For this to work,
calling stop_tx() after shutdown() must be allowed in all drivers; I'm
not sure if this is currently the case, and if it is desirable at all.

(2) Do the whole handling in the driver's ops->shutdown(), do not
further involve the serial core.

So far, I've implemented (1) locally for 8250_omap, and together with
"serial: 8250: 8250_omap: Avoid RS485 RTS glitch on ->set_termios()" it
fixes the issue on my test devices, but I'm not too happy about the
"shutdown then tx_stop" order of calls. So maybe (2) is the better
solution after all?