Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18

[Jarkko Sakkinen <jarkko@kernel.org>]

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@kernel.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.

BR, Jarkko