Message-ID: <aOPOZwp_inGui9Bx@kernel.org>
Date: Mon, 6 Oct 2025 17:12:55 +0300
From: Jarkko Sakkinen <jarkko@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Peter Huewe <peterhuewe@....de>, Jason Gunthorpe <jgg@...pe.ca>,
	David Howells <dhowells@...hat.com>, keyrings@...r.kernel.org,
	linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18

On Mon, Oct 06, 2025 at 02:58:17PM +0300, Jarkko Sakkinen wrote:
> On Sun, Oct 05, 2025 at 11:09:08AM -0700, Linus Torvalds wrote:
> > On Sun, 5 Oct 2025 at 08:47, Jarkko Sakkinen <jarkko@...nel.org> wrote:
> > >
> > >      This pull request disables
> > > TCG_TPM2_HMAC from the default configuration as it does not perform well
> > > enough [1].
> > >
> > > [1] https://lore.kernel.org/linux-integrity/20250825203223.629515-1-jarkko@kernel.org/
> > 
> > This link is entirely useless, and doesn't explain what the problem
> > was and *why* TPM2_TCG_HMAC shouldn't be on by default.
> > 
> > I think a much better link is
> > 
> >    https://lore.kernel.org/linux-integrity/20250814162252.3504279-1-cfenn@google.com/
> > 
> > which talks about the problems that TPM2_TCG_HMAC causes.
> > 
> > Which weren't just about "not performing well enough", but actually
> > about how it breaks TPM entirely for some cases.
> 
> Fair enough. I'll also enumerate the issues, and also roadmap
> to heal the feature.

So some of the arguments in Chris' email are debatable, such as
this list:

- TPM_RH_NULL
- TPM2_CreatePrimary
- TPM2_ContextSave
- ECDH-P256
- AES-128-CFB


We've never encountered a TPM chip without those TPM commands, and e.g.
/dev/tpmrm0 heavily relies on TPM2_ContextSave, and has been in the
mainline since 2017. And further, this has been the case on ARM too.

So using all of the arguments as rationale for the change that according
to Chris' email are broken because I cannnot objectively on all of the
arguments.

E.g. I have to assume to this day that all known TPM chips have those
commands because no smoking gun exists. And if DID exist, then I'd
assume someone would fixed /dev/tpmrm0 ages ago.

That said, I do agree on disabling the feature for the time being:
we have consensus on actions but not really on stimulus tbh.
And if there is stimulus I would postpone this patch to create
fix also for /dev/tpmrm0.

Argument where I meet with Chris suggestion are:

1. Performance. The key generation during boot is extremely bad
   idea and depending on the deployment the encryption cost is
   too much (e.g. with my laptop having fTPM it does not really
   matter).
2. Null seed was extremely bad idea. The way I'm planning to actually
   fix this is to parametrize the primary key to a persistent key handle
   stored into nvram of the chip instead of genration. This will address
   also ambiguity and can be linked directly to vendor ceritifcate
   for e.g. to perfom remote attesttion.

Things don't go broken by saying that they are broken and nothing
elsewhere in the mainline has supporting evidence that those commands
would be optional. I cannot agree on argument which I have zero
means to measure in any possible way.

This is exactly also the root reason why I wrote my own commit instead
with the same change: I could have never signed off the commit that
I don't believe is true in its storyline.

So if I write cover for the pull request where I use the subset of
arguments with shared consensus would that be enough to get this
through? As for primary key handle fix I rather do that with
time and proper care.

BR, Jarkko

Powered by blists - more mailing lists