linux-kernel - Re: [PATCH v4 0/3] tty/serial: Process XON/XOFF robustly

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] [day] [month] [year] [list]

Date:   Thu, 19 May 2022 18:34:44 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc:     linux-serial@...r.kernel.org, Jiri Slaby <jirislaby@...nel.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linux-kernel@...r.kernel.org,
        Gilles Buloz <gilles.buloz@...tron.com>,
        Johan Hovold <johan@...nel.org>
Subject: Re: [PATCH v4 0/3] tty/serial: Process XON/XOFF robustly

On Tue, Apr 26, 2022 at 05:49:32PM +0300, Ilpo Järvinen wrote:
> For this v4, I dropped Gilles' Tested-by as I made rather major
> modifications to the patch series so I don't want to claim he has
> tested this version (earlier versions are known to fix his problem and
> this one should too).
> 
> XON/XOFF are used for software flow-control on serial lines. XON and
> XOFF appear as characters within the stream but should be processed as
> soon as possible.
> 
> The characters received by the UART drivers are in intermediate buffers
> until TTY receives them. In the case where the TTY is not read from,
> the characters may get stuck into those intermediate buffers until
> user-space reads from the TTY. Among the characters stuck in the
> buffers, can also be those XON/XOFF flow control characters. A stuck
> flow-control character is not very useful.
> 
> This patch series addresses the issue by checking if TTY is slow to
> process characters, that is, eats less than the given amount. If TTY is
> slow, a lookahead is invoked for the characters that remain in the
> intermediate buffer(s).
> 
> Then at a later time, receive_buf needs to ensure the flow-control
> actions are not retaken when those same characters get pushed to TTY.
> 
> This patch series fixes an issue but I'm not able to pinpoint to a
> specific commit id to provide a Fixes tag. The last patch of the series
> is not needed for minimal fix (and has a small chance of regression
> too), thus that patch shouldn't be sent to stable.
> 
> v1 -> v2:
> - Add flow ctrl char funcs in separate change & rework logic a bit.
>   I believe it's now cleaner than it would have been with the
>   suggestions during v1 review, please recheck.
> - Renamed n_tty_lookahead_special to n_tty_lookahead_flow_ctrl
> - Fixed logic for START_CHAR == STOP_CHAR case
> - Use unsigned int for lookahead_count in receive_buf call chain
> - Use consistent types in lookahead call chain
> - Improved indentation & line splits for function params
> - Corrected tty_ldisc.h comments documenting tty_ldisc_ops
> - Tweaked comment format
> 
> v2 -> v3:
> - Split preparatory patch moving/rearranging code to two
> - Fix closing path giving change for ... || xx to execute
>   instead of skipping the flow-control char
> - Use the same flow-control char function on closing path
>   (just a cleanup, non-fix as last patch, a small change of
>   regression exists)
> 
> v3 -> v4:
> - Rework lookahead_count, it is now kept internal to n_tty ldisc rather
>   than passed around through the whole callchain
> - Dropped Gilles' Tested-by due to major changes
> - Improve comments & changelogs
> 
> Ilpo Järvinen (3):
>   tty: Rework receive flow control char logic
>   tty: Implement lookahead to process XON/XOFF timely
>   tty: Use flow-control char function on closing path
> 
>  drivers/tty/n_tty.c        | 107 ++++++++++++++++++++++++++++---------
>  drivers/tty/tty_buffer.c   |  59 ++++++++++++++++----
>  drivers/tty/tty_port.c     |  21 ++++++++
>  include/linux/tty_buffer.h |   1 +
>  include/linux/tty_ldisc.h  |  13 +++++
>  include/linux/tty_port.h   |   2 +
>  6 files changed, 170 insertions(+), 33 deletions(-)
> 
> -- 
> 2.30.2
> 

I took patch 1/3 of this series as it is "obviously correct".  I'd like
others to review the rest before taking them, and I don't have the
bandwidth today to do so, so I'll wait for others before considering
them.

thanks,

greg k-h