linux-kernel - Re: [External] Re: [PATCH v9 2/4] serial: 8250_dw: fix PSLVERR on RX

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

Message-ID: <84ikjqaoqi.fsf@jogness.linutronix.de>
Date: Thu, 17 Jul 2025 16:20:29 +0206
From: John Ogness <john.ogness@...utronix.de>
To: yunhui cui <cuiyunhui@...edance.com>, Douglas Anderson
 <dianders@...omium.org>
Cc: arnd@...db.de, andriy.shevchenko@...ux.intel.com,
 benjamin.larsson@...exis.eu, gregkh@...uxfoundation.org,
 heikki.krogerus@...ux.intel.com, ilpo.jarvinen@...ux.intel.com,
 jirislaby@...nel.org, jkeeping@...usicbrands.com,
 linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
 markus.mayer@...aro.org, matt.porter@...aro.org, namcao@...utronix.de,
 paulmck@...nel.org, pmladek@...e.com, schnelle@...ux.ibm.com,
 sunilvl@...tanamicro.com, tim.kryger@...aro.org, stable@...r.kernel.org
Subject: Re: [External] Re: [PATCH v9 2/4] serial: 8250_dw: fix PSLVERR on
 RX_TIMEOUT

Added Douglas Anderson, author of commit 424d79183af0 ("serial: 8250_dw:
Avoid "too much work" from bogus rx timeout interrupt").

On 2025-07-11, yunhui cui <cuiyunhui@...edance.com> wrote:
>> On 2025-06-23, yunhui cui <cuiyunhui@...edance.com> wrote:
>> >> The DW UART may trigger the RX_TIMEOUT interrupt without data
>> >> present and remain stuck in this state indefinitely. The
>> >> dw8250_handle_irq() function detects this condition by checking
>> >> if the UART_LSR_DR bit is not set when RX_TIMEOUT occurs. When
>> >> detected, it performs a "dummy read" to recover the DW UART from
>> >> this state.
>> >>
>> >> When the PSLVERR_RESP_EN parameter is set to 1, reading the UART_RX
>> >> while the FIFO is enabled and UART_LSR_DR is not set will generate a
>> >> PSLVERR error, which may lead to a system panic. There are two methods
>> >> to prevent PSLVERR: one is to check if UART_LSR_DR is set before reading
>> >> UART_RX when the FIFO is enabled, and the other is to read UART_RX when
>> >> the FIFO is disabled.
>> >>
>> >> Given these two scenarios, the FIFO must be disabled before the
>> >> "dummy read" operation and re-enabled afterward to maintain normal
>> >> UART functionality.
>> >>
>> >> Fixes: 424d79183af0 ("serial: 8250_dw: Avoid "too much work" from bogus rx timeout interrupt")
>> >> Signed-off-by: Yunhui Cui <cuiyunhui@...edance.com>
>> >> Cc: stable@...r.kernel.org
>> >> ---
>> >>  drivers/tty/serial/8250/8250_dw.c | 10 +++++++++-
>> >>  1 file changed, 9 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
>> >> index 1902f29444a1c..082b7fcf251db 100644
>> >> --- a/drivers/tty/serial/8250/8250_dw.c
>> >> +++ b/drivers/tty/serial/8250/8250_dw.c
>> >> @@ -297,9 +297,17 @@ static int dw8250_handle_irq(struct uart_port *p)
>> >>                 uart_port_lock_irqsave(p, &flags);
>> >>                 status = serial_lsr_in(up);
>> >>
>> >> -               if (!(status & (UART_LSR_DR | UART_LSR_BI)))
>> >> +               if (!(status & (UART_LSR_DR | UART_LSR_BI))) {
>> >> +                       /* To avoid PSLVERR, disable the FIFO first. */
>> >> +                       if (up->fcr & UART_FCR_ENABLE_FIFO)
>> >> +                               serial_out(up, UART_FCR, 0);
>> >> +
>> >>                         serial_port_in(p, UART_RX);
>> >>
>> >> +                       if (up->fcr & UART_FCR_ENABLE_FIFO)
>> >> +                               serial_out(up, UART_FCR, up->fcr);
>> >> +               }
>> >> +
>> >>                 uart_port_unlock_irqrestore(p, flags);
>> >>         }
>>
>> I do not know enough about the hardware. Is a dummy read really the only
>> way to exit the RX_TIMEOUT state?
>>
>> What if there are bytes in the TX-FIFO. Are they in danger of being
>> cleared?
>>
>> From [0] I see:
>>
>> "Writing a "0" to bit 0 will disable the FIFOs, in essence turning the
>>  UART into 8250 compatibility mode. In effect this also renders the rest
>>  of the settings in this register to become useless. If you write a "0"
>>  here it will also stop the FIFOs from sending or receiving data, so any
>>  data that is sent through the serial data port may be scrambled after
>>  this setting has been changed. It would be recommended to disable FIFOs
>>  only if you are trying to reset the serial communication protocol and
>>  clearing any working buffers you may have in your application
>>  software. Some documentation suggests that setting this bit to "0" also
>>  clears the FIFO buffers, but I would recommend explicit buffer clearing
>>  instead using bits 1 and 2."
>>
>> Have you performed tests where you fill the TX-FIFO and then
>> disable/enable the FIFO to see if the TX-bytes survive?
>
> Sorry, I haven't conducted relevant tests. The reason I made this
> modification is that it clearly contradicts the logic of avoiding
> PSLVERR. Disabling the FIFO can at least prevent the Panic() caused by
> PSVERR.

I am just wondering if there is some other way to avoid this. But since
we are talking about a hardware quirk and it is only related to
suspend/resume, maybe it is acceptable to risk data corruption in this
case. (?)

I am hoping Douglas can chime in.

John Ogness

>> [0] https://en.wikibooks.org/wiki/Serial_Programming/8250_UART_Programming