linux-kernel - Re: [RFC v2 04/39] char: impi, tpm: depend on 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: <07c39877d9e940a96be41e21e22fe45dbb73d949.camel@linux.ibm.com>
Date:   Fri, 29 Apr 2022 16:23:33 +0200
From:   Niklas Schnelle <schnelle@...ux.ibm.com>
To:     Arnd Bergmann <arnd@...db.de>,
        Ahmad Fatoum <a.fatoum@...gutronix.de>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
        linux-pci@...r.kernel.org, Arnd Bergmann <arnd@...nel.org>,
        Corey Minyard <minyard@....org>,
        Peter Huewe <peterhuewe@....de>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        "moderated list:IPMI SUBSYSTEM" 
        <openipmi-developer@...ts.sourceforge.net>,
        "open list:TPM DEVICE DRIVER" <linux-integrity@...r.kernel.org>
Subject: Re: [RFC v2 04/39] char: impi, tpm: depend on HAS_IOPORT

> Hello Niklas,
> 
> On 29.04.22 15:50, Niklas Schnelle wrote:
> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > not being declared. We thus need to add this dependency and ifdef
> > sections of code using inb()/outb() as alternative access methods.
> > 
> > Co-developed-by: Arnd Bergmann <arnd@...nel.org>
> > Signed-off-by: Niklas Schnelle <schnelle@...ux.ibm.com>
> 
> [snip]
> 
> > diff --git a/drivers/char/tpm/tpm_infineon.c b/drivers/char/tpm/tpm_infineon.c
> > index 9c924a1440a9..2d2ae37153ba 100644
> > --- a/drivers/char/tpm/tpm_infineon.c
> > +++ b/drivers/char/tpm/tpm_infineon.c
> > @@ -51,34 +51,40 @@ static struct tpm_inf_dev tpm_dev;
> >  
> >  static inline void tpm_data_out(unsigned char data, unsigned char offset)
> >  {
> > +#ifdef CONFIG_HAS_IOPORT
> >       if (tpm_dev.iotype == TPM_INF_IO_PORT)
> >               outb(data, tpm_dev.data_regs + offset);
> >       else
> > +#endif
> 
> This looks ugly. Can't you declare inb/outb anyway and skip the definition,
> so you can use IS_ENABLED() here instead?
> 
> You can mark the declarations with __compiletime_error("some message"), so
> if an IS_ENABLED() reference is not removed at compile time, you get some
> readable error message instead of a link error.
> 
> Cheers,
> Ahmad

I didn't know about __compiletime_error() that certainly sounds
interesting even when using a normal #ifdef.

That said either with the function not being declared or this
__compiletime_error() mechanism I would think that using IS_ENABLED()
relies on compiler optimizations not to compile in the missing/error
function call, right? I'm not sure if that is something we should do.