public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Huewe <peterhuewe@gmx.de>, Jason Gunthorpe <jgg@ziepe.ca>,
	David Howells <dhowells@redhat.com>,
	keyrings@vger.kernel.org, linux-integrity@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18
Date: Mon, 6 Oct 2025 17:12:55 +0300	[thread overview]
Message-ID: <aOPOZwp_inGui9Bx@kernel.org> (raw)
In-Reply-To: <aOOu1f1QWQNtkl6c@kernel.org>

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.

BR, Jarkko

  reply	other threads:[~2025-10-06 14:12 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-05 15:47 [GIT PULL] TPM DEVICE DRIVER: tpmdd-next-v6.18 Jarkko Sakkinen
2025-10-05 18:09 ` Linus Torvalds
2025-10-06 11:58   ` Jarkko Sakkinen
2025-10-06 14:12     ` Jarkko Sakkinen [this message]
2025-10-06 14:18       ` Jarkko Sakkinen
2025-10-06 14:30         ` Jarkko Sakkinen
2025-10-06 14:33       ` James Bottomley
2025-10-06 16:51         ` Jarkko Sakkinen
2025-10-06 16:57           ` Jarkko Sakkinen
2025-10-07 14:32             ` Jarkko Sakkinen
2025-10-07 14:38               ` Jarkko Sakkinen
2025-10-06 17:02           ` Jonathan McDowell
2025-10-06 18:50             ` Jarkko Sakkinen
2025-10-05 18:24 ` Linus Torvalds
2025-10-06 12:33   ` Jarkko Sakkinen
2025-10-06 21:40     ` Jonathan McDowell
2025-10-06 22:09       ` Linus Torvalds
2025-10-06 23:11       ` James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aOPOZwp_inGui9Bx@kernel.org \
    --to=jarkko@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=jgg@ziepe.ca \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterhuewe@gmx.de \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox