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: <alpine.DEB.2.21.2410022305040.45128@angie.orcam.me.uk>
Date: Wed, 2 Oct 2024 23:59:48 +0100 (BST)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: Arnd Bergmann <arnd@...nel.org>
cc: 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
Subject: Re: [PATCH 1/1] tty: serial: handle HAS_IOPORT dependencies

On Wed, 2 Oct 2024, Arnd Bergmann wrote:

> I agree that this shouldn't be hard to finish. The IS_ENABLED()
> check is not that easy to do as I think we need to keep calling
> inb()/outb() outside of an #ifdef a compile-time error.

 Can we just provide dummy prototypes with `__attribute__((error("...")))' 
instead?  This will surely prevent us from having to bend backwards so as 
to make sure the compiler won't see any spurious references to these 
inexistent functions or macros.  We already have a `__compiletime_error()' 
macro for this purpose even.

> Part of the problem that Niklas is trying to solve with the
> CONFIG_HAS_IOPORT annotations is to prevent an invalid inb()/outb()
> from turning into a NULL pointer dereference as it currently does
> on architectures that have no way to support PIO but get the
> default implementation from asm-generic/io.h.

 It can be worse than that.  Part of my confusion with the defxx driver 
trying to do port I/O with my POWER9 system came from the mapping actually 
resulting in non-NULL invalid pointers, dereferencing which caused a flood 
of obscure messages produced to the system console by the system firmware:

LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010014
IPMI: dropping non severe PEL event
LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010014
IPMI: dropping non severe PEL event
LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010014
IPMI: dropping non severe PEL event
LPC[000]: Got SYNC no-response error. Error address reg: 0xd0010014
IPMI: dropping non severe PEL event
[...]

from which it was all but obvious that they were caused by an attempt to 
use PCI port I/O with a system lacking support for it.

> It's not clear if having a silently non-working driver or one
> that crashes makes it easier to debug for users. Having a clear
> warning message in the PCI probe code is probably the best
> we can hope for.

 I agree.  Enthusiastically.

> I think that anyone using hardware that relies on port I/O on
> non-x86 is at this point going to have a reasonable understanding
> of the system, so I'm not too worried here. ;-)

 Well, virtually all non-x86 systems continue supporting PCI/e port I/O 
via a suitably decoded CPU-side MMIO window, so I think coming across one 
that doesn't can still be a nasty surprise even in 2024.  For instance 
I've been happily using a PC parallel port PCIe option card, one of the 
very few interfaces if not the only one remaining that have not ever seen 
an MMIO variant, with my RISC-V hardware, newer than said POWER9 system.

 So far it's been the s390 and a couple of POWER system implementations 
that have support for PCI/e port I/O removed.  Have I missed anything?

  Maciej