[PATCH v1 1/1] serial: core: Clearing the circular buffer before NULLifying it

From: Andy Shevchenko
Date: Thu Apr 04 2024 - 11:02:05 EST


The circular buffer is NULLified in uart_tty_port_shutdown()
under the spin lock. However, the PM or other timer based callbacks
may still trigger after this event without knowning that buffer pointer
is not valid. Since the serial code is a bit inconsistent in checking
the buffer state (some rely on the head-tail positions, some on the
buffer pointer), it's better to have both aligned, i.e. buffer pointer
to be NULL and head-tail possitions to be the same, meaning it's empty.
This will prevent asynchronous calls to dereference NULL pointer as
reported recently in 8250 case:

BUG: kernel NULL pointer dereference, address: 00000cf5
Workqueue: pm pm_runtime_work
EIP: serial8250_tx_chars (drivers/tty/serial/8250/8250_port.c:1809)
...
? serial8250_tx_chars (drivers/tty/serial/8250/8250_port.c:1809)
__start_tx (drivers/tty/serial/8250/8250_port.c:1551)
serial8250_start_tx (drivers/tty/serial/8250/8250_port.c:1654)
serial_port_runtime_suspend (include/linux/serial_core.h:667 drivers/tty/serial/serial_port.c:63)
__rpm_callback (drivers/base/power/runtime.c:393)
? serial_port_remove (drivers/tty/serial/serial_port.c:50)
rpm_suspend (drivers/base/power/runtime.c:447)

The proposed change will prevent ->start_tx() to be called during
suspend on shut down port.

Fixes: 43066e32227e ("serial: port: Don't suspend if the port is still busy")
Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
Closes: https://lore.kernel.org/oe-lkp/202404031607.2e92eebe-lkp@xxxxxxxxx
Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
---

I have got into the very similar issue while working on max3100 driver.
I haven't checked the 8250 case, but for mine the culprit is the same
and this patch fixes it. Hence I assume it will fix the 8250 case as
well.

drivers/tty/serial/serial_core.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index a005fc06a077..ba3a674a8bbf 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1788,6 +1788,7 @@ static void uart_tty_port_shutdown(struct tty_port *port)
* Free the transmit buffer.
*/
uart_port_lock_irq(uport);
+ uart_circ_clear(&state->xmit);
buf = state->xmit.buf;
state->xmit.buf = NULL;
uart_port_unlock_irq(uport);
--
2.43.0.rc1.1.gbec44491f096