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: <1a7da799-f15b-4714-a3bd-4c0b1f48fc09@app.fastmail.com>
Date: Mon, 25 Nov 2024 14:50:56 +0100
From: "Arnd Bergmann" <arnd@...nel.org>
To: "Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>
Cc: "Guenter Roeck" <linux@...ck-us.net>,
 "Niklas Schnelle" <schnelle@...ux.ibm.com>,
 "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, "Yang Yingliang" <yangyingliang@...wei.com>
Subject: Re: [PATCH 1/1] tty: serial: handle HAS_IOPORT dependencies

On Mon, Nov 25, 2024, at 12:26, Andy Shevchenko wrote:
> On Mon, Nov 25, 2024 at 12:06:16PM +0100, Arnd Bergmann wrote:
>> What I suspect is going on with the f4c23a140d80 commit
>> is the same bug I mentioned earlier in this thread, where
>> __serial8250_isa_init_ports() just always registers
>> 'nr_uarts' (CONFIG_SERIAL_8250_RUNTIME_UARTS) ports,
>> unlike any other serial driver.
>
> But the configuration can give less than old_serial_port contains.
> See dozens of the explicit settings in the defconfigs.

I don't see any of the upstream defconfigs doing this
though, the only ones setting CONFIG_SERIAL_8250_RUNTIME_UARTS
are those that have an empty old_serial_port[]. 

Note that SERIAL_PORT_DFNS is only defined on x86, alpha
and m68k (for q40), which are the main PC-like platforms.
I see that all three have identical definitions of
SERIAL_PORT_DFNS, so I think these should just be moved
next to the __serial8250_isa_init_ports definition, with
the entire thing moved into a separate ISA driver or
an #ifdef around it. This is of course not the problem
at hand, but it would help separate the x86/isa and
non-x86 platform device cases further.

>> This used to be required before 9d86719f8769 ("serial:
>> 8250: Allow using ports higher than SERIAL_8250_RUNTIME_UARTS"),
>> but I don't see why this is still a thing now, other than
>> for using setserial on i486-class PCs with nonstandard ISA
>> ports.
>> 
>> On non-x86 machines, it only ever seems to create extra
>> ports that are likely to crash the system if opened, either
>> because they lack proper serial_in/serial_out callbacks,
>> or because the default UPIO_PORT callbacks end up poking
>> unmapped memory.
>> 
>> Do you see any reason why we can't just do the version below?
>
> Perhaps we may do this way (it seems better to me than previous 
> suggestions), but it also needs to be carefully checked against
> those configurations that set it explicitly.

Yes, at least to make sure that the numbering of the uarts
does not change. I expect it's actually the same, but don't
know for sure.

> If we go this way, it would be also nice to add a comment explaining that
> this is module parameter (as it's done for those ones above).
>
>> +unsigned int nr_uarts = ARRAY_SIZE(old_serial_port);;

Right.

     Arnd