linux-kernel - Re: [PATCH v2 2/3] tpm: Address !chip->auth in tpm_buf_append

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: <91ccd10c3098782d540a3e9f5c70c5034f867928.camel@HansenPartnership.com>
Date: Thu, 04 Jul 2024 13:21:18 -0400
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Jarkko Sakkinen <jarkko@...nel.org>, linux-integrity@...r.kernel.org, 
 Thorsten Leemhuis <regressions@...mhuis.info>, stable@...r.kernel.org,
 Stefan Berger <stefanb@...ux.ibm.com>, Peter Huewe <peterhuewe@....de>,
 Jason Gunthorpe <jgg@...pe.ca>, Mimi Zohar <zohar@...ux.ibm.com>,  David
 Howells <dhowells@...hat.com>, Paul Moore <paul@...l-moore.com>, James
 Morris <jmorris@...ei.org>,  "Serge E. Hallyn" <serge@...lyn.com>, Ard
 Biesheuvel <ardb@...nel.org>, Mario Limonciello
 <mario.limonciello@....com>, linux-kernel@...r.kernel.org, 
 keyrings@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH v2 2/3] tpm: Address !chip->auth in tpm_buf_append_name()

On Thu, 2024-07-04 at 10:07 -0700, Linus Torvalds wrote:
> On Wed, 3 Jul 2024 at 13:11, James Bottomley
> <James.Bottomley@...senpartnership.com> wrote:
> > 
> > if (__and(IS_ENABLED(CONFIG_TCG_TPM2_HMAC), chip->auth))
> 
> Augh. Please don't do this.
> 
> That "__and()" thing may work, but it's entirely accidental that it
> does.
> 
> It's designed for config options _only_, and the fact that it then
> happens to work for "first argument is config option, second argument
> is C conditional".
> 
> The comment says that it's implementing "&&" using preprocessor
> expansion only, but it's a *really* limited form of it. The arguments
> are *not* arbitrary.
> 
> So no. Don't do this.
> 
> Just create a helper inline like
> 
>     static inline struct tpm2_auth *chip_auth(struct tpm_chip *chip)
>     {
>     #ifdef CONFIG_TCG_TPM2_HMAC
>         return chip->auth;
>     #else
>         return NULL;
>     #endif
>     }
> 
> and if we really want to have some kind of automatic way of doing
> this, we will *NOT* be using __and(), we'd do something like
> 
>         /* Return zero or 'value' depending on whether OPTION is
> enabled or not */
>         #define IF_ENABLED(option, value) __and(IS_ENABLED(option),
> value)
> 
> that actually would be documented and meaningful.
> 
> Not this internal random __and() implementation that is purely a
> kconfig.h helper macro and SHOULD NOT be used anywhere else.

I actually like the latter version, but instinct tells me that if this
is the first time the kernel has ever needed something like this then
perhaps we should go with the former because that's how everyone must
have handled it in the past.

James