linux-kernel - Re: [PATCH 3/3] serial: qcom-geni: fix garbage output after buffer flush

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]

Date: Wed, 26 Jun 2024 10:01:39 +0200
From: Johan Hovold <johan@...nel.org>
To: Doug Anderson <dianders@...omium.org>
Cc: Johan Hovold <johan+linaro@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>,
	Konrad Dybcio <konrad.dybcio@...aro.org>,
	Bjorn Andersson <andersson@...nel.org>,
	linux-arm-msm@...r.kernel.org, linux-serial@...r.kernel.org,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 3/3] serial: qcom-geni: fix garbage output after buffer
 flush

On Mon, Jun 24, 2024 at 03:19:33PM -0700, Doug Anderson wrote:
> On Mon, Jun 24, 2024 at 6:31 AM Johan Hovold <johan+linaro@...nel.org> wrote:
> >
> > The Qualcomm GENI serial driver does not handle buffer flushing and
> > outputs garbage (or NUL) characters for the remainder of any active TX
> > command after the write buffer has been cleared.
> >
> > Implement the flush_buffer() callback and use it to cancel any active TX
> > command when the write buffer has been emptied.
> 
> I could be reading it wrong, but in the kernel-doc of `struct
> tty_ldisc_ops` it seems to indicate that flush_buffer() is for the
> other direction. Specifically, it says:
> 
> This function instructs the line discipline to clear its buffers of
> any input characters it may have queued to be delivered to the user
> mode process.

Yes, but this a uart op (i.e. not tty_ldisc_ops), for which the doc
states:

	Flush any write buffers, reset any DMA state and stop any
	ongoing DMA transfers.

> I guess the underlying worry I have is that there's no guarantee that
> the flush function will be called when the kfifo loses bytes. If it
> ever happens we'll fall back to writing NUL bytes out and that doesn't
> seem amazing to me. To me it feels like
> qcom_geni_serial_send_chunk_fifo() should detect this situation and
> then it should be responsible for canceling, though better (in my
> mind) is if we never initiate any big transfers if we can get away
> with that and still be performant.

The flush buffer callback is called from the uart_flush_buffer() tty
operation (again, not tty_ldisc_ops) when the FIFO is reset.

Johan