Message-ID: <aOPSdstd1bk0pM3R@kernel.org>
Date: Mon, 6 Oct 2025 17:30:14 +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 05:18:13PM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 06, 2025 at 05:13:02PM +0300, Jarkko Sakkinen wrote:
> > 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.
> 
> I had to use few hours to remind why I did my commit instead of acking
> the original and this is the root. We've never had e.g. a bug in the
> wild that would /dev/tpmrm0 to be broken because ContextSave is not
> available, and it is *widely* used device across all major platforms.

Here's mobile client profile:

https://trustedcomputinggroup.org/wp-content/uploads/TPM_2.0_Mobile_Common_Profile_v2r31_FINAL.pdf

Unless I missed a tidbit I see nothing in it saying that ContextSave
would be optional. If there was even a known legit spec bringing some
context to the claims, that would move things forward.

Section 2.3 states this about ContextSave:

"The symmetric cipher mode TPM_ALG_CFB is REQUIRED by TCG TPM 2.0
Library specification Part 1 [1] and is also necessary for
implementation of TPM2_Create, TPM2_Load, TPM 2_ContextSave,
TPM2_ContextLoad, and other TPM commands"

which actually claims that TPM_ALG_CFB is required where as Chris'
patch claims 180 degrees opposite what the spec says.

Perhaps there's some other random TCG spec that I've missed, it's
entirely possible...

BR, Jarkko 

Powered by blists - more mailing lists