Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports
@ 2026-10-10  3:46 Hengyu Liang
  2026-10-10  3:59 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Hengyu Liang @ 2026-10-10  3:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Johan Hovold, linux-serial, linux-kernel, stable

Commit 8ca59f7baee7 ("serial: fix ioctl hangup race") made
uart_set_info() and uart_do_autoconfig() return -EIO when TTY_IO_ERROR
is set, so that they do not run on a port that has been hung up.

However, TTY_IO_ERROR is also set on a port that could not be started,
for example because its type is unknown, and by uart_shutdown(), which
both functions call. As of now, setserial cannot configure a port that
the kernel did not detect, and an autoconfig that follows a port, irq or
type change in the same setserial call fails on any port.

The issue can be reproduced with setserial (busybox) on a port without
a detected UART, for example ttyS3 of a QEMU guest:

    setserial /dev/ttyS3 uart 16550A
    setserial -g /dev/ttyS3

Before commit 8ca59f7baee7 ("serial: fix ioctl hangup race"), the
result is:

    /dev/ttyS3, UART: 16550A, Port: 0x02e8, IRQ: 3

After that commit, the result is:

    setserial: can't set serial info: Input/output error
    /dev/ttyS3, UART: unknown, Port: 0x02e8, IRQ: 3

This patch will check tty_port_active() in these two functions instead.
uart_hangup() clears it under the port mutex, and it stays set on an
open port that could not be started.

Fixes: 8ca59f7baee7 ("serial: fix ioctl hangup race")
Cc: stable@vger.kernel.org
Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
Commit 8ca59f7baee7 is in the stable queue for 7.2.

Tested on v7.3-rc6 in QEMU (8250). A TIOCSSERIAL that is held in
copy_from_user() with userfaultfd while the tty is hung up with
TIOCVHANGUP still gets -EIO from uart_set_info() with this patch.

 drivers/tty/serial/serial_core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 6332ed545c89..81a985424694 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -896,7 +896,11 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
 	upf_t old_flags, new_flags;
 	int retval;
 
-	if (!uport || tty_io_error(tty))
+	/*
+	 * TTY_IO_ERROR is also set on a port that could not be started, and
+	 * this is how such a port is configured. Only refuse a hung up port.
+	 */
+	if (!uport || !tty_port_active(port))
 		return -EIO;
 
 	new_port = new_info->port;
@@ -1144,7 +1148,8 @@ static int uart_do_autoconfig(struct tty_struct *tty, struct uart_state *state)
 	 */
 	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &port->mutex) {
 		uport = uart_port_check(state);
-		if (!uport || tty_io_error(tty))
+		/* As in uart_set_info(), only refuse a hung up port. */
+		if (!uport || !tty_port_active(port))
 			return -EIO;
 
 		if (tty_port_users(port) != 1)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports
  2026-10-10  3:46 [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports Hengyu Liang
@ 2026-10-10  3:59 ` sashiko-bot
  2026-10-10  6:16 ` Sasha Levin
  2026-10-11 12:16 ` Johan Hovold
  2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-10-10  3:59 UTC (permalink / raw)
  To: Hengyu Liang; +Cc: linux-serial

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] serial: hw access without PM in uart_do_autoconfig()
--

commit f48636ab329a71ecd2a47c54111b53dd3b5b35da
Author: Hengyu Liang <hengyul@cs.unc.edu>

serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports

This commit changes the check in uart_set_info() and uart_do_autoconfig()
from tty_io_error() to tty_port_active(). This allows setserial to configure
a port that the kernel did not detect, fixing an issue where uninitialized
ports could not be configured.

> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 319a4b427f3a3..36c59f5fadebd 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
[ ... ]
> @@ -1143,7 +1147,8 @@ static int uart_do_autoconfig(struct tty_struct *tty, struct uart_state *state)
>  	 */
>  	scoped_cond_guard(mutex_intr, return -ERESTARTSYS, &port->mutex) {
>  		uport = uart_port_check(state);
> -		if (!uport || tty_io_error(tty))
> +		/* As in uart_set_info(), only refuse a hung up port. */
> +		if (!uport || !tty_port_active(port))
>  			return -EIO;

[Severity: High]
Does relaxing this check to tty_port_active() allow the ioctl to proceed on
uninitialized (PORT_UNKNOWN) ports where the power management state is still
OFF?

A user with CAP_SYS_ADMIN could call open() on an uninitialized port which
succeeds but sets TTY_IO_ERROR. If they then invoke the TIOCSERCONFIG ioctl,
the active but uninitialized port would pass this check.

Further down in uart_do_autoconfig(), the code executes:

    uport->ops->config_port(uport, flags);

Could this unconditionally invoke the driver's config_port callback before
uart_startup() has a chance to enable power and clocks? On architectures with
strict power domain management, accessing hardware registers without runtime
PM enabled can trigger a fatal bus error (synchronous external abort) and
kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010034616.3915864-1-hengyul@cs.unc.edu?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports
  2026-10-10  3:46 [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports Hengyu Liang
  2026-10-10  3:59 ` sashiko-bot
@ 2026-10-10  6:16 ` Sasha Levin
  2026-10-11 12:16 ` Johan Hovold
  2 siblings, 0 replies; 5+ messages in thread
From: Sasha Levin @ 2026-10-10  6:16 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: Sasha Levin, Johan Hovold, linux-serial, linux-kernel, stable,
	Hengyu Liang

On Fri, Oct 09, 2026 at 11:46:16PM -0400, Hengyu Liang wrote:
> Commit 8ca59f7baee7 ("serial: fix ioctl hangup race") made
> uart_set_info() and uart_do_autoconfig() return -EIO when TTY_IO_ERROR
> is set, so that they do not run on a port that has been hung up.

Dropped 8ca59f7baee7 from the 7.2 queue along with the following dependency:
  - serial: abort TIOCMIWAIT on hangup (eb35b73e1336), which needs it

6.18 had already dropped both of them.

We will pick 8ca59f7baee7 back up together with your fix once that is in
mainline.

Thanks for the report.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports
  2026-10-10  3:46 [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports Hengyu Liang
  2026-10-10  3:59 ` sashiko-bot
  2026-10-10  6:16 ` Sasha Levin
@ 2026-10-11 12:16 ` Johan Hovold
  2026-10-11 15:00   ` Hengyu Liang
  2 siblings, 1 reply; 5+ messages in thread
From: Johan Hovold @ 2026-10-11 12:16 UTC (permalink / raw)
  To: Hengyu Liang
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	stable

On Fri, Oct 09, 2026 at 11:46:16PM -0400, Hengyu Liang wrote:
> Commit 8ca59f7baee7 ("serial: fix ioctl hangup race") made
> uart_set_info() and uart_do_autoconfig() return -EIO when TTY_IO_ERROR
> is set, so that they do not run on a port that has been hung up.
> 
> However, TTY_IO_ERROR is also set on a port that could not be started,
> for example because its type is unknown, and by uart_shutdown(), which
> both functions call. As of now, setserial cannot configure a port that
> the kernel did not detect, and an autoconfig that follows a port, irq or
> type change in the same setserial call fails on any port.

Thanks for catching this.

How did you find it by the way? I assume you are not using an ISA serial
card?

> This patch will check tty_port_active() in these two functions instead.
> uart_hangup() clears it under the port mutex, and it stays set on an
> open port that could not be started.

Yeah, this should work. (And the flag is now also cleared under the port
mutex by tty_port_hangup(), which serial core currently does not use.)
 
> Fixes: 8ca59f7baee7 ("serial: fix ioctl hangup race")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
> ---
> Commit 8ca59f7baee7 is in the stable queue for 7.2.
> 
> Tested on v7.3-rc6 in QEMU (8250). A TIOCSSERIAL that is held in
> copy_from_user() with userfaultfd while the tty is hung up with
> TIOCVHANGUP still gets -EIO from uart_set_info() with this patch.

Tested-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Johan Hovold <johan@kernel.org>

Johan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports
  2026-10-11 12:16 ` Johan Hovold
@ 2026-10-11 15:00   ` Hengyu Liang
  0 siblings, 0 replies; 5+ messages in thread
From: Hengyu Liang @ 2026-10-11 15:00 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel,
	stable

Hi Johan,

Thanks for testing and reviewing the patch.

> How did you find it by the way? I assume you are not using an ISA serial
> card?

I found it by comparing kernel behavior across different versions. I was
testing in QEMU with the 8250 driver, without a physical ISA serial card.

Hengyu

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-11 15:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-10  3:46 [PATCH] serial: core: fix TIOCSSERIAL and TIOCSERCONFIG on uninitialised ports Hengyu Liang
2026-10-10  3:59 ` sashiko-bot
2026-10-10  6:16 ` Sasha Levin
2026-10-11 12:16 ` Johan Hovold
2026-10-11 15:00   ` Hengyu Liang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox