[PATCH 00/24] tty/serial: a bunch of llm-found fixes

[PATCH 00/24] tty/serial: a bunch of llm-found fixes

[Greg Kroah-Hartman]

Here's a series of some small llm-found fixes that have been sent to me
recently.  Most of them are just minor bugfixes and locking cleanups,
but they are good to have from what I can tell.

Submitting them here to get a solid Sashiko run and some second eyes on
them before merging if all goes well.

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Chris Mason (1):
      vt: serialise the producers of the flip buffer

Josef Bacik (12):
      serial: core: respect runtime PM when sending x_char
      serial: 8250: don't wait for the em485 timers locked
      serial: 8250: don't report a wakeup to a closed tty
      serial: 8250_lpss: don't divide by 0 above 6.25 Mbaud
      serial: 8250_mxpcie: handle hangup during break
      serial: fsl_lpuart: fix tty ref of wakeup event
      serial: imx: fix the tty reference of the wakeup event
      serial: rda: free the interrupt on shutdown
      serial: samsung: don't leak the tty on an RX IRQ
      serial: samsung: put the previous baud clock
      serial: st-asc: don't report a wakeup to a closed tty
      serial: stm32: don't report a wakeup to a closed tty

Joshua Rogers (11):
      serial: 8250: clear subdriver state on port unregister
      serial: 8250: add bounds check for line array in setup_serial()
      serial: 8250: hold hash_mutex across irq_info lookup and linking
      serial: 8250: cancel overrun backoff work on port shutdown and unregister
      serial: 8250: 8250_bcm7271: fix DMA address truncation to 32 bits
      serial: 8250: pericom: validate port offset against BAR length
      serial: 8250: cap OxSemi Tornado UART count against BAR size
      serial: 8250: dfl: add bounds check for device-provided regshift
      serial: 8250: dfl: validate MMIO resource size and regshift
      serial: 8250: ioc3: store registered line number in driver data
      serial: 8250: omap: acquire rx_dma_lock in DMA completion callback

 drivers/tty/serial/8250/8250.h         |  1 +
 drivers/tty/serial/8250/8250_bcm7271.c |  2 +-
 drivers/tty/serial/8250/8250_core.c    | 41 ++++++++++++++++---
 drivers/tty/serial/8250/8250_dfl.c     | 16 ++++++++
 drivers/tty/serial/8250/8250_ioc3.c    |  1 +
 drivers/tty/serial/8250/8250_lpss.c    |  3 ++
 drivers/tty/serial/8250/8250_mxpcie.c  | 13 ++++--
 drivers/tty/serial/8250/8250_omap.c    |  5 +++
 drivers/tty/serial/8250/8250_pci.c     | 10 +++++
 drivers/tty/serial/8250/8250_pericom.c |  3 ++
 drivers/tty/serial/8250/8250_port.c    | 74 ++++++++++++++++++++++++++++------
 drivers/tty/serial/8250/serial_cs.c    |  5 +++
 drivers/tty/serial/fsl_lpuart.c        | 10 ++++-
 drivers/tty/serial/imx.c               | 10 ++++-
 drivers/tty/serial/rda-uart.c          |  2 +
 drivers/tty/serial/samsung_tty.c       |  8 ++--
 drivers/tty/serial/serial_core.c       | 25 +++++++-----
 drivers/tty/serial/serial_port.c       |  4 +-
 drivers/tty/serial/st-asc.c            | 11 ++++-
 drivers/tty/serial/stm32-usart.c       | 11 ++++-
 drivers/tty/vt/keyboard.c              |  6 +++
 drivers/tty/vt/vt.c                    |  2 +
 include/linux/serial_8250.h            |  1 +
 23 files changed, 217 insertions(+), 47 deletions(-)
---
base-commit: d135ab453a7400a34930bdda1ef4bdc8acfac80e
change-id: 20261027-tty-llm-82725ab5b9b3

Best regards,
--  
Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[PATCH 01/24] vt: serialise the producers of the flip buffer

[Greg Kroah-Hartman]

From: Chris Mason <mason@kernel.org>

Typing on a virtual console while the console replies to an escape
sequence can make the kernel write past the end of a buffer. A user who
can write to the foreground console can cause this on purpose if they
can also inject key events, through /dev/uinput or by writing to the
/dev/input/event* node of a keyboard.

Input for a tty goes through the flip buffer of its port. A driver
inserts the data with tty_insert_flip_*() and then calls
tty_flip_buffer_push() to pass it on to the line discipline. The insert
functions take no lock: they check that the current struct tty_buffer
has room, copy the data to offset ->used, and then advance ->used. So a
flip buffer supports only one producer at a time, and a driver that has
several must serialise them.

Four functions feed the flip buffer of a virtual console:

- put_queue(), puts_queue() and fn_send_intr() insert typed characters
  and breaks for the keyboard handler. They run under kbd_event_lock.
- respond_string() inserts the reply when a program writes an escape
  sequence that requests one, such as a cursor position report. It runs
  under the console lock.

The keyboard handler gets its events from the input layer and feeds the
foreground console. Key events that a program writes to /dev/uinput or
to an event node reach it in the same way as typed keys.

The keyboard handler and respond_string() share no lock, so they can
interleave:

    keyboard handler                respond_string()
    ================                ================
    room check passes
                                    room check passes
    memcpy() to offset used
    used += len
                                    memcpy() to offset used
                                    used += len
                                      /* used > size of the buffer */

The room check subtracts ->used from the size, and both are unsigned.
Once ->used exceeds the size, the result wraps, so the check always
passes, and each later insert lands further past the buffer.

Reproduced as root, with keys from a uinput keyboard and cursor position
reports on the same console: KFENCE reported an out-of-bounds write
1,024 bytes past the buffer in under 10 s.

Fix this by taking port->lock, with interrupts disabled, around the
insert and the push in these four functions. kbd_keycode() already takes
port->lock under kbd_event_lock, so this lock ordering is not new.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Nicolas Pitre <npitre@baylibre.com>
Cc: Kees Cook <kees+treewide@kernel.org>
Cc: Joshua Rogers <linux@joshua.hu>
Cc: Chris Mason <mason@kernel.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Mingyu Wang <25181214217@stu.xidian.edu.cn>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Ronan Pigott <ronan@rjp.ie>
Cc: Liav Mordouch <liavmordouch@gmail.com>
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/vt/keyboard.c | 6 ++++++
 drivers/tty/vt/vt.c       | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index ef4a19aeb3ce..f4b742e091fb 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -322,15 +322,20 @@ int kbd_rate(struct kbd_repeat *rpt)
 
 /*
  * Helper Functions.
+ *
+ * A flip buffer allows one producer at a time, and respond_string() in vt.c
+ * feeds the same one under the console lock. So take the lock of the port.
  */
 static void put_queue(struct vc_data *vc, int ch)
 {
+	guard(spinlock_irqsave)(&vc->port.lock);
 	tty_insert_flip_char(&vc->port, ch, 0);
 	tty_flip_buffer_push(&vc->port);
 }
 
 static void puts_queue(struct vc_data *vc, const char *cp)
 {
+	guard(spinlock_irqsave)(&vc->port.lock);
 	tty_insert_flip_string(&vc->port, cp, strlen(cp));
 	tty_flip_buffer_push(&vc->port);
 }
@@ -587,6 +592,7 @@ static void fn_inc_console(struct vc_data *vc)
 
 static void fn_send_intr(struct vc_data *vc)
 {
+	guard(spinlock_irqsave)(&vc->port.lock);
 	tty_insert_flip_char(&vc->port, 0, TTY_BREAK);
 	tty_flip_buffer_push(&vc->port);
 }
diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 8415e274e4be..5c2c761aa0c1 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -1971,6 +1971,8 @@ static void csi_m(struct vc_data *vc)
 
 static void respond_string(const char *p, size_t len, struct tty_port *port)
 {
+	/* The keyboard handler feeds the same flip buffer. */
+	guard(spinlock_irqsave)(&port->lock);
 	tty_insert_flip_string(port, p, len);
 	tty_flip_buffer_push(port);
 }

-- 
2.56.0

[PATCH 02/24] serial: core: respect runtime PM when sending x_char

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

__uart_start() calls the start_tx operation only if the port is runtime
active.  Otherwise it asks for a resume, and
serial_port_runtime_resume() starts the transmitter.

uart_send_xchar() calls start_tx at once, under the port lock, whatever
the state of the device.  With 8250_omap, serial8250_start_tx() then
calls pm_runtime_get_sync() for a suspended device: that sleeps, and
omap8250_runtime_resume() takes the port lock again.  A tcflow(TCIOFF)
on an idle port is enough, if autosuspend has been enabled.  It was
allowed as long as that driver used pm_runtime_irq_safe().

Share the part of __uart_start() that deals with runtime PM.  A stopped
transmission still does not hold up an XON or XOFF character.  Let a
pending x_char count as work for serial_port_runtime_resume(), and keep
the port from being suspended with it.

Fixes: 8700a7ea5519 ("serial: 8250_omap: Drop pm_runtime_irq_safe()")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: Johan Hovold <johan@kernel.org>
Cc: Karl Mehltretter <kmehltretter@gmail.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: Tony Lindgren <tony.lindgren@linux.intel.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/serial_core.c | 25 +++++++++++++++----------
 drivers/tty/serial/serial_port.c |  4 ++--
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index c0deec0e6af4..b06d2649be34 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -135,17 +135,11 @@ static void uart_stop(struct tty_struct *tty)
 	uart_port_unlock_deref(port, flags);
 }
 
-static void __uart_start(struct uart_state *state)
+static void uart_port_start_tx(struct uart_port *port)
 {
-	struct uart_port *port = state->uart_port;
-	struct serial_port_device *port_dev;
+	struct serial_port_device *port_dev = port->port_dev;
 	int err;
 
-	if (!port || port->flags & UPF_DEAD || uart_tx_stopped(port))
-		return;
-
-	port_dev = port->port_dev;
-
 	/* Increment the runtime PM usage count for the active check below */
 	err = pm_runtime_get(&port_dev->dev);
 	if (err < 0 && err != -EINPROGRESS) {
@@ -164,6 +158,16 @@ static void __uart_start(struct uart_state *state)
 	pm_runtime_put_autosuspend(&port_dev->dev);
 }
 
+static void __uart_start(struct uart_state *state)
+{
+	struct uart_port *port = state->uart_port;
+
+	if (!port || port->flags & UPF_DEAD || uart_tx_stopped(port))
+		return;
+
+	uart_port_start_tx(port);
+}
+
 static void uart_start(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
@@ -718,8 +722,9 @@ static void uart_send_xchar(struct tty_struct *tty, u8 ch)
 	else {
 		guard(uart_port_lock_irqsave)(port);
 		port->x_char = ch;
-		if (ch)
-			port->ops->start_tx(port);
+		/* It goes out even if the transmission is stopped */
+		if (ch && !(port->flags & UPF_DEAD))
+			uart_port_start_tx(port);
 	}
 	uart_port_deref(port);
 }
diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c
index 2fc48cd63f6c..f2f7cdd3a54c 100644
--- a/drivers/tty/serial/serial_port.c
+++ b/drivers/tty/serial/serial_port.c
@@ -23,8 +23,8 @@
 /* Only considers pending TX for now. Caller must take care of locking */
 static int __serial_port_busy(struct uart_port *port)
 {
-	return !uart_tx_stopped(port) &&
-		!kfifo_is_empty(&port->state->port.xmit_fifo);
+	return port->x_char || (!uart_tx_stopped(port) &&
+		!kfifo_is_empty(&port->state->port.xmit_fifo));
 }
 
 static int serial_port_runtime_resume(struct device *dev)

-- 
2.56.0

[PATCH 03/24] serial: 8250: don't wait for the em485 timers locked

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

serial8250_em485_destroy() is called with the port lock held and
interrupts off, from ->rs485_config() and from
serial8250_unregister_port().  It calls hrtimer_cancel(), which waits
for a handler which is running, and both handlers start by taking the
port lock:

  CPU0                                  CPU1
  ioctl(TIOCSRS485), rs485 off
  uart_set_rs485_config()
    uart_port_lock_irqsave()            serial8250_em485_handle_stop_tx()
    serial8250_em485_config()             uart_port_lock_irqsave()
      serial8250_em485_destroy()            spins
        hrtimer_cancel()
          spins

All it takes is to turn RS485 off right after the end of a transmission,
when the timer for delay_rts_after_send expires.

The handlers do nothing unless em485->active_timer points to their
timer, so it is enough to clear that pointer.  What cannot be done with
the lock held is to free the structure: hrtimer_try_to_cancel() fails
for a timer whose handler is running, even if it has been started again
in the meantime.  So keep the structure in the port when RS485 is turned
off, use it again when RS485 is turned on, and free it when the port is
unregistered, where the timers can be waited for without the lock.

A port on which RS485 emulation has been used once keeps the structure,
about 150 bytes, until it is unregistered.

Fixes: 6e0a5de2136b ("serial: 8250: Use hrtimers for rs485 delays")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Tate Whiteberg <whiteberg@arista.com>
Cc: Christian Marangi <ansuelsmth@gmail.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: Fushuai Wang <wangfushuai@baidu.com>
Cc: Rafael Gago Castano <rgc@hms.se>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250.h      |  1 +
 drivers/tty/serial/8250/8250_core.c |  5 +---
 drivers/tty/serial/8250/8250_port.c | 60 ++++++++++++++++++++++++++++++-------
 include/linux/serial_8250.h         |  1 +
 4 files changed, 52 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index 21ed9c41b751..82a3b8359643 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -235,6 +235,7 @@ int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
 void serial8250_em485_start_tx(struct uart_8250_port *p, bool toggle_ier);
 void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier);
 void serial8250_em485_destroy(struct uart_8250_port *p);
+void serial8250_em485_free(struct uart_8250_port *p);
 extern struct serial_rs485 serial8250_em485_supported;
 
 /* MCR <-> TIOCM conversion */
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 42319843cb5b..0951d3e42e1f 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -947,10 +947,7 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
 {
 	guard(mutex)(&serial_mutex);
 
-	if (uart->em485) {
-		guard(uart_port_lock_irqsave)(&uart->port);
-		serial8250_em485_destroy(uart);
-	}
+	serial8250_em485_free(uart);
 
 	uart_remove_one_port(&serial8250_reg, &uart->port);
 	if (serial8250_isa_devs) {
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 36b2b0a2c05c..3d47f535d6f1 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -574,15 +574,22 @@ static int serial8250_em485_init(struct uart_8250_port *p)
 	if (p->em485)
 		goto deassert_rts;
 
-	p->em485 = kmalloc_obj(struct uart_8250_em485, GFP_ATOMIC);
-	if (!p->em485)
-		return -ENOMEM;
+	if (p->em485_idle) {
+		p->em485 = p->em485_idle;
+		p->em485_idle = NULL;
+	} else {
+		p->em485 = kmalloc_obj(struct uart_8250_em485, GFP_ATOMIC);
+		if (!p->em485)
+			return -ENOMEM;
 
-	hrtimer_setup(&p->em485->stop_tx_timer, &serial8250_em485_handle_stop_tx, CLOCK_MONOTONIC,
-		      HRTIMER_MODE_REL);
-	hrtimer_setup(&p->em485->start_tx_timer, &serial8250_em485_handle_start_tx, CLOCK_MONOTONIC,
-		      HRTIMER_MODE_REL);
-	p->em485->port = p;
+		hrtimer_setup(&p->em485->stop_tx_timer,
+			      &serial8250_em485_handle_stop_tx, CLOCK_MONOTONIC,
+			      HRTIMER_MODE_REL);
+		hrtimer_setup(&p->em485->start_tx_timer,
+			      &serial8250_em485_handle_start_tx, CLOCK_MONOTONIC,
+			      HRTIMER_MODE_REL);
+		p->em485->port = p;
+	}
 	p->em485->active_timer = NULL;
 	p->em485->tx_stopped = true;
 
@@ -604,6 +611,11 @@ static int serial8250_em485_init(struct uart_8250_port *p)
  *	The function is supposed to be called from .rs485_config callback
  *	or from any other callback protected with p->port.lock spinlock.
  *
+ *	The handlers of the timers take that lock, so they cannot be waited for
+ *	here. One which is running does nothing, as no timer is active any more.
+ *	The memory is kept for the next serial8250_em485_init() and freed by
+ *	serial8250_em485_free().
+ *
  *	See also serial8250_em485_init()
  */
 void serial8250_em485_destroy(struct uart_8250_port *p)
@@ -611,14 +623,40 @@ void serial8250_em485_destroy(struct uart_8250_port *p)
 	if (!p->em485)
 		return;
 
-	hrtimer_cancel(&p->em485->start_tx_timer);
-	hrtimer_cancel(&p->em485->stop_tx_timer);
+	hrtimer_try_to_cancel(&p->em485->start_tx_timer);
+	hrtimer_try_to_cancel(&p->em485->stop_tx_timer);
+	p->em485->active_timer = NULL;
 
-	kfree(p->em485);
+	p->em485_idle = p->em485;
 	p->em485 = NULL;
 }
 EXPORT_SYMBOL_GPL(serial8250_em485_destroy);
 
+/**
+ *	serial8250_em485_free() - release the rs485 emulation of a port
+ *	@p:	uart_8250_port port instance
+ *
+ *	Must be called without p->port.lock, as it waits for the timers.
+ */
+void serial8250_em485_free(struct uart_8250_port *p)
+{
+	struct uart_8250_em485 *em485;
+
+	scoped_guard(uart_port_lock_irqsave, &p->port) {
+		serial8250_em485_destroy(p);
+		em485 = p->em485_idle;
+		p->em485_idle = NULL;
+	}
+
+	if (!em485)
+		return;
+
+	hrtimer_cancel(&em485->start_tx_timer);
+	hrtimer_cancel(&em485->stop_tx_timer);
+	kfree(em485);
+}
+EXPORT_SYMBOL_GPL(serial8250_em485_free);
+
 struct serial_rs485 serial8250_em485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
 		 SER_RS485_TERMINATE_BUS | SER_RS485_RX_DURING_TX,
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index d91c0477be9d..0556bb722db4 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -173,6 +173,7 @@ struct uart_8250_port {
 	void			(*dl_write)(struct uart_8250_port *up, u32 value);
 
 	struct uart_8250_em485 *em485;
+	struct uart_8250_em485 *em485_idle;	/* kept while rs485 is off */
 	void			(*rs485_start_tx)(struct uart_8250_port *up, bool toggle_ier);
 	void			(*rs485_stop_tx)(struct uart_8250_port *up, bool toggle_ier);
 

-- 
2.56.0

Re: [PATCH 03/24] serial: 8250: don't wait for the em485 timers locked

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 7875 bytes --]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Josef Bacik <josef@toxicpanda.com>
> 
> serial8250_em485_destroy() is called with the port lock held and
> interrupts off, from ->rs485_config() and from
> serial8250_unregister_port().  It calls hrtimer_cancel(), which waits
> for a handler which is running, and both handlers start by taking the
> port lock:
> 
>   CPU0                                  CPU1
>   ioctl(TIOCSRS485), rs485 off
>   uart_set_rs485_config()
>     uart_port_lock_irqsave()            serial8250_em485_handle_stop_tx()
>     serial8250_em485_config()             uart_port_lock_irqsave()
>       serial8250_em485_destroy()            spins
>         hrtimer_cancel()
>           spins
> 
> All it takes is to turn RS485 off right after the end of a transmission,
> when the timer for delay_rts_after_send expires.
> 
> The handlers do nothing unless em485->active_timer points to their
> timer, so it is enough to clear that pointer.  What cannot be done with
> the lock held is to free the structure: hrtimer_try_to_cancel() fails
> for a timer whose handler is running, even if it has been started again
> in the meantime.  So keep the structure in the port when RS485 is turned
> off, use it again when RS485 is turned on, and free it when the port is
> unregistered, where the timers can be waited for without the lock.
> 
> A port on which RS485 emulation has been used once keeps the structure,
> about 150 bytes, until it is unregistered.
> 
> Fixes: 6e0a5de2136b ("serial: 8250: Use hrtimers for rs485 delays")
> Cc: stable <stable@kernel.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Andy Shevchenko <andriy.shevchenko@intel.com>
> Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> Cc: Tate Whiteberg <whiteberg@arista.com>
> Cc: Christian Marangi <ansuelsmth@gmail.com>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> Cc: Fushuai Wang <wangfushuai@baidu.com>
> Cc: Rafael Gago Castano <rgc@hms.se>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250.h      |  1 +
>  drivers/tty/serial/8250/8250_core.c |  5 +---
>  drivers/tty/serial/8250/8250_port.c | 60 ++++++++++++++++++++++++++++++-------
>  include/linux/serial_8250.h         |  1 +
>  4 files changed, 52 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index 21ed9c41b751..82a3b8359643 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -235,6 +235,7 @@ int serial8250_em485_config(struct uart_port *port, struct ktermios *termios,
>  void serial8250_em485_start_tx(struct uart_8250_port *p, bool toggle_ier);
>  void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier);
>  void serial8250_em485_destroy(struct uart_8250_port *p);
> +void serial8250_em485_free(struct uart_8250_port *p);
>  extern struct serial_rs485 serial8250_em485_supported;
>  
>  /* MCR <-> TIOCM conversion */
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 42319843cb5b..0951d3e42e1f 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -947,10 +947,7 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
>  {
>  	guard(mutex)(&serial_mutex);
>  
> -	if (uart->em485) {
> -		guard(uart_port_lock_irqsave)(&uart->port);
> -		serial8250_em485_destroy(uart);
> -	}
> +	serial8250_em485_free(uart);
>  
>  	uart_remove_one_port(&serial8250_reg, &uart->port);
>  	if (serial8250_isa_devs) {
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 36b2b0a2c05c..3d47f535d6f1 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -574,15 +574,22 @@ static int serial8250_em485_init(struct uart_8250_port *p)
>  	if (p->em485)
>  		goto deassert_rts;
>  
> -	p->em485 = kmalloc_obj(struct uart_8250_em485, GFP_ATOMIC);
> -	if (!p->em485)
> -		return -ENOMEM;
> +	if (p->em485_idle) {
> +		p->em485 = p->em485_idle;
> +		p->em485_idle = NULL;
> +	} else {
> +		p->em485 = kmalloc_obj(struct uart_8250_em485, GFP_ATOMIC);
> +		if (!p->em485)
> +			return -ENOMEM;
>  
> -	hrtimer_setup(&p->em485->stop_tx_timer, &serial8250_em485_handle_stop_tx, CLOCK_MONOTONIC,
> -		      HRTIMER_MODE_REL);
> -	hrtimer_setup(&p->em485->start_tx_timer, &serial8250_em485_handle_start_tx, CLOCK_MONOTONIC,
> -		      HRTIMER_MODE_REL);
> -	p->em485->port = p;
> +		hrtimer_setup(&p->em485->stop_tx_timer,
> +			      &serial8250_em485_handle_stop_tx, CLOCK_MONOTONIC,
> +			      HRTIMER_MODE_REL);
> +		hrtimer_setup(&p->em485->start_tx_timer,
> +			      &serial8250_em485_handle_start_tx, CLOCK_MONOTONIC,
> +			      HRTIMER_MODE_REL);
> +		p->em485->port = p;
> +	}
>  	p->em485->active_timer = NULL;
>  	p->em485->tx_stopped = true;
>  
> @@ -604,6 +611,11 @@ static int serial8250_em485_init(struct uart_8250_port *p)
>   *	The function is supposed to be called from .rs485_config callback
>   *	or from any other callback protected with p->port.lock spinlock.
>   *
> + *	The handlers of the timers take that lock, so they cannot be waited for
> + *	here. One which is running does nothing, as no timer is active any more.
> + *	The memory is kept for the next serial8250_em485_init() and freed by
> + *	serial8250_em485_free().
> + *
>   *	See also serial8250_em485_init()
>   */
>  void serial8250_em485_destroy(struct uart_8250_port *p)
> @@ -611,14 +623,40 @@ void serial8250_em485_destroy(struct uart_8250_port *p)
>  	if (!p->em485)
>  		return;
>  
> -	hrtimer_cancel(&p->em485->start_tx_timer);
> -	hrtimer_cancel(&p->em485->stop_tx_timer);
> +	hrtimer_try_to_cancel(&p->em485->start_tx_timer);
> +	hrtimer_try_to_cancel(&p->em485->stop_tx_timer);
> +	p->em485->active_timer = NULL;
>  
> -	kfree(p->em485);
> +	p->em485_idle = p->em485;
>  	p->em485 = NULL;
>  }
>  EXPORT_SYMBOL_GPL(serial8250_em485_destroy);

The name is pretty misleading after this change when all it now does is 
stop timers.

--
 i.

>  
> +/**
> + *	serial8250_em485_free() - release the rs485 emulation of a port
> + *	@p:	uart_8250_port port instance
> + *
> + *	Must be called without p->port.lock, as it waits for the timers.
> + */
> +void serial8250_em485_free(struct uart_8250_port *p)
> +{
> +	struct uart_8250_em485 *em485;
> +
> +	scoped_guard(uart_port_lock_irqsave, &p->port) {
> +		serial8250_em485_destroy(p);
> +		em485 = p->em485_idle;
> +		p->em485_idle = NULL;
> +	}
> +
> +	if (!em485)
> +		return;
> +
> +	hrtimer_cancel(&em485->start_tx_timer);
> +	hrtimer_cancel(&em485->stop_tx_timer);
> +	kfree(em485);
> +}
> +EXPORT_SYMBOL_GPL(serial8250_em485_free);
> +
>  struct serial_rs485 serial8250_em485_supported = {
>  	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | SER_RS485_RTS_AFTER_SEND |
>  		 SER_RS485_TERMINATE_BUS | SER_RS485_RX_DURING_TX,
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index d91c0477be9d..0556bb722db4 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -173,6 +173,7 @@ struct uart_8250_port {
>  	void			(*dl_write)(struct uart_8250_port *up, u32 value);
>  
>  	struct uart_8250_em485 *em485;
> +	struct uart_8250_em485 *em485_idle;	/* kept while rs485 is off */
>  	void			(*rs485_start_tx)(struct uart_8250_port *up, bool toggle_ier);
>  	void			(*rs485_stop_tx)(struct uart_8250_port *up, bool toggle_ier);
>  
> 
> 

[PATCH 04/24] serial: 8250: don't report a wakeup to a closed tty

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

If its interrupt is a wakeup source, serial8250_handle_irq_locked()
reports the characters which arrive as a wakeup event of
tport->tty->dev.  That pointer is only set while the port is open.  The
port of a console is not shut down by the last close, so its interrupt
handler goes on running without a tty.  If the system is then woken up
by a character on the console, the handler dereferences a NULL pointer.

Take a reference to the tty, and leave the event out if there is none.

Fixes: 0ba9e3a13c6a ("serial: 8250: Add missing wakeup event reporting")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_port.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 3d47f535d6f1..4afebd96e062 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1900,8 +1900,15 @@ void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
 		struct irq_data *d;
 
 		d = irq_get_irq_data(port->irq);
-		if (d && irqd_is_wakeup_set(d))
-			pm_wakeup_event(tport->tty->dev, 0);
+		if (d && irqd_is_wakeup_set(d)) {
+			struct tty_struct *tty = tty_port_tty_get(tport);
+
+			/* A console goes on receiving after the last close */
+			if (tty) {
+				pm_wakeup_event(tty->dev, 0);
+				tty_kref_put(tty);
+			}
+		}
 		if (!up->dma || handle_rx_dma(up, iir))
 			status = serial8250_rx_chars(up, status);
 	}

-- 
2.56.0

Re: [PATCH 04/24] serial: 8250: don't report a wakeup to a closed tty

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2264 bytes --]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Josef Bacik <josef@toxicpanda.com>
> 
> If its interrupt is a wakeup source, serial8250_handle_irq_locked()
> reports the characters which arrive as a wakeup event of
> tport->tty->dev.  That pointer is only set while the port is open.  The
> port of a console is not shut down by the last close, so its interrupt
> handler goes on running without a tty.  If the system is then woken up
> by a character on the console, the handler dereferences a NULL pointer.
> 
> Take a reference to the tty, and leave the event out if there is none.
> 
> Fixes: 0ba9e3a13c6a ("serial: 8250: Add missing wakeup event reporting")
> Cc: stable <stable@kernel.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> Cc: Petr Mladek <pmladek@suse.com>
> Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_port.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 3d47f535d6f1..4afebd96e062 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1900,8 +1900,15 @@ void serial8250_handle_irq_locked(struct uart_port *port, unsigned int iir)
>  		struct irq_data *d;
>  
>  		d = irq_get_irq_data(port->irq);
> -		if (d && irqd_is_wakeup_set(d))
> -			pm_wakeup_event(tport->tty->dev, 0);
> +		if (d && irqd_is_wakeup_set(d)) {
> +			struct tty_struct *tty = tty_port_tty_get(tport);
> +
> +			/* A console goes on receiving after the last close */
> +			if (tty) {
> +				pm_wakeup_event(tty->dev, 0);
> +				tty_kref_put(tty);
> +			}
> +		}
>  		if (!up->dma || handle_rx_dma(up, iir))
>  			status = serial8250_rx_chars(up, status);
>  	}

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

[PATCH 05/24] serial: 8250: clear subdriver state on port unregister

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

When serial8250_unregister_port() re-adds a port as a fallback ISA port,
it only partially resets the port state via serial8250_init_port(). All
subdriver-owned function pointers and per-driver state copied during
registration (such as startup, shutdown, handle_break, get_mctrl,
set_mctrl, set_termios, private_data, dma, gpios, and rs485 callbacks)
are left intact. After the original driver module is unloaded, these
become dangling pointers that can trigger use-after-free on subsequent
port operations.

Clear all subdriver-copied fields and call serial8250_set_defaults() to
restore default I/O accessors before re-adding the fallback port.

Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: Fushuai Wang <wangfushuai@baidu.com>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_core.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 0951d3e42e1f..673fde55828a 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -956,7 +956,35 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
 		uart->port.dev = &serial8250_isa_devs->dev;
 		uart->port.port_id = uart->port.line;
 		uart->capabilities = 0;
+
+		/*
+		 * Clear subdriver-owned state copied by
+		 * serial8250_register_8250_port() to prevent dangling
+		 * pointers after the original driver module is unloaded.
+		 */
+		uart->port.private_data = NULL;
+		uart->port.set_termios = NULL;
+		uart->port.set_ldisc = NULL;
+		uart->port.get_mctrl = NULL;
+		uart->port.set_mctrl = NULL;
+		uart->port.get_divisor = NULL;
+		uart->port.set_divisor = NULL;
+		uart->port.startup = NULL;
+		uart->port.shutdown = NULL;
+		uart->port.throttle = NULL;
+		uart->port.unthrottle = NULL;
+		uart->port.handle_break = NULL;
+		uart->port.rs485_config = NULL;
+		memset(&uart->port.rs485, 0, sizeof(uart->port.rs485));
+		memset(&uart->port.rs485_supported, 0,
+		       sizeof(uart->port.rs485_supported));
+		uart->gpios = NULL;
+		uart->dma = NULL;
+		uart->rs485_start_tx = NULL;
+		uart->rs485_stop_tx = NULL;
+
 		serial8250_init_port(uart);
+		serial8250_set_defaults(uart);
 		serial8250_apply_quirks(uart);
 		uart_add_one_port(&serial8250_reg, &uart->port);
 	} else {

-- 
2.56.0

Re: [PATCH 05/24] serial: 8250: clear subdriver state on port unregister

[Jiri Slaby]

On 27. 10. 26, 12:06, Greg Kroah-Hartman wrote:
> From: Joshua Rogers <linux@joshua.hu>
> 
> When serial8250_unregister_port() re-adds a port as a fallback ISA port,
> it only partially resets the port state via serial8250_init_port(). All
> subdriver-owned function pointers and per-driver state copied during
> registration (such as startup, shutdown, handle_break, get_mctrl,
> set_mctrl, set_termios, private_data, dma, gpios, and rs485 callbacks)
> are left intact. After the original driver module is unloaded, these
> become dangling pointers that can trigger use-after-free on subsequent
> port operations.
> 
> Clear all subdriver-copied fields and call serial8250_set_defaults() to
> restore default I/O accessors before re-adding the fallback port.
> 
> Cc: stable <stable@kernel.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> Cc: Fushuai Wang <wangfushuai@baidu.com>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   drivers/tty/serial/8250/8250_core.c | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 0951d3e42e1f..673fde55828a 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -956,7 +956,35 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
>   		uart->port.dev = &serial8250_isa_devs->dev;
>   		uart->port.port_id = uart->port.line;
>   		uart->capabilities = 0;
> +
> +		/*
> +		 * Clear subdriver-owned state copied by
> +		 * serial8250_register_8250_port() to prevent dangling
> +		 * pointers after the original driver module is unloaded.
> +		 */
> +		uart->port.private_data = NULL;
> +		uart->port.set_termios = NULL;
> +		uart->port.set_ldisc = NULL;
> +		uart->port.get_mctrl = NULL;
> +		uart->port.set_mctrl = NULL;
> +		uart->port.get_divisor = NULL;
> +		uart->port.set_divisor = NULL;
> +		uart->port.startup = NULL;
> +		uart->port.shutdown = NULL;
> +		uart->port.throttle = NULL;
> +		uart->port.unthrottle = NULL;
> +		uart->port.handle_break = NULL;
> +		uart->port.rs485_config = NULL;
> +		memset(&uart->port.rs485, 0, sizeof(uart->port.rs485));
> +		memset(&uart->port.rs485_supported, 0,
> +		       sizeof(uart->port.rs485_supported));
> +		uart->gpios = NULL;
> +		uart->dma = NULL;
> +		uart->rs485_start_tx = NULL;
> +		uart->rs485_stop_tx = NULL;
> +

Ha! The above gives an answer to my today's:
https://lore.kernel.org/all/53624662-e94e-4fcb-9902-7fa4143c7689@kernel.org/

(And obviously this conflicts with that patch.)


And with the current state of LLMs, I should perhaps revive my "move ops 
to a separate struct" as the above is a PITA. I have been unable to 
finish the series on my own to date.

>   		serial8250_init_port(uart);
> +		serial8250_set_defaults(uart);
>   		serial8250_apply_quirks(uart);
>   		uart_add_one_port(&serial8250_reg, &uart->port);
>   	} else {
> 

thanks,
-- 
js
suse labs

Re: [PATCH 05/24] serial: 8250: clear subdriver state on port unregister

[James Hilliard]

On Tue, Oct 27, 2026 at 5:16 AM Jiri Slaby <jirislaby@kernel.org> wrote:
>
> On 27. 10. 26, 12:06, Greg Kroah-Hartman wrote:
> > From: Joshua Rogers <linux@joshua.hu>
> >
> > When serial8250_unregister_port() re-adds a port as a fallback ISA port,
> > it only partially resets the port state via serial8250_init_port(). All
> > subdriver-owned function pointers and per-driver state copied during
> > registration (such as startup, shutdown, handle_break, get_mctrl,
> > set_mctrl, set_termios, private_data, dma, gpios, and rs485 callbacks)
> > are left intact. After the original driver module is unloaded, these
> > become dangling pointers that can trigger use-after-free on subsequent
> > port operations.
> >
> > Clear all subdriver-copied fields and call serial8250_set_defaults() to
> > restore default I/O accessors before re-adding the fallback port.
> >
> > Cc: stable <stable@kernel.org>
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: John Ogness <john.ogness@linutronix.de>
> > Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> > Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> > Cc: Fushuai Wang <wangfushuai@baidu.com>
> > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >   drivers/tty/serial/8250/8250_core.c | 28 ++++++++++++++++++++++++++++
> >   1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> > index 0951d3e42e1f..673fde55828a 100644
> > --- a/drivers/tty/serial/8250/8250_core.c
> > +++ b/drivers/tty/serial/8250/8250_core.c
> > @@ -956,7 +956,35 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
> >               uart->port.dev = &serial8250_isa_devs->dev;
> >               uart->port.port_id = uart->port.line;
> >               uart->capabilities = 0;
> > +
> > +             /*
> > +              * Clear subdriver-owned state copied by
> > +              * serial8250_register_8250_port() to prevent dangling
> > +              * pointers after the original driver module is unloaded.
> > +              */
> > +             uart->port.private_data = NULL;
> > +             uart->port.set_termios = NULL;
> > +             uart->port.set_ldisc = NULL;
> > +             uart->port.get_mctrl = NULL;
> > +             uart->port.set_mctrl = NULL;
> > +             uart->port.get_divisor = NULL;
> > +             uart->port.set_divisor = NULL;
> > +             uart->port.startup = NULL;
> > +             uart->port.shutdown = NULL;
> > +             uart->port.throttle = NULL;
> > +             uart->port.unthrottle = NULL;
> > +             uart->port.handle_break = NULL;
> > +             uart->port.rs485_config = NULL;
> > +             memset(&uart->port.rs485, 0, sizeof(uart->port.rs485));
> > +             memset(&uart->port.rs485_supported, 0,
> > +                    sizeof(uart->port.rs485_supported));
> > +             uart->gpios = NULL;
> > +             uart->dma = NULL;
> > +             uart->rs485_start_tx = NULL;
> > +             uart->rs485_stop_tx = NULL;
> > +
>
> Ha! The above gives an answer to my today's:
> https://lore.kernel.org/all/53624662-e94e-4fcb-9902-7fa4143c7689@kernel.org/

Yes, this addresses the broader cleanup issue. My RX-trigger fix [0]
complements it by clearing get_rxtrig and set_rxtrig after
uart_remove_one_port(), before registering the fallback port.

It also copies those callbacks unconditionally during registration,
including NULL, so a replacement without custom handlers cannot inherit
the previous owner's callbacks.

[0] https://lore.kernel.org/r/20261024-submit-serial-8250-rxtrig-lifetime-v1-v1-1-731b78d5003d@gmail.com/

>
> (And obviously this conflicts with that patch.)
>
>
> And with the current state of LLMs, I should perhaps revive my "move ops
> to a separate struct" as the above is a PITA. I have been unable to
> finish the series on my own to date.
>
> >               serial8250_init_port(uart);
> > +             serial8250_set_defaults(uart);
> >               serial8250_apply_quirks(uart);
> >               uart_add_one_port(&serial8250_reg, &uart->port);
> >       } else {
> >
>
> thanks,
> --
> js
> suse labs
>

[PATCH 06/24] serial: 8250: add bounds check for line array in setup_serial()

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

setup_serial() writes to info->line[info->ndev] without verifying that
ndev is within the array bounds. A malformed PCMCIA card with a large
I/O window can cause info->multi to exceed 4, leading to out-of-bounds
writes past the line[4] array and corruption of adjacent struct members.

Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/serial_cs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/tty/serial/8250/serial_cs.c b/drivers/tty/serial/8250/serial_cs.c
index 5dcd1544e478..8bd88f3d6d86 100644
--- a/drivers/tty/serial/8250/serial_cs.c
+++ b/drivers/tty/serial/8250/serial_cs.c
@@ -349,6 +349,11 @@ static int setup_serial(struct pcmcia_device *handle, struct serial_info *info,
 {
 	struct uart_8250_port uart;
 
+	if (info->ndev >= ARRAY_SIZE(info->line)) {
+		dev_err(&handle->dev, "no room for more serial ports\n");
+		return -EINVAL;
+	}
+
 	memset(&uart, 0, sizeof(uart));
 	uart.port.iobase = iobase;
 	uart.port.irq = irq;

-- 
2.56.0

Re: [PATCH 06/24] serial: 8250: add bounds check for line array in setup_serial()

[Ilpo Järvinen]

The prefix could be less generic.

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> setup_serial() writes to info->line[info->ndev] without verifying that
> ndev is within the array bounds. A malformed PCMCIA card with a large

Yeah right... Are the "malformed" ones only ones remaining... :-)

> I/O window can cause info->multi to exceed 4, leading to out-of-bounds
> writes past the line[4] array and corruption of adjacent struct members.
> 
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/serial_cs.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/serial_cs.c b/drivers/tty/serial/8250/serial_cs.c
> index 5dcd1544e478..8bd88f3d6d86 100644
> --- a/drivers/tty/serial/8250/serial_cs.c
> +++ b/drivers/tty/serial/8250/serial_cs.c
> @@ -349,6 +349,11 @@ static int setup_serial(struct pcmcia_device *handle, struct serial_info *info,
>  {
>  	struct uart_8250_port uart;
>  
> +	if (info->ndev >= ARRAY_SIZE(info->line)) {
> +		dev_err(&handle->dev, "no room for more serial ports\n");
> +		return -EINVAL;
> +	}
> +
>  	memset(&uart, 0, sizeof(uart));
>  	uart.port.iobase = iobase;
>  	uart.port.irq = irq;
> 
> 

-- 
 i.

[PATCH 07/24] serial: 8250: hold hash_mutex across irq_info lookup and linking

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

serial_get_or_create_irq_info() released hash_mutex before returning the
irq_info pointer, allowing serial_unlink_irq_chain() on another CPU to
free the irq_info before serial_link_irq_chain() could link its port,
causing a use-after-free. Hold hash_mutex in serial_link_irq_chain() for
its entire scope so the irq_info lifetime is protected across both
lookup and linking.

Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Fushuai Wang <wangfushuai@baidu.com>
Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 673fde55828a..5730ef97f836 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -131,12 +131,11 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
  * - find the corresponding info in the hashtable and return it, or
  * - allocate a new one, add it to the hashtable and return it.
  */
+/* Caller must hold hash_mutex */
 static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
 {
 	struct irq_info *i;
 
-	guard(mutex)(&hash_mutex);
-
 	hash_for_each_possible(irq_lists, i, node, up->port.irq)
 		if (i->irq == up->port.irq)
 			return i;
@@ -157,6 +156,8 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
 	struct irq_info *i;
 	int ret;
 
+	guard(mutex)(&hash_mutex);
+
 	i = serial_get_or_create_irq_info(up);
 	if (IS_ERR(i))
 		return PTR_ERR(i);

-- 
2.56.0

Re: [PATCH 07/24] serial: 8250: hold hash_mutex across irq_info lookup and linking

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2272 bytes --]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> serial_get_or_create_irq_info() released hash_mutex before returning the
> irq_info pointer, allowing serial_unlink_irq_chain() on another CPU to
> free the irq_info before serial_link_irq_chain() could link its port,
> causing a use-after-free. Hold hash_mutex in serial_link_irq_chain() for
> its entire scope so the irq_info lifetime is protected across both
> lookup and linking.
> 
> Cc: stable <stable@kernel.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Fushuai Wang <wangfushuai@baidu.com>
> Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 673fde55828a..5730ef97f836 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -131,12 +131,11 @@ static void serial_do_unlink(struct irq_info *i, struct uart_8250_port *up)
>   * - find the corresponding info in the hashtable and return it, or
>   * - allocate a new one, add it to the hashtable and return it.
>   */
> +/* Caller must hold hash_mutex */

We do have lockdep asserts to actually check these.

I also don't understand why own comment is opened for this. And it would 
be better to follow kerneldoc's syntax by starting with Context:

>  static struct irq_info *serial_get_or_create_irq_info(const struct uart_8250_port *up)
>  {
>  	struct irq_info *i;
>  
> -	guard(mutex)(&hash_mutex);
> -
>  	hash_for_each_possible(irq_lists, i, node, up->port.irq)
>  		if (i->irq == up->port.irq)
>  			return i;
> @@ -157,6 +156,8 @@ static int serial_link_irq_chain(struct uart_8250_port *up)
>  	struct irq_info *i;
>  	int ret;
>  
> +	guard(mutex)(&hash_mutex);
> +
>  	i = serial_get_or_create_irq_info(up);
>  	if (IS_ERR(i))
>  		return PTR_ERR(i);
> 
> 

-- 
 i.

[PATCH 08/24] serial: 8250: cancel overrun backoff work on port shutdown and unregister

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

The overrun_backoff delayed work can be scheduled from IRQ handlers in
8250_fsl.c and 8250_omap.c via schedule_delayed_work(). However, this
work is never canceled in the shutdown or unregister paths, so it can
fire after the port has been torn down, causing stale hardware access or
use-after-free.

The existing synchronize_irq() calls in the shutdown path only
synchronize with the IRQ handler itself, not with workqueue callbacks
that the handler has already enqueued. Add cancel_delayed_work_sync()
after the IRQ is quiesced in serial8250_do_shutdown() (the generic
shutdown path used by FSL and others), omap_8250_shutdown() (the
OMAP-specific shutdown path), and serial8250_unregister_port() (the
module unload / hot-unplug path).

Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Joshua Rogers <linux@joshua.hu>
Cc: Fushuai Wang <wangfushuai@baidu.com>
Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: Moteen Shah <m-shah@ti.com>
Cc: Kendall Willis <k-willis@ti.com>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_core.c | 3 +++
 drivers/tty/serial/8250/8250_omap.c | 3 +++
 drivers/tty/serial/8250/8250_port.c | 3 +++
 3 files changed, 9 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 5730ef97f836..af94ed4e633e 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -950,6 +950,9 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
 
 	serial8250_em485_free(uart);
 
+	if (uart->overrun_backoff_time_ms)
+		cancel_delayed_work_sync(&uart->overrun_backoff);
+
 	uart_remove_one_port(&serial8250_reg, &uart->port);
 	if (serial8250_isa_devs) {
 		uart->port.flags &= ~UPF_BOOT_AUTOCONF;
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 29a203ed93fd..07ebed6ac6c9 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -786,6 +786,9 @@ static void omap_8250_shutdown(struct uart_port *port)
 
 	disable_irq_nosync(port->irq);
 
+	if (up->overrun_backoff_time_ms)
+		cancel_delayed_work_sync(&up->overrun_backoff);
+
 	serial8250_release_dma(up);
 	up->dma = NULL;
 
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 4afebd96e062..710d2a28b918 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2497,6 +2497,9 @@ void serial8250_do_shutdown(struct uart_port *port)
 
 	synchronize_irq(port->irq);
 
+	if (up->overrun_backoff_time_ms)
+		cancel_delayed_work_sync(&up->overrun_backoff);
+
 	if (up->dma)
 		serial8250_release_dma(up);
 

-- 
2.56.0

Re: [PATCH 08/24] serial: 8250: cancel overrun backoff work on port shutdown and unregister

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3701 bytes --]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> The overrun_backoff delayed work can be scheduled from IRQ handlers in
> 8250_fsl.c and 8250_omap.c via schedule_delayed_work(). However, this
> work is never canceled in the shutdown or unregister paths, so it can
> fire after the port has been torn down, causing stale hardware access or
> use-after-free.
> 
> The existing synchronize_irq() calls in the shutdown path only
> synchronize with the IRQ handler itself, not with workqueue callbacks
> that the handler has already enqueued. Add cancel_delayed_work_sync()
> after the IRQ is quiesced in serial8250_do_shutdown() (the generic
> shutdown path used by FSL and others), omap_8250_shutdown() (the
> OMAP-specific shutdown path), and serial8250_unregister_port() (the
> module unload / hot-unplug path).
> 
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: John Ogness <john.ogness@linutronix.de>
> Cc: Joshua Rogers <linux@joshua.hu>
> Cc: Fushuai Wang <wangfushuai@baidu.com>
> Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> Cc: Moteen Shah <m-shah@ti.com>
> Cc: Kendall Willis <k-willis@ti.com>
> Cc: Frank Li <Frank.Li@nxp.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
> Cc: Josef Bacik <josef@toxicpanda.com>
> Cc: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_core.c | 3 +++
>  drivers/tty/serial/8250/8250_omap.c | 3 +++
>  drivers/tty/serial/8250/8250_port.c | 3 +++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index 5730ef97f836..af94ed4e633e 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -950,6 +950,9 @@ void serial8250_unregister_port(struct uart_8250_port *uart)
>  
>  	serial8250_em485_free(uart);
>  
> +	if (uart->overrun_backoff_time_ms)
> +		cancel_delayed_work_sync(&uart->overrun_backoff);
> +
>  	uart_remove_one_port(&serial8250_reg, &uart->port);
>  	if (serial8250_isa_devs) {
>  		uart->port.flags &= ~UPF_BOOT_AUTOCONF;
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 29a203ed93fd..07ebed6ac6c9 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -786,6 +786,9 @@ static void omap_8250_shutdown(struct uart_port *port)
>  
>  	disable_irq_nosync(port->irq);
>  
> +	if (up->overrun_backoff_time_ms)
> +		cancel_delayed_work_sync(&up->overrun_backoff);
> +
>  	serial8250_release_dma(up);
>  	up->dma = NULL;
>  
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 4afebd96e062..710d2a28b918 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -2497,6 +2497,9 @@ void serial8250_do_shutdown(struct uart_port *port)
>  
>  	synchronize_irq(port->irq);
>  
> +	if (up->overrun_backoff_time_ms)
> +		cancel_delayed_work_sync(&up->overrun_backoff);

Wouldn't it be better for fsl to add own shutdown function to do this 
there and not within the generic code?

This entire overrun_backoff machinery seems something that should be part 
of the respective drivers but that's beyond the scope of this fix.

> +
>  	if (up->dma)
>  		serial8250_release_dma(up);
>  
> 
> 

-- 
 i.

[PATCH 09/24] serial: 8250: 8250_bcm7271: fix DMA address truncation to 32 bits

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

The local variable daddr is declared as u32 but assigned from dma_addr_t
fields (priv->rx_addr and priv->tx_addr), which are 64-bit on ARM64.
This silently truncates the upper 32 bits, causing upper_32_bits(daddr)
to always produce zero when programming the UART DMA descriptor high
address registers.  Change the type to dma_addr_t to preserve the full
DMA address.

Cc: Al Cooper <alcooperx@gmail.com>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: stable <stable@kernel.org>
Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_bcm7271.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
index 974d9af146a1..4fdd261e55f5 100644
--- a/drivers/tty/serial/8250/8250_bcm7271.c
+++ b/drivers/tty/serial/8250/8250_bcm7271.c
@@ -315,7 +315,7 @@ static int brcmuart_arbitration(struct brcmuart_priv *priv, bool acquire)
 
 static void brcmuart_init_dma_hardware(struct brcmuart_priv *priv)
 {
-	u32 daddr;
+	dma_addr_t daddr;
 	u32 value;
 	int x;
 

-- 
2.56.0

Re: [PATCH 09/24] serial: 8250: 8250_bcm7271: fix DMA address truncation to 32 bits

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> The local variable daddr is declared as u32 but assigned from dma_addr_t
> fields (priv->rx_addr and priv->tx_addr), which are 64-bit on ARM64.
> This silently truncates the upper 32 bits, causing upper_32_bits(daddr)
> to always produce zero when programming the UART DMA descriptor high
> address registers.  Change the type to dma_addr_t to preserve the full
> DMA address.
> 
> Cc: Al Cooper <alcooperx@gmail.com>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: stable <stable@kernel.org>
> Cc: Broadcom internal kernel review list <bcm-kernel-feedback-list@broadcom.com>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_bcm7271.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_bcm7271.c b/drivers/tty/serial/8250/8250_bcm7271.c
> index 974d9af146a1..4fdd261e55f5 100644
> --- a/drivers/tty/serial/8250/8250_bcm7271.c
> +++ b/drivers/tty/serial/8250/8250_bcm7271.c
> @@ -315,7 +315,7 @@ static int brcmuart_arbitration(struct brcmuart_priv *priv, bool acquire)
>  
>  static void brcmuart_init_dma_hardware(struct brcmuart_priv *priv)
>  {
> -	u32 daddr;
> +	dma_addr_t daddr;
>  	u32 value;
>  	int x;

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH 09/24] serial: 8250: 8250_bcm7271: fix DMA address truncation to 32 bits

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 12:06:35PM +0100, Greg Kroah-Hartman wrote:
> 
> The local variable daddr is declared as u32 but assigned from dma_addr_t
> fields (priv->rx_addr and priv->tx_addr), which are 64-bit on ARM64.
> This silently truncates the upper 32 bits, causing upper_32_bits(daddr)
> to always produce zero when programming the UART DMA descriptor high
> address registers.  Change the type to dma_addr_t to preserve the full
> DMA address.

Since this value goes to HW, it should be u64. Even on 32-bit machines there
are possibilities to cover a bigger address space / DMA space.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 09/24] serial: 8250: 8250_bcm7271: fix DMA address truncation to 32 bits

[Florian Fainelli]

On 10/27/26 08:10, Andy Shevchenko wrote:
> On Tue, Oct 27, 2026 at 12:06:35PM +0100, Greg Kroah-Hartman wrote:
>>
>> The local variable daddr is declared as u32 but assigned from dma_addr_t
>> fields (priv->rx_addr and priv->tx_addr), which are 64-bit on ARM64.
>> This silently truncates the upper 32 bits, causing upper_32_bits(daddr)
>> to always produce zero when programming the UART DMA descriptor high
>> address registers.  Change the type to dma_addr_t to preserve the full
>> DMA address.
> 
> Since this value goes to HW, it should be u64. Even on 32-bit machines there
> are possibilities to cover a bigger address space / DMA space.

That is one way to go about it, but since the size of dma_addr_t follows 
ARCH_DMA_ADDR_T_64BIT, we could also take this patch, and also make the 
upper 32-bits programming conditional upon CONFIG_ARCH_DMA_ADDR_T_64BIT.
-- 
Florian

Re: [PATCH 09/24] serial: 8250: 8250_bcm7271: fix DMA address truncation to 32 bits

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 09:41:17AM -0700, Florian Fainelli wrote:
> On 10/27/26 08:10, Andy Shevchenko wrote:
> > On Tue, Oct 27, 2026 at 12:06:35PM +0100, Greg Kroah-Hartman wrote:
> > > 
> > > The local variable daddr is declared as u32 but assigned from dma_addr_t
> > > fields (priv->rx_addr and priv->tx_addr), which are 64-bit on ARM64.
> > > This silently truncates the upper 32 bits, causing upper_32_bits(daddr)
> > > to always produce zero when programming the UART DMA descriptor high
> > > address registers.  Change the type to dma_addr_t to preserve the full
> > > DMA address.
> > 
> > Since this value goes to HW, it should be u64. Even on 32-bit machines there
> > are possibilities to cover a bigger address space / DMA space.
> 
> That is one way to go about it, but since the size of dma_addr_t follows
> ARCH_DMA_ADDR_T_64BIT, we could also take this patch, and also make the
> upper 32-bits programming conditional upon CONFIG_ARCH_DMA_ADDR_T_64BIT.

You never know this. The use of u64 is simple as that: it goes to HW as 64-bit
value no matter what. Note, in the past we used to have a problematic use of
dma_addr_t exactly due to its dependency on the data bus size when programming
HW, and hence, based on my experience, I hardly recommend not to use dma_addr_t
in this case, use plain fixed u64.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 09/24] serial: 8250: 8250_bcm7271: fix DMA address truncation to 32 bits

[Florian Fainelli]

On 10/27/26 10:14, Andy Shevchenko wrote:
> On Tue, Oct 27, 2026 at 09:41:17AM -0700, Florian Fainelli wrote:
>> On 10/27/26 08:10, Andy Shevchenko wrote:
>>> On Tue, Oct 27, 2026 at 12:06:35PM +0100, Greg Kroah-Hartman wrote:
>>>>
>>>> The local variable daddr is declared as u32 but assigned from dma_addr_t
>>>> fields (priv->rx_addr and priv->tx_addr), which are 64-bit on ARM64.
>>>> This silently truncates the upper 32 bits, causing upper_32_bits(daddr)
>>>> to always produce zero when programming the UART DMA descriptor high
>>>> address registers.  Change the type to dma_addr_t to preserve the full
>>>> DMA address.
>>>
>>> Since this value goes to HW, it should be u64. Even on 32-bit machines there
>>> are possibilities to cover a bigger address space / DMA space.
>>
>> That is one way to go about it, but since the size of dma_addr_t follows
>> ARCH_DMA_ADDR_T_64BIT, we could also take this patch, and also make the
>> upper 32-bits programming conditional upon CONFIG_ARCH_DMA_ADDR_T_64BIT.
> 
> You never know this. The use of u64 is simple as that: it goes to HW as 64-bit
> value no matter what. Note, in the past we used to have a problematic use of
> dma_addr_t exactly due to its dependency on the data bus size when programming
> HW, and hence, based on my experience, I hardly recommend not to use dma_addr_t
> in this case, use plain fixed u64.

It's not a hot path where optimizing for bus transactions would make 
sense, so I guess it's fine to use u64 and have the two halves 
programmed accordingly.
--
Florian

[PATCH 10/24] serial: 8250: pericom: validate port offset against BAR length

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

The fourth port on 4-port Pericom devices uses offset 0x38 instead of
the linear 0x18, but maxnr is computed assuming uniform 8-byte spacing.
A malicious PCI device with an undersized BAR could cause I/O access
outside the assigned resource. Check that offset + 8 fits within the BAR
before registering each port.

Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_pericom.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_pericom.c b/drivers/tty/serial/8250/8250_pericom.c
index bbbf48c6d07d..f7a7a3f737be 100644
--- a/drivers/tty/serial/8250/8250_pericom.c
+++ b/drivers/tty/serial/8250/8250_pericom.c
@@ -123,6 +123,9 @@ static int pericom8250_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	for (i = 0; i < nr && i < maxnr; i++) {
 		unsigned int offset = (i == 3 && nr == 4) ? 0x38 : i * 0x8;
 
+		if (offset + 8 > pci_resource_len(pdev, bar))
+			break;
+
 		uart.port.iobase = pci_resource_start(pdev, bar) + offset;
 
 		dev_dbg(&pdev->dev, "Setup PCI port: port %lx, irq %d, type %d\n",

-- 
2.56.0

Re: [PATCH 10/24] serial: 8250: pericom: validate port offset against BAR length

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 12:06:36PM +0100, Greg Kroah-Hartman wrote:
> 
> The fourth port on 4-port Pericom devices uses offset 0x38 instead of
> the linear 0x18, but maxnr is computed assuming uniform 8-byte spacing.
> A malicious PCI device with an undersized BAR could cause I/O access
> outside the assigned resource. Check that offset + 8 fits within the BAR
> before registering each port.

...

>  		unsigned int offset = (i == 3 && nr == 4) ? 0x38 : i * 0x8;
>  
> +		if (offset + 8 > pci_resource_len(pdev, bar))

For the consistency's sake 0x8?

> +			break;
> +
>  		uart.port.iobase = pci_resource_start(pdev, bar) + offset;

-- 
With Best Regards,
Andy Shevchenko

[PATCH 11/24] serial: 8250: cap OxSemi Tornado UART count against BAR size

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

pci_oxsemi_tornado_init() reads the UART count from device-controlled
BAR0 memory without validation, allowing a malicious device to report up
to 255 ports. Cap the device-reported count against what actually fits
in the BAR using the Tornado register layout.

Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Marnix Rijnart <marnix.rijnart@iwell.eu>
Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_pci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 1dda63be6b89..9919504334bb 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1083,6 +1083,7 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
 	u8 __iomem *p;
 	unsigned long deviceID;
 	unsigned int  number_uarts = 0;
+	resource_size_t bar_len;
 
 	if (!pci_oxsemi_tornado_p(dev))
 		return 0;
@@ -1095,6 +1096,15 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
 	/* Tornado device */
 	if (deviceID == 0x07000200) {
 		number_uarts = ioread8(p + 4);
+
+		/* Cap against BAR0 size (Tornado layout: 0x1000 + n * 0x200) */
+		bar_len = pci_resource_len(dev, 0);
+		if (bar_len >= 0x1000)
+			number_uarts = min_t(unsigned int, number_uarts,
+					     (bar_len - 0x1000) / 0x200);
+		else
+			number_uarts = 0;
+
 		pci_dbg(dev, "%d ports detected on %s PCI Express device\n",
 			number_uarts,
 			dev->vendor == PCI_VENDOR_ID_ENDRUN ?

-- 
2.56.0

Re: [PATCH 11/24] serial: 8250: cap OxSemi Tornado UART count against BAR size

[Jiri Slaby]

On 27. 10. 26, 12:06, Greg Kroah-Hartman wrote:
> From: Joshua Rogers <linux@joshua.hu>
> 
> pci_oxsemi_tornado_init() reads the UART count from device-controlled
> BAR0 memory without validation, allowing a malicious device to report up
> to 255 ports. Cap the device-reported count against what actually fits
> in the BAR using the Tornado register layout.
> 
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Marnix Rijnart <marnix.rijnart@iwell.eu>
> Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   drivers/tty/serial/8250/8250_pci.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 1dda63be6b89..9919504334bb 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1083,6 +1083,7 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
>   	u8 __iomem *p;
>   	unsigned long deviceID;
>   	unsigned int  number_uarts = 0;
> +	resource_size_t bar_len;
>   
>   	if (!pci_oxsemi_tornado_p(dev))
>   		return 0;
> @@ -1095,6 +1096,15 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
>   	/* Tornado device */
>   	if (deviceID == 0x07000200) {
>   		number_uarts = ioread8(p + 4);
> +
> +		/* Cap against BAR0 size (Tornado layout: 0x1000 + n * 0x200) */
> +		bar_len = pci_resource_len(dev, 0);
> +		if (bar_len >= 0x1000)
> +			number_uarts = min_t(unsigned int, number_uarts,
> +					     (bar_len - 0x1000) / 0x200);

Is the _t variant necessary? Both are unsigned (int and u64).

> +		else
> +			number_uarts = 0;
> +
>   		pci_dbg(dev, "%d ports detected on %s PCI Express device\n",
>   			number_uarts,
>   			dev->vendor == PCI_VENDOR_ID_ENDRUN ?
> 


-- 
js
suse labs

Re: [PATCH 11/24] serial: 8250: cap OxSemi Tornado UART count against BAR size

[Greg Kroah-Hartman]

On Tue, Oct 27, 2026 at 12:23:14PM +0100, Jiri Slaby wrote:
> On 27. 10. 26, 12:06, Greg Kroah-Hartman wrote:
> > From: Joshua Rogers <linux@joshua.hu>
> > 
> > pci_oxsemi_tornado_init() reads the UART count from device-controlled
> > BAR0 memory without validation, allowing a malicious device to report up
> > to 255 ports. Cap the device-reported count against what actually fits
> > in the BAR using the Tornado register layout.
> > 
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Lukas Wunner <lukas@wunner.de>
> > Cc: Marnix Rijnart <marnix.rijnart@iwell.eu>
> > Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> > Cc: stable <stable@kernel.org>
> > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >   drivers/tty/serial/8250/8250_pci.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> > index 1dda63be6b89..9919504334bb 100644
> > --- a/drivers/tty/serial/8250/8250_pci.c
> > +++ b/drivers/tty/serial/8250/8250_pci.c
> > @@ -1083,6 +1083,7 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
> >   	u8 __iomem *p;
> >   	unsigned long deviceID;
> >   	unsigned int  number_uarts = 0;
> > +	resource_size_t bar_len;
> >   	if (!pci_oxsemi_tornado_p(dev))
> >   		return 0;
> > @@ -1095,6 +1096,15 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
> >   	/* Tornado device */
> >   	if (deviceID == 0x07000200) {
> >   		number_uarts = ioread8(p + 4);
> > +
> > +		/* Cap against BAR0 size (Tornado layout: 0x1000 + n * 0x200) */
> > +		bar_len = pci_resource_len(dev, 0);
> > +		if (bar_len >= 0x1000)
> > +			number_uarts = min_t(unsigned int, number_uarts,
> > +					     (bar_len - 0x1000) / 0x200);
> 
> Is the _t variant necessary? Both are unsigned (int and u64).

Ah, yes, the true tell of a LLM that was based on an "old" kernel
history.  I'll go fix this up for the next round of these patches, for
those that still need work, thanks!

greg k-h

Re: [PATCH 11/24] serial: 8250: cap OxSemi Tornado UART count against BAR size

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> pci_oxsemi_tornado_init() reads the UART count from device-controlled
> BAR0 memory without validation, allowing a malicious device to report up
> to 255 ports. Cap the device-reported count against what actually fits
> in the BAR using the Tornado register layout.
> 
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Lukas Wunner <lukas@wunner.de>
> Cc: Marnix Rijnart <marnix.rijnart@iwell.eu>
> Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_pci.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 1dda63be6b89..9919504334bb 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1083,6 +1083,7 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
>  	u8 __iomem *p;
>  	unsigned long deviceID;
>  	unsigned int  number_uarts = 0;
> +	resource_size_t bar_len;
>  
>  	if (!pci_oxsemi_tornado_p(dev))
>  		return 0;
> @@ -1095,6 +1096,15 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
>  	/* Tornado device */
>  	if (deviceID == 0x07000200) {
>  		number_uarts = ioread8(p + 4);
> +
> +		/* Cap against BAR0 size (Tornado layout: 0x1000 + n * 0x200) */
> +		bar_len = pci_resource_len(dev, 0);
> +		if (bar_len >= 0x1000)
> +			number_uarts = min_t(unsigned int, number_uarts,
> +					     (bar_len - 0x1000) / 0x200);

IMO, these literals would be better written as SZ_xx + make sure you've 
the include for it if changing to that.

> +		else
> +			number_uarts = 0;
> +
>  		pci_dbg(dev, "%d ports detected on %s PCI Express device\n",
>  			number_uarts,
>  			dev->vendor == PCI_VENDOR_ID_ENDRUN ?
> 
> 

-- 
 i.

Re: [PATCH 11/24] serial: 8250: cap OxSemi Tornado UART count against BAR size

[Maciej W. Rozycki]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> pci_oxsemi_tornado_init() reads the UART count from device-controlled
> BAR0 memory without validation, allowing a malicious device to report up
> to 255 ports. Cap the device-reported count against what actually fits
> in the BAR using the Tornado register layout.

 Hmm, what is "a malicious device" supposed to be, a broken chip?

> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 1dda63be6b89..9919504334bb 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -1095,6 +1096,15 @@ static int pci_oxsemi_tornado_init(struct pci_dev *dev)
>  	/* Tornado device */
>  	if (deviceID == 0x07000200) {
>  		number_uarts = ioread8(p + 4);
> +
> +		/* Cap against BAR0 size (Tornado layout: 0x1000 + n * 0x200) */
> +		bar_len = pci_resource_len(dev, 0);
> +		if (bar_len >= 0x1000)
> +			number_uarts = min_t(unsigned int, number_uarts,
> +					     (bar_len - 0x1000) / 0x200);
> +		else
> +			number_uarts = 0;
> +

 Technically an overkill and duplicating the FL_REGION_SZ_CAP feature 
rather than making use of it, and then falsifying the actual number of 
ports report in the kernel log (which I think ought to be accurate even if 
do clamp the number).

 Sashiko is however right that this function ought to return an error 
rather than 0, and also should BAR0 be smaller than 8, we already violate 
access with the ioread8() call quoted (and possibly ioread32() earlier 
on).

 So I think overall this ought to look like (unchecked):

static int pci_oxsemi_tornado_init(struct pci_dev *dev)
{
	u8 __iomem *p;
	unsigned long deviceID;
	int number_uarts = -ENXIO;

	if (!pci_oxsemi_tornado_p(dev) || pci_resource_len(dev, 0) < 8)
		return -ENXIO;

	p = pci_iomap(dev, 0, 5);
	if (p == NULL)
		return -ENOMEM;

	deviceID = ioread32(p);
	/* Tornado device */
	if (deviceID == 0x07000200) {
		number_uarts = ioread8(p + 4);
		pci_dbg(dev, "%d ports detected on %s PCI Express device\n",
			number_uarts,
			dev->vendor == PCI_VENDOR_ID_ENDRUN ?
			"EndRun" : "Oxford");
		if (number_uarts < 1)
			number_uarts = -ENXIO;
	}
	pci_iounmap(dev, p);
	return number_uarts;
}

with the FL_REGION_SZ_CAP flag set for the relevant entries, and possibly 
setting ->num_ports to 0 too since we need to determine that at run time.

 Not sure how this interferes with Brainboxes devices, support for which 
seems not to have been fully merged given that pci_oxsemi_tornado_p() does 
not let PCI_VENDOR_ID_INTASHIELD devices through.  Cameron?

 NB BAR0 size is 16k by the spec and the maximum # of UARTs is 16, for a 
dual-OXPCIe958 master/slave setup, still leaving 4k of the available space 
unoccupied.

  Maciej

Re: [PATCH 11/24] serial: 8250: cap OxSemi Tornado UART count against BAR size

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 02:29:21PM +0000, Maciej W. Rozycki wrote:
> On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:
> 
> > pci_oxsemi_tornado_init() reads the UART count from device-controlled
> > BAR0 memory without validation, allowing a malicious device to report up
> > to 255 ports. Cap the device-reported count against what actually fits
> > in the BAR using the Tornado register layout.
> 
>  Hmm, what is "a malicious device" supposed to be, a broken chip?

Carefully reconstructed malicious device in FPGA that gets connected via
Thunderbolt. This is real vector of attacks (assuming the Thunderbolt side is
misconfigured to automatically authorise anything connected to it).

IRL I don't believe it would ever happen as the result of this attack most
likely would be minor if any.

...

> static int pci_oxsemi_tornado_init(struct pci_dev *dev)
> {
> 	u8 __iomem *p;
> 	unsigned long deviceID;
> 	int number_uarts = -ENXIO;

> 	if (!pci_oxsemi_tornado_p(dev) || pci_resource_len(dev, 0) < 8)
> 		return -ENXIO;
> 
> 	p = pci_iomap(dev, 0, 5);

Why not 8 instead of 5?

> 	if (p == NULL)
> 		return -ENOMEM;

This is so interesting... For MMIO it gives a page, I don't see how it can go
outside the boundaries. For port based IO I think it can't be less than 4 for
sure (due to PCI bits to interpret) and most likely can't be less than 8. But
I have a weak memory of PCI specifications.

> 	deviceID = ioread32(p);
> 	/* Tornado device */
> 	if (deviceID == 0x07000200) {
> 		number_uarts = ioread8(p + 4);
> 		pci_dbg(dev, "%d ports detected on %s PCI Express device\n",
> 			number_uarts,
> 			dev->vendor == PCI_VENDOR_ID_ENDRUN ?
> 			"EndRun" : "Oxford");
> 		if (number_uarts < 1)
> 			number_uarts = -ENXIO;
> 	}
> 	pci_iounmap(dev, p);
> 	return number_uarts;
> }
> 
> with the FL_REGION_SZ_CAP flag set for the relevant entries, and possibly 
> setting ->num_ports to 0 too since we need to determine that at run time.
> 
>  Not sure how this interferes with Brainboxes devices, support for which 
> seems not to have been fully merged given that pci_oxsemi_tornado_p() does 
> not let PCI_VENDOR_ID_INTASHIELD devices through.  Cameron?

-- 
With Best Regards,
Andy Shevchenko

[PATCH 12/24] serial: 8250: dfl: add bounds check for device-provided regshift

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

The DFL UART driver reads regshift from the FPGA feature parameters
without validating it, which can cause undefined behavior or
out-of-bounds MMIO access in the 8250 core's offset << regshift
calculations. Cap regshift at 4 to reject unreasonable values.

Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_dfl.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dfl.c b/drivers/tty/serial/8250/8250_dfl.c
index 1343375897a8..72938990ea81 100644
--- a/drivers/tty/serial/8250/8250_dfl.c
+++ b/drivers/tty/serial/8250/8250_dfl.c
@@ -84,6 +84,10 @@ static int dfl_uart_get_params(struct dfl_device *dfl_dev, struct uart_8250_port
 		return dev_err_probe(dev, ret, "missing REG_LAYOUT param\n");
 
 	uart->port.regshift = FIELD_GET(DFHv1_PARAM_REG_LAYOUT_SHIFT, reg_layout);
+	if (uart->port.regshift > 4)
+		return dev_err_probe(dev, -EINVAL, "unsupported reg-shift %u\n",
+				     uart->port.regshift);
+
 	reg_width = FIELD_GET(DFHv1_PARAM_REG_LAYOUT_WIDTH, reg_layout);
 	switch (reg_width) {
 	case 4:

-- 
2.56.0

Re: [PATCH 12/24] serial: 8250: dfl: add bounds check for device-provided regshift

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 12:06:38PM +0100, Greg Kroah-Hartman wrote:
> 
> The DFL UART driver reads regshift from the FPGA feature parameters
> without validating it, which can cause undefined behavior or
> out-of-bounds MMIO access in the 8250 core's offset << regshift
> calculations. Cap regshift at 4 to reject unreasonable values.

8 sounds pretty reasonable to me. I think this patch is odd as it's up to FPGA
what to set there. You need to compare a maximum accessed offset (taking into
account given reg-shift) to the BAR size.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 13/24] serial: 8250: dfl: validate MMIO resource size and regshift

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

The driver passes the DFL feature's raw MMIO resource to the 8250 core
without verifying that the resource covers the full UART register
window.  Add a regshift sanity check and validate that the MMIO resource
is large enough for 8250 register accesses up to register 7 at the
configured register spacing and access width.

Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Joshua Rogers <linux@joshua.hu>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
Cc: Danilo Krummrich <dakr@kernel.org>
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_dfl.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dfl.c b/drivers/tty/serial/8250/8250_dfl.c
index 72938990ea81..e9329935bc47 100644
--- a/drivers/tty/serial/8250/8250_dfl.c
+++ b/drivers/tty/serial/8250/8250_dfl.c
@@ -110,6 +110,7 @@ static int dfl_uart_probe(struct dfl_device *dfl_dev)
 {
 	struct device *dev = &dfl_dev->dev;
 	struct uart_8250_port uart = { };
+	resource_size_t min_size;
 	int ret;
 
 	uart.port.flags = UPF_IOREMAP;
@@ -120,6 +121,17 @@ static int dfl_uart_probe(struct dfl_device *dfl_dev)
 	if (ret < 0)
 		return dev_err_probe(dev, ret, "failed uart feature walk\n");
 
+	if (uart.port.regshift > 3)
+		return dev_err_probe(dev, -EINVAL,
+				     "unsupported regshift %u\n",
+				     uart.port.regshift);
+
+	min_size = (7 << uart.port.regshift) +
+		   ((uart.port.iotype == UPIO_MEM32) ? 4 : 2);
+	if (uart.port.mapsize < min_size)
+		return dev_err_probe(dev, -EINVAL,
+				     "MMIO resource too small for register layout\n");
+
 	if (dfl_dev->num_irqs == 1)
 		uart.port.irq = dfl_dev->irqs[0];
 

-- 
2.56.0

Re: [PATCH 13/24] serial: 8250: dfl: validate MMIO resource size and regshift

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> The driver passes the DFL feature's raw MMIO resource to the 8250 core
> without verifying that the resource covers the full UART register
> window.  Add a regshift sanity check and validate that the MMIO resource
> is large enough for 8250 register accesses up to register 7 at the
> configured register spacing and access width.
> 
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Joshua Rogers <linux@joshua.hu>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: "Uwe Kleine-König (The Capable Hub)" <u.kleine-koenig@baylibre.com>
> Cc: Danilo Krummrich <dakr@kernel.org>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_dfl.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_dfl.c b/drivers/tty/serial/8250/8250_dfl.c
> index 72938990ea81..e9329935bc47 100644
> --- a/drivers/tty/serial/8250/8250_dfl.c
> +++ b/drivers/tty/serial/8250/8250_dfl.c
> @@ -110,6 +110,7 @@ static int dfl_uart_probe(struct dfl_device *dfl_dev)
>  {
>  	struct device *dev = &dfl_dev->dev;
>  	struct uart_8250_port uart = { };
> +	resource_size_t min_size;
>  	int ret;
>  
>  	uart.port.flags = UPF_IOREMAP;
> @@ -120,6 +121,17 @@ static int dfl_uart_probe(struct dfl_device *dfl_dev)
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "failed uart feature walk\n");
>  
> +	if (uart.port.regshift > 3)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "unsupported regshift %u\n",
> +				     uart.port.regshift);
> +
> +	min_size = (7 << uart.port.regshift) +

Shouldn't this be based on a named register UART_xx?

> +		   ((uart.port.iotype == UPIO_MEM32) ? 4 : 2);
> +	if (uart.port.mapsize < min_size)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "MMIO resource too small for register layout\n");
> +
>  	if (dfl_dev->num_irqs == 1)
>  		uart.port.irq = dfl_dev->irqs[0];
>  
> 
> 

-- 
 i.

Re: [PATCH 13/24] serial: 8250: dfl: validate MMIO resource size and regshift

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 12:06:39PM +0100, Greg Kroah-Hartman wrote:

> The driver passes the DFL feature's raw MMIO resource to the 8250 core
> without verifying that the resource covers the full UART register
> window.  Add a regshift sanity check and validate that the MMIO resource
> is large enough for 8250 register accesses up to register 7 at the
> configured register spacing and access width.

Same comment as per previous patch. Take the bar size, take all data from FPGA,
validate against given limits.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 14/24] serial: 8250: ioc3: store registered line number in driver data

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

The registered line number returned by serial8250_register_8250_port()
was stored only in the local variable 'line' and never saved into
data->line, causing the remove path to always unregister line 0
instead of the correct port.

Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_ioc3.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/8250/8250_ioc3.c b/drivers/tty/serial/8250/8250_ioc3.c
index 088242de2eff..3ab50e04be45 100644
--- a/drivers/tty/serial/8250/8250_ioc3.c
+++ b/drivers/tty/serial/8250/8250_ioc3.c
@@ -63,6 +63,7 @@ static int serial8250_ioc3_probe(struct platform_device *pdev)
 	if (IS_ERR(uport))
 		return PTR_ERR(uport);
 
+	data->line = line;
 	platform_set_drvdata(pdev, uport);
 	return 0;
 }

-- 
2.56.0

Re: [PATCH 14/24] serial: 8250: ioc3: store registered line number in driver data

[Ilpo Järvinen]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> The registered line number returned by serial8250_register_8250_port()
> was stored only in the local variable 'line' and never saved into
> data->line, causing the remove path to always unregister line 0
> instead of the correct port.
> 
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_ioc3.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_ioc3.c b/drivers/tty/serial/8250/8250_ioc3.c
> index 088242de2eff..3ab50e04be45 100644
> --- a/drivers/tty/serial/8250/8250_ioc3.c
> +++ b/drivers/tty/serial/8250/8250_ioc3.c
> @@ -63,6 +63,7 @@ static int serial8250_ioc3_probe(struct platform_device *pdev)
>  	if (IS_ERR(uport))
>  		return PTR_ERR(uport);
>  
> +	data->line = line;
>  	platform_set_drvdata(pdev, uport);
>  	return 0;
>  }

Eh? Didn't Jiri just kill the entire line madness? In d6a0791098be 
("serial: 8250: make port operations work on struct uart_8250_port"):

-struct ioc3_8250_data {
-       int line;
-};

There's no "line" in the entire file anymore?!

On which commit you're basing these on? How was this tested??? ;-) ...I 
hope you're not just wasting our time by sending unfiltered LLM output to 
us.


-- 
 i.

Re: [PATCH 14/24] serial: 8250: ioc3: store registered line number in driver data

[Greg Kroah-Hartman]

On Tue, Oct 27, 2026 at 02:38:49PM +0200, Ilpo Järvinen wrote:
> On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:
> 
> > From: Joshua Rogers <linux@joshua.hu>
> > 
> > The registered line number returned by serial8250_register_8250_port()
> > was stored only in the local variable 'line' and never saved into
> > data->line, causing the remove path to always unregister line 0
> > instead of the correct port.
> > 
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: stable <stable@kernel.org>
> > Signed-off-by: Joshua Rogers <linux@joshua.hu>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/tty/serial/8250/8250_ioc3.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/tty/serial/8250/8250_ioc3.c b/drivers/tty/serial/8250/8250_ioc3.c
> > index 088242de2eff..3ab50e04be45 100644
> > --- a/drivers/tty/serial/8250/8250_ioc3.c
> > +++ b/drivers/tty/serial/8250/8250_ioc3.c
> > @@ -63,6 +63,7 @@ static int serial8250_ioc3_probe(struct platform_device *pdev)
> >  	if (IS_ERR(uport))
> >  		return PTR_ERR(uport);
> >  
> > +	data->line = line;
> >  	platform_set_drvdata(pdev, uport);
> >  	return 0;
> >  }
> 
> Eh? Didn't Jiri just kill the entire line madness? In d6a0791098be 
> ("serial: 8250: make port operations work on struct uart_8250_port"):
> 
> -struct ioc3_8250_data {
> -       int line;
> -};
> 
> There's no "line" in the entire file anymore?!
> 
> On which commit you're basing these on? How was this tested??? ;-) ...I 
> hope you're not just wasting our time by sending unfiltered LLM output to 
> us.

I built this on top of Linus's latest tree, odds are I don't have a
.config option set somewhere, sorry abou tthat.

greg k-h

[PATCH 15/24] serial: 8250: omap: acquire rx_dma_lock in DMA completion callback

[Greg Kroah-Hartman]

From: Joshua Rogers <linux@joshua.hu>

The __dma_rx_do_complete() helper requires priv->rx_dma_lock to be held,
but the DMA completion callback __dma_rx_complete() calls it without
that lock, only holding the UART port lock. This allows a race between
the callback and omap_8250_rx_dma_flush() which properly holds
rx_dma_lock, leading to concurrent tty_insert_flip_string() calls that
can corrupt flip-buffer state. Acquire rx_dma_lock around the call,
matching the established lock ordering (port lock then rx_dma_lock).

Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Moteen Shah <m-shah@ti.com>
Cc: Kendall Willis <k-willis@ti.com>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: stable <stable@kernel.org>
Signed-off-by: Joshua Rogers <linux@joshua.hu>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_omap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 07ebed6ac6c9..0afd648733da 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -975,7 +975,9 @@ static void __dma_rx_complete(void *param)
 	if (dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state) != DMA_COMPLETE)
 		return;
 
+	spin_lock(&priv->rx_dma_lock);
 	__dma_rx_do_complete(p);
+	spin_unlock(&priv->rx_dma_lock);
 	if (priv->throttled)
 		return;
 

-- 
2.56.0

Re: [PATCH 15/24] serial: 8250: omap: acquire rx_dma_lock in DMA completion callback

[Ilpo Järvinen]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Joshua Rogers <linux@joshua.hu>
> 
> The __dma_rx_do_complete() helper requires priv->rx_dma_lock to be held,
> but the DMA completion callback __dma_rx_complete() calls it without
> that lock, only holding the UART port lock. This allows a race between
> the callback and omap_8250_rx_dma_flush() which properly holds
> rx_dma_lock, leading to concurrent tty_insert_flip_string() calls that
> can corrupt flip-buffer state.

When the commit message mentions flush and buffer corruption, I 
immediately ask why James Hilliard needs so many lines to fix the flush 
issues [1] while this change just wraps one call with a lock?? I'd tend to 
think this change lacks most of the understanding for the relevant problem 
space.

I think this should be deferred to be part of James' series.

[1] https://lore.kernel.org/linux-serial/20261024-submit-serial-8250-dma-cancel-v1-v6-5-6f366591f566@gmail.com/

> Acquire rx_dma_lock around the call,
> matching the established lock ordering (port lock then rx_dma_lock).
>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Moteen Shah <m-shah@ti.com>
> Cc: Kendall Willis <k-willis@ti.com>
> Cc: Frank Li <Frank.Li@nxp.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: stable <stable@kernel.org>
> Signed-off-by: Joshua Rogers <linux@joshua.hu>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/8250/8250_omap.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 07ebed6ac6c9..0afd648733da 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -975,7 +975,9 @@ static void __dma_rx_complete(void *param)
>  	if (dmaengine_tx_status(dma->rxchan, dma->rx_cookie, &state) != DMA_COMPLETE)
>  		return;
>  
> +	spin_lock(&priv->rx_dma_lock);
>  	__dma_rx_do_complete(p);
> +	spin_unlock(&priv->rx_dma_lock);
>  	if (priv->throttled)
>  		return;



-- 
 i.

[PATCH 16/24] serial: 8250_lpss: don't divide by 0 above 6.25 Mbaud

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

byt_set_termios() derives the clock of the UART from the baud rate in
the termios, which nobody has looked at yet:

	fuart = baud * 16;
	...
	fuart *= rounddown_pow_of_two(fref / fuart);
	...
	p->uartclk = fuart;

fref is 100 MHz. For a baud rate above 6250000, which BOTHER allows,
fref / fuart is 0, and rounddown_pow_of_two(0) shifts 1UL by -1. On x86
that is 1UL << 63, the product with a multiple of 16 is 0, and so is
uartclk. serial8250_get_baud_rate() then passes 0 for both limits,
uart_get_baud_rate() finds no rate which fits, WARNs and returns 0, and
uart_get_divisor() or dw8250_get_divisor() divide by 16 * 0.

Everybody who may open the tty can oops the kernel with one TCSETS2.

Don't ask for more than fref. The limits are those of a 100 MHz clock
then, and uart_get_baud_rate() falls back to the old rate as for any
other rate that the port cannot do. 8250_mid, which has the same
formula, treats this case separately.

Fixes: 21947ba654a6 ("serial: 8250_pci: replace switch-case by formula")
Cc: stable <stable@kernel.org>
Cc: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_lpss.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c
index f1b4b975571a..7e88f2b793b5 100644
--- a/drivers/tty/serial/8250/8250_lpss.c
+++ b/drivers/tty/serial/8250/8250_lpss.c
@@ -83,6 +83,9 @@ static void byt_set_termios(struct uart_port *p, struct ktermios *termios,
 	/* Gracefully handle the B0 case: fall back to B9600 */
 	fuart = fuart ? fuart : 9600 * 16;
 
+	/* Fuart <= Fref: leave it to the core to refuse a higher baud rate */
+	fuart = min(fuart, fref);
+
 	/* Get Fuart closer to Fref */
 	fuart *= rounddown_pow_of_two(fref / fuart);
 

-- 
2.56.0

[PATCH 17/24] serial: 8250_mxpcie: handle hangup during break

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

Ending a software break on a Moxa PCIe port in RS-485 2-wire mode
dereferences a NULL tty pointer if the port was hung up while the break
was being sent.

send_break() calls ->break_ctl(tty, -1), sleeps for the break duration
and then calls ->break_ctl(tty, 0), without excluding a concurrent
hangup. A hangup (a modem DCD drop, or the session leader exiting while
another process of the session is in TCSBRK/TCSBRKP) runs uart_hangup(),
which shuts the port down and clears port->state->port.tty. The second
call still reaches mxpcie8250_software_break_ctl() through
uart_break_ctl(), which then does:

    struct tty_struct *tty = port->state->port.tty;
    ...
    baud = tty_get_baud_rate(tty);

with tty == NULL.

The bug was found by code inspection and has not been reproduced.

Fix by taking a reference to the port's tty with tty_port_tty_get() and
only restoring the divisor when a tty is still attached; a hung up port
is shut down, and the next open programs the divisor again through
set_termios().

Fixes: ade78d3d253c ("serial: 8250_mxpcie: add break support for RS485 using MUEx50 features")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Linmao Li <lilinmao@kylinos.cn>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/8250/8250_mxpcie.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mxpcie.c b/drivers/tty/serial/8250/8250_mxpcie.c
index 4935a5d1c1a3..b3418c1ccb05 100644
--- a/drivers/tty/serial/8250/8250_mxpcie.c
+++ b/drivers/tty/serial/8250/8250_mxpcie.c
@@ -412,7 +412,7 @@ static int mxpcie8250_handle_irq(struct uart_port *port)
 static void mxpcie8250_software_break_ctl(struct uart_port *port, int break_state)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
-	struct tty_struct *tty = port->state->port.tty;
+	struct tty_struct *tty;
 	unsigned int baud, quot;
 	u8 sfr, tx_byte = 0x01;
 
@@ -439,9 +439,14 @@ static void mxpcie8250_software_break_ctl(struct uart_port *port, int break_stat
 
 		serial_out(up, UART_FCR, UART_FCR_CLEAR_XMIT);
 
-		baud = tty_get_baud_rate(tty);
-		quot = uart_get_divisor(port, baud);
-		serial8250_do_set_divisor(port, baud, quot);
+		/* A hangup during the break detaches the tty */
+		tty = tty_port_tty_get(&port->state->port);
+		if (tty) {
+			baud = tty_get_baud_rate(tty);
+			quot = uart_get_divisor(port, baud);
+			serial8250_do_set_divisor(port, baud, quot);
+			tty_kref_put(tty);
+		}
 		serial_out(up, UART_LCR, up->lcr);
 	}
 }

-- 
2.56.0

[PATCH 18/24] serial: fsl_lpuart: fix tty ref of wakeup event

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

To report a wakeup event, lpuart_resume_noirq() gets the tty with
tty_port_tty_get() and never puts it: each wakeup by the UART leaks a
reference, and the tty is not freed when it is closed.

With no_console_suspend, the function also gets there for a console
which is not open.  Then tty_port_tty_get() returns NULL, which is
dereferenced.

Fixes: 18bdfccf3c30 ("tty: serial: fsl_lpuart: Add missing wakeup event reporting")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Shitalkumar Gandhi <shital.gandhi45@gmail.com>
Cc: Sherry Sun <sherry.sun@nxp.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/fsl_lpuart.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index c8575c965203..e54b4c44db42 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -3113,8 +3113,14 @@ static int lpuart_resume_noirq(struct device *dev)
 			/* check whether lpuart wakeup was triggered */
 			wake_active = stat & (UARTSTAT_RDRF | UARTSTAT_RXEDGIF);
 
-			if (wake_active && irqd_is_wakeup_set(irq_get_irq_data(sport->port.irq)))
-				pm_wakeup_event(tty_port_tty_get(port)->dev, 0);
+			if (wake_active && irqd_is_wakeup_set(irq_get_irq_data(sport->port.irq))) {
+				struct tty_struct *tty = tty_port_tty_get(port);
+
+				if (tty) {
+					pm_wakeup_event(tty->dev, 0);
+					tty_kref_put(tty);
+				}
+			}
 		}
 	}
 

-- 
2.56.0

[PATCH 19/24] serial: imx: fix the tty reference of the wakeup event

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

To report a wakeup event, imx_uart_enable_wakeup() gets the tty with
tty_port_tty_get() and never puts it: each wakeup by the UART leaks a
reference, and the tty is not freed when it is closed.

Fixes: 0cfadf4bcd28 ("tty: serial: imx: Add missing wakeup event reporting")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Frank Li <Frank.Li@nxp.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Robin Gong <yibin.gong@nxp.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: John Ogness <john.ogness@linutronix.de>
Cc: Pan Chuang <panchuang@vivo.com>
Cc: Karl Mehltretter <kmehltretter@gmail.com>
Cc: Sherry Sun <sherry.sun@nxp.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/imx.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 986fefb45c63..18892fc4a35b 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -2742,8 +2742,14 @@ static void imx_uart_enable_wakeup(struct imx_port *sport, bool on)
 		imx_uart_writel(sport, ucr1, UCR1);
 	}
 
-	if (wake_active && irqd_is_wakeup_set(irq_get_irq_data(sport->port.irq)))
-		pm_wakeup_event(tty_port_tty_get(port)->dev, 0);
+	if (wake_active && irqd_is_wakeup_set(irq_get_irq_data(sport->port.irq))) {
+		struct tty_struct *tty = tty_port_tty_get(port);
+
+		if (tty) {
+			pm_wakeup_event(tty->dev, 0);
+			tty_kref_put(tty);
+		}
+	}
 
 	uart_port_unlock_irq(&sport->port);
 }

-- 
2.56.0

[PATCH 20/24] serial: rda: free the interrupt on shutdown

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

rda_uart_startup() requests the interrupt, and nothing ever frees it.
It is not shared, so once the port has been closed, the next startup
fails with -EBUSY and the port cannot be opened any more.

Free it in rda_uart_shutdown().

Fixes: c10b13325ced ("tty: serial: Add RDA8810PL UART driver")
Cc: stable <stable@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Olof Johansson <olof@lixom.net>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/rda-uart.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/tty/serial/rda-uart.c b/drivers/tty/serial/rda-uart.c
index 87fa30d68687..9c1785771cbb 100644
--- a/drivers/tty/serial/rda-uart.c
+++ b/drivers/tty/serial/rda-uart.c
@@ -471,6 +471,8 @@ static void rda_uart_shutdown(struct uart_port *port)
 	rda_uart_write(port, val, RDA_UART_CTRL);
 
 	uart_port_unlock_irqrestore(port, flags);
+
+	free_irq(port->irq, port);
 }
 
 static const char *rda_uart_type(struct uart_port *port)

-- 
2.56.0

Re: [PATCH 20/24] serial: rda: free the interrupt on shutdown

[Manivannan Sadhasivam]

On Tue, Oct 27, 2026 at 12:06:46PM +0100, Greg Kroah-Hartman wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> rda_uart_startup() requests the interrupt, and nothing ever frees it.
> It is not shared, so once the port has been closed, the next startup
> fails with -EBUSY and the port cannot be opened any more.
> 
> Free it in rda_uart_shutdown().
> 
> Fixes: c10b13325ced ("tty: serial: Add RDA8810PL UART driver")
> Cc: stable <stable@kernel.org>
> Cc: Manivannan Sadhasivam <mani@kernel.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: "Andreas Färber" <afaerber@suse.de>
> Cc: Olof Johansson <olof@lixom.net>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

- Mani

-- 
மணிவண்ணன் சதாசிவம்

[PATCH 21/24] serial: samsung: don't leak the tty on an RX IRQ

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

s3c24xx_serial_rx_chars_dma() starts by taking a reference to the tty.
If the interrupt is not a receive timeout, it switches the port from PIO
back to DMA and leaves through the "finish" label, which does not drop
the reference.

A port with RX DMA changes to PIO on every receive timeout, that is at
the end of every burst of data, and comes back this way when the next
burst starts.  So each burst takes a reference which is never dropped,
and the tty structure is not freed when the port is closed for the last
time.

Drop the reference on both paths.

Fixes: b543c301c2c5 ("serial: samsung: add DMA support for RX")
Cc: stable <stable@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Peter Griffin <peter.griffin@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Robert Baldyga <r.baldyga@samsung.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/samsung_tty.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index 640f85e2d636..c9a870a8d155 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -736,14 +736,13 @@ static irqreturn_t s3c24xx_serial_rx_chars_dma(struct s3c24xx_uart_port *ourport
 
 	s3c24xx_serial_rx_drain_fifo(ourport);
 
-	if (tty) {
+	if (tty)
 		tty_flip_buffer_push(t);
-		tty_kref_put(tty);
-	}
 
 	wr_regl(port, S3C2410_UTRSTAT, S3C2410_UTRSTAT_TIMEOUT);
 
 finish:
+	tty_kref_put(tty);
 	uart_port_unlock(port);
 
 	return IRQ_HANDLED;

-- 
2.56.0

[PATCH 22/24] serial: samsung: put the previous baud clock

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

s3c24xx_serial_getclk() returns the best baud clock with the reference
which clk_get() has taken.  s3c24xx_serial_set_termios() stores it in
ourport->baudclk and disables the clock which was there before, but does
not put it.

clk_get() hands out a new handle on every call, so the new clock never
equals the old one, and every call of ->set_termios() leaks a handle: a
struct clk and a copy of the connection name.  Whoever has the port open
can repeat that at will with tcsetattr().

Put the old clock when it is replaced, and the new one in case it is not
stored.

Fixes: 5f5a7a5578c5 ("serial: samsung: switch to clkdev based clock lookup")
Cc: stable <stable@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Peter Griffin <peter.griffin@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
Cc: Thomas Abraham <thomas.abraham@linaro.org>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/samsung_tty.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
index c9a870a8d155..f437f7363fd3 100644
--- a/drivers/tty/serial/samsung_tty.c
+++ b/drivers/tty/serial/samsung_tty.c
@@ -1501,11 +1501,14 @@ static void s3c24xx_serial_set_termios(struct uart_port *port,
 
 		if (!IS_ERR(ourport->baudclk)) {
 			clk_disable_unprepare(ourport->baudclk);
+			clk_put(ourport->baudclk);
 			ourport->baudclk = ERR_PTR(-EINVAL);
 		}
 
 		ourport->baudclk = clk;
 		ourport->baudclk_rate = clk ? clk_get_rate(clk) : 0;
+	} else {
+		clk_put(clk);
 	}
 
 	if (ourport->info->has_divslot) {

-- 
2.56.0

[PATCH 23/24] serial: st-asc: don't report a wakeup to a closed tty

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

If its interrupt is a wakeup source, asc_receive_chars() reports the
characters which arrive as a wakeup event of tport->tty->dev.  That
pointer is only set while the port is open.  The port of a console is
not shut down by the last close, so its interrupt handler goes on
running without a tty.  If the system is then woken up by a character on
the console, the handler dereferences a NULL pointer.

Take a reference to the tty, and leave the event out if there is none.

Fixes: c4b058560762 ("serial:st-asc: Add ST ASC driver.")
Cc: stable <stable@kernel.org>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Srinivas Kandagatla <srini@kernel.org>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/st-asc.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index 6ed9a327702b..006874598b6c 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -262,8 +262,15 @@ static void asc_receive_chars(struct uart_port *port)
 	if (mode == ASC_CTL_MODE_8BIT || mode == ASC_CTL_MODE_8BIT_PAR)
 		ignore_pe = true;
 
-	if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
-		pm_wakeup_event(tport->tty->dev, 0);
+	if (irqd_is_wakeup_set(irq_get_irq_data(port->irq))) {
+		struct tty_struct *tty = tty_port_tty_get(tport);
+
+		/* A console goes on receiving after the last close */
+		if (tty) {
+			pm_wakeup_event(tty->dev, 0);
+			tty_kref_put(tty);
+		}
+	}
 
 	while ((status = asc_in(port, ASC_STA)) & ASC_STA_RBF) {
 		c = asc_in(port, ASC_RXBUF) | ASC_RXBUF_DUMMY_RX;

-- 
2.56.0

Re: [PATCH 23/24] serial: st-asc: don't report a wakeup to a closed tty

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 12:06:49PM +0100, Greg Kroah-Hartman wrote:
> 
> If its interrupt is a wakeup source, asc_receive_chars() reports the
> characters which arrive as a wakeup event of tport->tty->dev.  That
> pointer is only set while the port is open.  The port of a console is
> not shut down by the last close, so its interrupt handler goes on
> running without a tty.  If the system is then woken up by a character on
> the console, the handler dereferences a NULL pointer.
> 
> Take a reference to the tty, and leave the event out if there is none.

...

> -	if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
> -		pm_wakeup_event(tport->tty->dev, 0);
> +	if (irqd_is_wakeup_set(irq_get_irq_data(port->irq))) {
> +		struct tty_struct *tty = tty_port_tty_get(tport);
> +
> +		/* A console goes on receiving after the last close */
> +		if (tty) {
> +			pm_wakeup_event(tty->dev, 0);
> +			tty_kref_put(tty);
> +		}
> +	}

I believe I saw already something like this. Taking into consideration that can
be a helper for all who wants the same functionality.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 24/24] serial: stm32: don't report a wakeup to a closed tty

[Greg Kroah-Hartman]

From: Josef Bacik <josef@toxicpanda.com>

stm32_usart_interrupt() reports a wakeup interrupt as a wakeup event of
tport->tty->dev.  That pointer is only set while the port is open.  The
port of a console is not shut down by the last close: it stays
initialized, so the wakeup interrupt is still enabled on suspend.  If
the system is then woken up by the console, the handler dereferences a
NULL pointer.

Take a reference to the tty, and leave the event out if there is none.

Fixes: 48a6092fb41f ("serial: stm32-usart: Add STM32 USART Driver")
Cc: stable <stable@kernel.org>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
Cc: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/tty/serial/stm32-usart.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index ad06b760cfca..3bc2ce877585 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -872,8 +872,15 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
 		writel_relaxed(USART_ICR_WUCF,
 			       port->membase + ofs->icr);
 		stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_WUFIE);
-		if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
-			pm_wakeup_event(tport->tty->dev, 0);
+		if (irqd_is_wakeup_set(irq_get_irq_data(port->irq))) {
+			struct tty_struct *tty = tty_port_tty_get(tport);
+
+			/* A console stays set up after the last close */
+			if (tty) {
+				pm_wakeup_event(tty->dev, 0);
+				tty_kref_put(tty);
+			}
+		}
 		ret = IRQ_HANDLED;
 	}
 

-- 
2.56.0

Re: [PATCH 24/24] serial: stm32: don't report a wakeup to a closed tty

[Ilpo Järvinen]

On Tue, 27 Oct 2026, Greg Kroah-Hartman wrote:

> From: Josef Bacik <josef@toxicpanda.com>
> 
> stm32_usart_interrupt() reports a wakeup interrupt as a wakeup event of
> tport->tty->dev.  That pointer is only set while the port is open.  The
> port of a console is not shut down by the last close: it stays
> initialized, so the wakeup interrupt is still enabled on suspend.  If
> the system is then woken up by the console, the handler dereferences a
> NULL pointer.
> 
> Take a reference to the tty, and leave the event out if there is none.
> 
> Fixes: 48a6092fb41f ("serial: stm32-usart: Add STM32 USART Driver")
> Cc: stable <stable@kernel.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> Cc: Peter Hurley <peter@hurleysoftware.com>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/tty/serial/stm32-usart.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
> index ad06b760cfca..3bc2ce877585 100644
> --- a/drivers/tty/serial/stm32-usart.c
> +++ b/drivers/tty/serial/stm32-usart.c
> @@ -872,8 +872,15 @@ static irqreturn_t stm32_usart_interrupt(int irq, void *ptr)
>  		writel_relaxed(USART_ICR_WUCF,
>  			       port->membase + ofs->icr);
>  		stm32_usart_clr_bits(port, ofs->cr3, USART_CR3_WUFIE);
> -		if (irqd_is_wakeup_set(irq_get_irq_data(port->irq)))
> -			pm_wakeup_event(tport->tty->dev, 0);
> +		if (irqd_is_wakeup_set(irq_get_irq_data(port->irq))) {
> +			struct tty_struct *tty = tty_port_tty_get(tport);
> +
> +			/* A console stays set up after the last close */
> +			if (tty) {
> +				pm_wakeup_event(tty->dev, 0);
> +				tty_kref_put(tty);
> +			}
> +		}

Could we like add a helper into serial core to do this tty wakeup dance? 
It is duplicated in quite many of these patches.


-- 
 i.

Re: [PATCH 00/24] tty/serial: a bunch of llm-found fixes

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 12:06:26PM +0100, Greg Kroah-Hartman wrote:
> Here's a series of some small llm-found fixes that have been sent to me
> recently.  Most of them are just minor bugfixes and locking cleanups,
> but they are good to have from what I can tell.
> 
> Submitting them here to get a solid Sashiko run and some second eyes on
> them before merging if all goes well.

Since it looks like v2 might be issued, can we move all Cc lists in the patches
outside of the commit messages? (Usually it can be done by moving them below
'---' line.) This reduces a noise in the commit messages and allows to see
better history on a single screen page.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 00/24] tty/serial: a bunch of llm-found fixes

[Greg Kroah-Hartman]

On Tue, Oct 27, 2026 at 05:06:31PM +0200, Andy Shevchenko wrote:
> On Tue, Oct 27, 2026 at 12:06:26PM +0100, Greg Kroah-Hartman wrote:
> > Here's a series of some small llm-found fixes that have been sent to me
> > recently.  Most of them are just minor bugfixes and locking cleanups,
> > but they are good to have from what I can tell.
> > 
> > Submitting them here to get a solid Sashiko run and some second eyes on
> > them before merging if all goes well.
> 
> Since it looks like v2 might be issued, can we move all Cc lists in the patches
> outside of the commit messages? (Usually it can be done by moving them below
> '---' line.) This reduces a noise in the commit messages and allows to see
> better history on a single screen page.

b4 doesn't do that "below the --- line" mess, but I will trim things
better next time around, this was a mess, sorry about that.

greg k-h

Re: [PATCH 00/24] tty/serial: a bunch of llm-found fixes

[Andy Shevchenko]

On Tue, Oct 27, 2026 at 04:50:43PM +0100, Greg Kroah-Hartman wrote:
> On Tue, Oct 27, 2026 at 05:06:31PM +0200, Andy Shevchenko wrote:
> > On Tue, Oct 27, 2026 at 12:06:26PM +0100, Greg Kroah-Hartman wrote:
> > > Here's a series of some small llm-found fixes that have been sent to me
> > > recently.  Most of them are just minor bugfixes and locking cleanups,
> > > but they are good to have from what I can tell.
> > > 
> > > Submitting them here to get a solid Sashiko run and some second eyes on
> > > them before merging if all goes well.
> > 
> > Since it looks like v2 might be issued, can we move all Cc lists in the patches
> > outside of the commit messages? (Usually it can be done by moving them below
> > '---' line.) This reduces a noise in the commit messages and allows to see
> > better history on a single screen page.
> 
> b4 doesn't do that "below the --- line" mess, but I will trim things
> better next time around, this was a mess, sorry about that.

Actually you can achieve that by keeping them with --- locally. Another
possibility is to use `b4 trailers`. It was a useful discussion here on
the topic: https://lore.kernel.org/lkml/20260123113708.416727-19-bigeasy@linutronix.de/
(read the patch 18 subthread).

-- 
With Best Regards,
Andy Shevchenko