linux-kernel - RE: [PATCH 1/2] printk: nbcon: move locked_port flag to struct uart

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 25 Jan 2024 01:08:24 +0000
From: "Chang, Junxiao" <junxiao.chang@...el.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>, John Ogness
	<john.ogness@...utronix.de>
CC: "tglx@...utronix.de" <tglx@...utronix.de>, "rostedt@...dmis.org"
	<rostedt@...dmis.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Li, Hao3" <hao3.li@...el.com>, "Li, Lili"
	<lili.li@...el.com>, "Gao, Jianfeng" <jianfeng.gao@...el.com>,
	"linux-rt-users@...r.kernel.org" <linux-rt-users@...r.kernel.org>
Subject: RE: [PATCH 1/2] printk: nbcon: move locked_port flag to struct
 uart_port

> > > Console pointer in uart_port might be shared among multiple uart 
> > > ports.
> > 
> > I still want to investigate why the pointer is shared. This sounds 
> > sloppy or dangerous.

> I have x86 a server box and PNP enumerates two UARTs (8250). Only one is wired up but both can be specified as console=.
> What do I need to do to reproduce this here? Using console= twice does not do the trick.

Issue could be reproduced with our hardware every time. My cmdline is: BOOT_IMAGE=(hd0,gpt2)/boot/bzImage-linux-intel-iot-lts-rt-6.6-kernel root=PARTLABEL=primary rootwait console=ttyS0,115200 console=tty0 init=/sbin/preinit-env console=ttyS4,115200n8 console=ttyS5,115200n8

If you would like to try any debug patch with my ADL hardware, please feel free to let me know.

For console pointer sharing issue, from code logic point of view, the call chain looks like:
serial8250_register_8250_port -> uart_add_one_port -> serial_ctrl_register_port -> serial_core_register_port -> serial_core_add_one_port

In API serial_core_add_one_port, uart_port's console pointer is assigned with driver's console pointer:
	uport->cons = drv->cons;
Driver's console pointer points to static structure "univ8250_console" which is defined in 8250_core.c

That is, all 8250 serial devices' console pointer are same, they point to univ8250_console.

Below is debug log output:
sh-5.1# dmesg | grep @@@@
[    1.687121] @@@@ univ8250_console_init univ8250_console address:ffffffff935df1e0
[    3.419880] @@@@ serial8250_register_8250_port: name:ttyS4, cons pointer:ffffffff935df1e0
[    3.534954] @@@@ serial8250_register_8250_port: name:ttyS5, cons pointer:ffffffff935df1e0
[   11.971345] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS0, line 0, cons pointer:ffffffff935df1e0
[   11.971506] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS4, line 4, cons pointer:ffffffff935df1e0
[   11.971849] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS4, line 4, cons pointer:ffffffff935df1e0
[   11.983072] @@@@ serial8250_do_shutdown, curr thread:agetty, name:ttyS4, line 4, cons pointer:ffffffff935df1e0
[   11.983595] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS5, line 5, cons pointer:ffffffff935df1e0
[   11.983895] @@@@ serial8250_do_shutdown, curr thread:(agetty), name:ttyS5, line 5, cons pointer:ffffffff935df1e0
[   12.009632] @@@@ serial8250_do_shutdown, curr thread:agetty, name:ttyS5, line 5, cons pointer:ffffffff935df1e0
sh-5.1#

All cons pointers address are same "ffffffff935df1e0".

Debug patch:
diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
index 30434718fad80..740fd7e133a28 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -755,6 +755,7 @@ static int __init univ8250_console_init(void)

        serial8250_isa_init_ports();
        register_console(&univ8250_console);
+       printk("@@@@ %s univ8250_console address:%lx\n", __func__, (unsigned long)(&univ8250_console));
        return 0;
 }
 console_initcall(univ8250_console_init);
@@ -1181,6 +1182,7 @@ int serial8250_register_8250_port(const struct uart_8250_port *up)
                        if (ret)
                                goto err;

+                       printk("@@@@ %s: name:%s, cons pointer:%lx\n", __func__, uart->port.name, (unsigned long)uart->port.cons);
                        ret = uart->port.line;
                } else {
                        dev_info(uart->port.dev,
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 437a7d3d34cde..93f1e548d1301 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2510,6 +2510,7 @@ void serial8250_do_shutdown(struct uart_port *port)
        struct uart_8250_port *up = up_to_u8250p(port);
        unsigned long flags;

+       printk("@@@@ %s, curr thread:%s, name:%s, line %d, cons pointer:%lx\n", __func__, current->comm, port->name, port->line, (unsigned long)port->cons);
        serial8250_rpm_get(up);
        /*
         * Disable interrupts from this port

Thanks,
Junxiao