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

[Jiri Slaby <jirislaby@kernel.org>]

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