linux-kernel - Re: [PATCH tty-next v1 1/4] serial: 8250: Use @ier bits to determine if Rx is stopped

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]

Message-ID: <Z2CQ8NvNhIOpPcBn@smile.fi.intel.com>
Date: Mon, 16 Dec 2024 22:43:28 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: John Ogness <john.ogness@...utronix.de>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>,
	Esben Haabendal <esben@...nix.com>, linux-serial@...r.kernel.org,
	linux-kernel@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
	Rengarajan S <rengarajan.s@...rochip.com>,
	Bart Van Assche <bvanassche@....org>,
	Niklas Schnelle <schnelle@...ux.ibm.com>,
	Serge Semin <fancer.lancer@...il.com>,
	Lino Sanfilippo <l.sanfilippo@...bus.com>,
	Peter Collingbourne <pcc@...gle.com>
Subject: Re: [PATCH tty-next v1 1/4] serial: 8250: Use @ier bits to determine
 if Rx is stopped

On Mon, Dec 16, 2024 at 10:31:06PM +0200, Andy Shevchenko wrote:
> On Mon, Dec 16, 2024 at 06:18:41PM +0106, John Ogness wrote:
> > Commit f19c3f6c8109 ("serial: 8250_port: Don't service RX FIFO if
> > throttled") uses @read_status_mask (bit UART_LSR_DR) to determine
> > if Rx has been stopped. However, the bit UART_LSR_DR is not
> > managed properly in @read_status_mask for all Rx stop/start
> > situations and is therefore not suitable for this purpose.
> > 
> > Use the UART_IER_RLSI and UART_IER_RDI bits in @ier instead, as
> > this is already common in 8250-variants and drivers.
> 
> Hmm... IER is Interrupt Enable Register, so it has been programmed to the value
> we control, on the opposite the LSR is Line Status Register and defines status
> on the line at the moment of reading. Can you elaborate how your change is correct
> substitute?

Additionally the common IRQ handler may be called at last in the custom ones
and hence potentially the value of saved IER might be different to what the
actual register is programmed to.

Besides that I don't remember all of the mysterious ways of DMA. May it affect
the value of IER and when we swtich from DMA to PIO and vice versa we get an
incorrect value in the saved variable?

-- 
With Best Regards,
Andy Shevchenko