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: <Z0Sa5nnKIQm7h-CA@smile.fi.intel.com>
Date: Mon, 25 Nov 2024 17:42:30 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Arnd Bergmann <arnd@...nel.org>
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 02:50:56PM +0100, Arnd Bergmann wrote:
> 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[]. 

A-ha, a good catch. I haven't checked the actual contents of the
old_serial_port for those configurations.

> 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.

It's nice idea, but yes, we can think about it later.

> >> 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.

Me neither. And the issue with NULL pointer dereference needs to be retested.

-- 
With Best Regards,
Andy Shevchenko