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

Krzysztof Kozlowski posted 1 patch 2 weeks, 3 days ago
There is a newer version of this series
drivers/tty/serial/serial_core.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] serial: Fix not set tty->port race condition
Posted by Krzysztof Kozlowski 2 weeks, 3 days ago
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>
---
 drivers/tty/serial/serial_core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0534b2eb1682..116f33f0643f 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3077,6 +3077,7 @@ 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_link_device(port, drv->tty_driver, uport->line);
 	uart_configure_port(drv, state, uport);
 
 	port->console = uart_console(uport);
-- 
2.51.0
Re: [PATCH] serial: Fix not set tty->port race condition
Posted by Jiri Slaby 2 weeks, 2 days ago
On 22. 01. 26, 18:00, 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>
> ---
>   drivers/tty/serial/serial_core.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 0534b2eb1682..116f33f0643f 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3077,6 +3077,7 @@ 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_link_device(port, drv->tty_driver, uport->line);

Bah, so add a comment or I (or somebody) remove it again eventually :(.

thanks,
-- 
js
suse labs
Re: [PATCH] serial: Fix not set tty->port race condition
Posted by Krzysztof Kozlowski 2 weeks, 2 days ago
On 23/01/2026 06:55, Jiri Slaby wrote:
> On 22. 01. 26, 18:00, Krzysztof Kozlowski wrote:
>>
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 0534b2eb1682..116f33f0643f 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -3077,6 +3077,7 @@ 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_link_device(port, drv->tty_driver, uport->line);
> 
> Bah, so add a comment or I (or somebody) remove it again eventually :(.
> 

Good point.

Best regards,
Krzysztof
Re: [PATCH] serial: Fix not set tty->port race condition
Posted by Krzysztof Kozlowski 2 weeks, 3 days ago
On 22/01/2026 18:00, Krzysztof Kozlowski wrote:
> Revert commit bfc467db60b7 ("serial: remove redundant
> tty_port_link_device()") because the tty_port_link_device() is not

And grumpy side note because I was looking at this for more than a day
blaming my new hardware:

I really wish commits (e.g. bfc467db60b7) calling something redundant
had that much of message written why something is redundant as the
commit (fb2b90014d78) which introduced that part of code.

If someone wrote one page of text why foo is needed, we should write not
less why it is not needed :)

Best regards,
Krzysztof
Re: [PATCH] serial: Fix not set tty->port race condition
Posted by Jiri Slaby 2 weeks, 2 days ago
On 22. 01. 26, 18:11, Krzysztof Kozlowski wrote:
> On 22/01/2026 18:00, Krzysztof Kozlowski wrote:
>> Revert commit bfc467db60b7 ("serial: remove redundant
>> tty_port_link_device()") because the tty_port_link_device() is not
> 
> And grumpy side note because I was looking at this for more than a day
> blaming my new hardware:
> 
> I really wish commits (e.g. bfc467db60b7) calling something redundant
> had that much of message written why something is redundant as the
> commit (fb2b90014d78) which introduced that part of code.

It was clear enough: because tty_port_register_device_attr_serdev() 
links the port few lines below.

But it/I somehow didn't take the hidden uart_console() in 
uart_configure_port() into account.

> If someone wrote one page of text why foo is needed, we should write not
> less why it is not needed :)

I think I could generate a bloat of text. But you will still have a 
broken kernel the same way :)?

thanks,
-- 
js
suse labs