public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] serial: Fix not set tty->port race condition
@ 2026-01-22 17:00 Krzysztof Kozlowski
  2026-01-22 17:11 ` Krzysztof Kozlowski
  2026-01-23  5:55 ` Jiri Slaby
  0 siblings, 2 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-22 17:00 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
  Cc: Krzysztof Kozlowski, stable

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


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

* Re: [PATCH] serial: Fix not set tty->port race condition
  2026-01-22 17:00 [PATCH] serial: Fix not set tty->port race condition Krzysztof Kozlowski
@ 2026-01-22 17:11 ` Krzysztof Kozlowski
  2026-01-23  5:59   ` Jiri Slaby
  2026-01-23  5:55 ` Jiri Slaby
  1 sibling, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-22 17:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial; +Cc: stable

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

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

* Re: [PATCH] serial: Fix not set tty->port race condition
  2026-01-22 17:00 [PATCH] serial: Fix not set tty->port race condition Krzysztof Kozlowski
  2026-01-22 17:11 ` Krzysztof Kozlowski
@ 2026-01-23  5:55 ` Jiri Slaby
  2026-01-23  7:13   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2026-01-23  5:55 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Greg Kroah-Hartman, linux-kernel,
	linux-serial; +Cc: stable

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

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

* Re: [PATCH] serial: Fix not set tty->port race condition
  2026-01-22 17:11 ` Krzysztof Kozlowski
@ 2026-01-23  5:59   ` Jiri Slaby
  0 siblings, 0 replies; 5+ messages in thread
From: Jiri Slaby @ 2026-01-23  5:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Greg Kroah-Hartman, linux-kernel,
	linux-serial; +Cc: stable

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

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

* Re: [PATCH] serial: Fix not set tty->port race condition
  2026-01-23  5:55 ` Jiri Slaby
@ 2026-01-23  7:13   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 5+ messages in thread
From: Krzysztof Kozlowski @ 2026-01-23  7:13 UTC (permalink / raw)
  To: Jiri Slaby, Greg Kroah-Hartman, linux-kernel, linux-serial; +Cc: stable

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

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

end of thread, other threads:[~2026-01-23  7:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-22 17:00 [PATCH] serial: Fix not set tty->port race condition Krzysztof Kozlowski
2026-01-22 17:11 ` Krzysztof Kozlowski
2026-01-23  5:59   ` Jiri Slaby
2026-01-23  5:55 ` Jiri Slaby
2026-01-23  7:13   ` Krzysztof Kozlowski

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