linux-kernel - Re: [PATCH 1/1] tty: serial: handle HAS

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]

Message-ID: <3d1e5e90-4675-4319-9f58-3c3d5b6f4b8a@roeck-us.net>
Date: Fri, 22 Nov 2024 18:14:13 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Arnd Bergmann <arnd@...nel.org>, Niklas Schnelle <schnelle@...ux.ibm.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Jiri Slaby <jirislaby@...nel.org>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
 linux-serial@...r.kernel.org, Heiko Carstens <hca@...ux.ibm.com>,
 linux-kernel@...r.kernel.org,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH 1/1] tty: serial: handle HAS_IOPORT dependencies

On 11/22/24 14:51, Arnd Bergmann wrote:
> On Fri, Nov 22, 2024, at 21:44, Guenter Roeck wrote:
>> On 11/22/24 11:24, Arnd Bergmann wrote:
>>>
>>> serial8250_setup_port() is where it ends up using the
>>> uninitialized serial8250_ports[index] contents. Since
>>> port->iotype is not set to anything here, it defaults to
>>> '0', which happens to be UPIO_PORT.
>>>
>>> The reason it doesn't immediately crash and burn is that
>>> this is still only setting up the structures for later
>>> use, but I assume that trying to use console=ttyS0, or
>>> opening /dev/ttyS0 on the uninitialized port would
>>> then cause an oops.
>>>
>>
>> All four affected platforms use ttyS0, only it is mmio based,
>> not io port based.
> 
> Right, so I think the problem is really the 8250_platform.c
> file trying to handle all possible cases on x86 (acpi,
> isapnp, legacy isa) that may define the same UARTs, but
> also trying to handle non-legacy ports with their own
> special cases on other architectures.
> 
> The patch below is a first idea of how we can skip the
> legacy ISA case on architectures that don't define those.
> If this works, we can try to come up with a better way of
> doing that. Ideally all the pre-DT boardfile machines that
> define their own "serial8250" platform_device (with platform_data)
> should use a different identifier the x86-legacy case that
> uses the platform_device (without platform_data). Similarly,
> I think the riscv special ACPI port needs its own trivial
> driver without all the ISA magic.
> 
>        Arnd
> 
> diff --git a/drivers/tty/serial/8250/8250_platform.c b/drivers/tty/serial/8250/8250_platform.c
> index 66fd6d5d4835..610b31517734 100644
> --- a/drivers/tty/serial/8250/8250_platform.c
> +++ b/drivers/tty/serial/8250/8250_platform.c

That didn't work.



[    0.005735] Console: colour dummy device 80x25
[    0.006393] BUG: spinlock bad magic on CPU#0, swapper/0
[    0.006459]  lock: serial8250_ports+0x0/0x980, .magic: 00000000, .owner: <none>/-1, .owner_cpu: 0
[    0.007102] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.12.0-07881-g6206e39999ae #1
[    0.007359] Stack:
[    0.007671] > 00000100 00000000 d09f9e10 d0951c2c
[    0.007765]   d0951c2c 00000000 00000000 d0b06170
[    0.007841]   9003f5ee d09f9e60 00000055 d0dd9900
[    0.007884]   00000000 00000004 d09f9e40 00000000
[    0.007925]   9003f681 d09f9e80 d0dd9900 d08f5924
[    0.007976] > 00000000 d08f5890 ffffffff 00000000
[    0.008019]   906f66b9 d09f9ea0 d0dd9900 00000000
[    0.008059] > 00000000 00000480 00000000 d0a06d74
[    0.008099]   904efe6a d09f9ec0 d0dd9900 00060300
[    0.008139] > 00000000 00000000 00000001 00000000
[    0.008180]   9004368d d09f9ef0 d0aa7238 d7ff70a4
[    0.008220] > d04efe6a 00000000 00000001 00000000
[    0.008260]   00000000 00000000 00000001 00000000
[    0.008301]   90044a84 d09f9f10 d0aa7238 d04efe14
[    0.008340] > d0dd9900 000000c4 d0a06d74 d0a00400
[    0.008381]   90ae6710 d09f9f30 d0aa7238 d0cedea0
[    0.008443] Call Trace:
[    0.008502]  [<d06ed63d>] dump_stack+0x9/0xc
[    0.008624]  [<d003f5ee>] spin_dump+0x4a/0x78
[    0.008677]  [<d003f681>] do_raw_spin_lock+0x1d/0x68
[    0.008727]  [<d06f66b9>] _raw_spin_lock_irqsave+0x41/0x54
[    0.008780]  [<d04efe6a>] univ8250_console_setup+0x56/0x104
[    0.008829]  [<d004368d>] try_enable_preferred_console+0x75/0xf4
[    0.008877]  [<d0044a84>] register_console+0xf4/0x3e4
[    0.008923]  [<d0ae6710>] univ8250_console_init+0x20/0x24
[    0.008970]  [<d0ad44f0>] console_init+0x18/0x20
[    0.009018]  [<d0acfdaf>] start_kernel+0x3db/0x4cc
[    0.009063]  [<d06ee02b>] _startup+0x13b/0x13b

That was xtensa. Others are just silent.

Guenter