[PATCH v2] serial: Fix not set tty->port race condition

[PATCH v2] serial: Fix not set tty->port race condition

[Krzysztof Kozlowski]

Revert commit bfc467db60b7 ("serial: remove redundant
tty_port_link_device()") because the tty_port_link_device() is not
redundant: the tty->port has to be confured before we call
uart_configure_port(), otherwise user-space can open console without TTY
linked to the driver.

This tty_port_link_device() was added explicitly to avoid this exact
issue in commit fb2b90014d78 ("tty: link tty and port before configuring
it as console"), so offending commit basically reverted the fix saying
it is redundant without addressing the actual race condition presented
there.

Reproducible always as tty->port warning on Qualcomm SoC with most of
devices disabled, so with very fast boot, and one serial device being
the console:

  printk: legacy console [ttyMSM0] enabled
  printk: legacy console [ttyMSM0] enabled
  printk: legacy bootconsole [qcom_geni0] disabled
  printk: legacy bootconsole [qcom_geni0] disabled
  ------------[ cut here ]------------
  tty_init_dev: ttyMSM driver does not set tty->port. This would crash the kernel. Fix the driver!
  WARNING: drivers/tty/tty_io.c:1414 at tty_init_dev.part.0+0x228/0x25c, CPU#2: systemd/1
  Modules linked in: socinfo tcsrcc_eliza gcc_eliza sm3_ce fuse ipv6
  CPU: 2 UID: 0 PID: 1 Comm: systemd Tainted: G S                  6.19.0-rc4-next-20260108-00024-g2202f4d30aa8 #73 PREEMPT
  Tainted: [S]=CPU_OUT_OF_SPEC
  Hardware name: Qualcomm Technologies, Inc. Eliza (DT)
  ...
  tty_init_dev.part.0 (drivers/tty/tty_io.c:1414 (discriminator 11)) (P)
  tty_open (arch/arm64/include/asm/atomic_ll_sc.h:95 (discriminator 3) drivers/tty/tty_io.c:2073 (discriminator 3) drivers/tty/tty_io.c:2120 (discriminator 3))
  chrdev_open (fs/char_dev.c:411)
  do_dentry_open (fs/open.c:962)
  vfs_open (fs/open.c:1094)
  do_open (fs/namei.c:4634)
  path_openat (fs/namei.c:4793)
  do_filp_open (fs/namei.c:4820)
  do_sys_openat2 (fs/open.c:1391 (discriminator 3))
  ...
  Starting Network Name Resolution...

Apparently the flow with this small Yocto-based ramdisk user-space is:

driver (qcom_geni_serial.c):                  user-space:
============================                  ===========
qcom_geni_serial_probe()
 uart_add_one_port()
  serial_core_register_port()
   serial_core_add_one_port()
    uart_configure_port()
     register_console()
    |
    |                                         open console
    |                                          ...
    |                                          tty_init_dev()
    |                                           driver->ports[idx] is NULL
    |
    tty_port_register_device_attr_serdev()
     tty_port_link_device() <- set driver->ports[idx]

Fixes: bfc467db60b7 ("serial: remove redundant tty_port_link_device()")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

---

Changes in v2:
1. Add comment to the code.
---
 drivers/tty/serial/serial_core.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9930023e924c..2805cad10511 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3074,6 +3074,12 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
 	if (uport->cons && uport->dev)
 		of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
 
+	/*
+	 * TTY port has to be linked with the driver before register_console()
+	 * in uart_configure_port(), because user-space could open the console
+	 * immediately after.
+	 */
+	tty_port_link_device(port, drv->tty_driver, uport->line);
 	uart_configure_port(drv, state, uport);
 
 	port->console = uart_console(uport);
-- 
2.51.0

Re: [PATCH v2] serial: Fix not set tty->port race condition

[Jiri Slaby]

On 23. 01. 26, 8:21, Krzysztof Kozlowski wrote:
> Revert commit bfc467db60b7 ("serial: remove redundant
> tty_port_link_device()") because the tty_port_link_device() is not
> redundant: the tty->port has to be confured before we call
> uart_configure_port(), otherwise user-space can open console without TTY
> linked to the driver.
> 
> This tty_port_link_device() was added explicitly to avoid this exact
> issue in commit fb2b90014d78 ("tty: link tty and port before configuring
> it as console"), so offending commit basically reverted the fix saying
> it is redundant without addressing the actual race condition presented
> there.
> 
> Reproducible always as tty->port warning on Qualcomm SoC with most of
> devices disabled, so with very fast boot, and one serial device being
> the console:
> 
>    printk: legacy console [ttyMSM0] enabled
>    printk: legacy console [ttyMSM0] enabled
>    printk: legacy bootconsole [qcom_geni0] disabled
>    printk: legacy bootconsole [qcom_geni0] disabled
>    ------------[ cut here ]------------
>    tty_init_dev: ttyMSM driver does not set tty->port. This would crash the kernel. Fix the driver!
>    WARNING: drivers/tty/tty_io.c:1414 at tty_init_dev.part.0+0x228/0x25c, CPU#2: systemd/1
>    Modules linked in: socinfo tcsrcc_eliza gcc_eliza sm3_ce fuse ipv6
>    CPU: 2 UID: 0 PID: 1 Comm: systemd Tainted: G S                  6.19.0-rc4-next-20260108-00024-g2202f4d30aa8 #73 PREEMPT
>    Tainted: [S]=CPU_OUT_OF_SPEC
>    Hardware name: Qualcomm Technologies, Inc. Eliza (DT)
>    ...
>    tty_init_dev.part.0 (drivers/tty/tty_io.c:1414 (discriminator 11)) (P)
>    tty_open (arch/arm64/include/asm/atomic_ll_sc.h:95 (discriminator 3) drivers/tty/tty_io.c:2073 (discriminator 3) drivers/tty/tty_io.c:2120 (discriminator 3))
>    chrdev_open (fs/char_dev.c:411)
>    do_dentry_open (fs/open.c:962)
>    vfs_open (fs/open.c:1094)
>    do_open (fs/namei.c:4634)
>    path_openat (fs/namei.c:4793)
>    do_filp_open (fs/namei.c:4820)
>    do_sys_openat2 (fs/open.c:1391 (discriminator 3))
>    ...
>    Starting Network Name Resolution...
> 
> Apparently the flow with this small Yocto-based ramdisk user-space is:
> 
> driver (qcom_geni_serial.c):                  user-space:
> ============================                  ===========
> qcom_geni_serial_probe()
>   uart_add_one_port()
>    serial_core_register_port()
>     serial_core_add_one_port()
>      uart_configure_port()
>       register_console()
>      |
>      |                                         open console
>      |                                          ...
>      |                                          tty_init_dev()
>      |                                           driver->ports[idx] is NULL
>      |
>      tty_port_register_device_attr_serdev()
>       tty_port_link_device() <- set driver->ports[idx]
> 
> Fixes: bfc467db60b7 ("serial: remove redundant tty_port_link_device()")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Thanks for the update.

Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

-- 
js
suse labs