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.2410041724450.45128@angie.orcam.me.uk>
Date: Fri, 4 Oct 2024 17:34:41 +0100 (BST)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: Niklas Schnelle <schnelle@...ux.ibm.com>
cc: Arnd Bergmann <arnd@...nel.org>, 
    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 Fri, 4 Oct 2024, Niklas Schnelle wrote:

> >  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.
> 
> This is already done in the final patch of my series when disabling
> inb()/outb() and friends.

 Good!

> >  I agree.  Enthusiastically.
> 
> I think there was also a bit of a misunderstanding. My argument that
> this would be very ugly in the general case was really meant as general
> case outside of drivers like 8250 that deals with both I/O port and
> MMIO i.e. we can't warn/error when !HAS_IOPORT deactivates a whole
> driver because seeing an I/O port BAR in common PCI code doesn't mean
> that it is required for use of the device.

 Absolutely.  Just seeing an I/O bar does not mean it's ever going to be 
used.  Even conventional PCI hardware is often dual-mapped and it's up to 
the driver to choose which mapping to use.

> I'm working on a new proposal for 8250 now. Basically I think we can
> put the warning/error in serial8250_pci_setup_port(). And then for
> those 8250_pci.c "sub drivers" that require I/O ports instead of just
> ifdeffing out their setup/init/exit we can define anything but setup to
> NULL and setup to pci_default_setup() such that the latter will find
> the I/O port BAR via FL_GET_BASE() and subsequently cause the error
> print in serial8250_pci_setup_port(). It's admittedly a bit odd but it
> also keeps the #ifdefs to just around the code that wouldn't compile.

 I'd rather you did what Arnd example patch does and just made original 
handlers bail out right away unless IS_ENABLED(CONFIG_HAS_IOPORT).  I do 
hope it will make no #ifdef necessary in 8250_pci.c at all.

  Maciej