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

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

[Jarkko Sakkinen]

The following changes since commit 6093a688a07da07808f0122f9aa2a3eed250d853:

  Merge tag 'char-misc-6.18-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc (2025-10-04 16:26:32 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git tags/tpmdd-next-v6.18

for you to fetch changes up to 642688ae78cf7c6ede28db7884b09c0d242496cb:

  tpm: Require O_EXCL for exclusive /dev/tpm access (2025-10-05 18:36:48 +0300)

----------------------------------------------------------------
Hi,

and apologies for this late pull request. This pull request disables
TCG_TPM2_HMAC from the default configuration as it does not perform well
enough [1].

During the next release cycle I'll prepare gradual improvements to the
aforementioned feature. I delayed my PR because I wanted to consider
whether to include some of them already for 6.18. The end resolution
is that it is better to fully address the feature during the release
cycle and postpone improvements to 6.19.

BR, Jarkko

[1] https://lore.kernel.org/linux-integrity/20250825203223.629515-1-jarkko@kernel.org/

----------------------------------------------------------------
Denis Aleksandrov (1):
      tpm: Prevent local DOS via tpm/tpm0/ppi/*operations

Eric Biggers (2):
      tpm: Compare HMAC values in constant time
      tpm: Use HMAC-SHA256 library instead of open-coded HMAC

Gunnar Kudrjavets (1):
      tpm_tis: Fix incorrect arguments in tpm_tis_probe_irq_single

Jarkko Sakkinen (2):
      tpm: Disable TPM2_TCG_HMAC by default
      tpm: use a map for tpm2_calc_ordinal_duration()

Jonathan McDowell (4):
      tpm: Ensure exclusive userspace access when using /dev/tpm<n>
      tpm: Remove tpm_find_get_ops
      tpm: Allow for exclusive TPM access when using /dev/tpm<n>
      tpm: Require O_EXCL for exclusive /dev/tpm access

 drivers/char/tpm/Kconfig          |   3 +-
 drivers/char/tpm/tpm-chip.c       |  90 +++++++++++++--------------
 drivers/char/tpm/tpm-dev-common.c |   8 +--
 drivers/char/tpm/tpm-dev.c        |  27 ++++++--
 drivers/char/tpm/tpm-dev.h        |   1 +
 drivers/char/tpm/tpm-interface.c  |  22 +++++--
 drivers/char/tpm/tpm.h            |   5 +-
 drivers/char/tpm/tpm2-cmd.c       | 127 ++++++++++----------------------------
 drivers/char/tpm/tpm2-sessions.c  | 104 +++++++++----------------------
 drivers/char/tpm/tpm2-space.c     |   5 +-
 drivers/char/tpm/tpm_ppi.c        |  89 +++++++++++++++++++-------
 drivers/char/tpm/tpm_tis_core.c   |   7 +--
 drivers/char/tpm/tpmrm-dev.c      |  20 +++++-
 include/linux/tpm.h               |   8 ++-
 14 files changed, 249 insertions(+), 267 deletions(-)

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

[Linus Torvalds]

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.

              Linus

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

[Jarkko Sakkinen]

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.

> 
>               Linus

BR, Jarkko

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

[Jarkko Sakkinen]

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

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

[Jarkko Sakkinen]

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

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

[Jarkko Sakkinen]

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

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 

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

[James Bottomley]

On Mon, 2025-10-06 at 17:12 +0300, Jarkko Sakkinen wrote:
> 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.

Just a minute, there's been no discussion or debate about this on the
list.  The rationale for using the NULL seed is clearly laid out here:

https://docs.kernel.org/security/tpm/tpm-security.html

But in brief it is the only way to detect reset attacks against the TPM
and a reset attack is the single simplest attack an interposer can do.

If you think there's a problem with the approach, by all means let's
have a debate, since TPM security is always a trade off, but you can't
simply come to your own opinion and try to impose it by fiat without at
least raising whatever issue you think you've found with the parties
who contributed the code in the first place.

Regards,

James

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

[Jarkko Sakkinen]

On Mon, Oct 06, 2025 at 10:33:40AM -0400, James Bottomley wrote:
> On Mon, 2025-10-06 at 17:12 +0300, Jarkko Sakkinen wrote:
> > 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.
> 
> Just a minute, there's been no discussion or debate about this on the
> list.  The rationale for using the NULL seed is clearly laid out here:
> 
> https://docs.kernel.org/security/tpm/tpm-security.html
> 
> But in brief it is the only way to detect reset attacks against the TPM
> and a reset attack is the single simplest attack an interposer can do.
> 
> If you think there's a problem with the approach, by all means let's
> have a debate, since TPM security is always a trade off, but you can't
> simply come to your own opinion and try to impose it by fiat without at
> least raising whatever issue you think you've found with the parties
> who contributed the code in the first place.

Ok fair enough, it's quite context dependent what is not secure and
what is secure.

What I've thought, or have planned to implement, is not to discard null
seed but instead parmetrize the primary key as a kernel command-line
parameter.

E.g. "tpm.integrity_key={off,null,handle}" and
"tpm.integrity_key_handle" to specify an NV index. The default value is
off and I think also that with this change and possibly with some
additional polishing it can reappear in default config,

This out of context for the PR but I will take your comment into account
in the pull request.

My main issue preventing sending a new pull request is that weird list
of core TPM2 features that is claimed "not to be required" with zero
references. Especially it is contraditory claim that TPM2_CreatePrimary
would be optional feature as the whole chip standard is based on three
random seeds from which primary keys are templated and used as root
keys for other keys.

So I guess I cherry-pick the claims from Chris' patch that I can cope
with, look what I wrote to my commit and adjust that accordingly and
finally write a tag message with summarization of all this. I exactly
drop the arguments with no quantitative evidence, which is probably
a sane way to move forward.

> 
> Regards,
> 
> James

BR, Jarkko

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

[Jarkko Sakkinen]

On Mon, Oct 06, 2025 at 07:51:51PM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 06, 2025 at 10:33:40AM -0400, James Bottomley wrote:
> > On Mon, 2025-10-06 at 17:12 +0300, Jarkko Sakkinen wrote:
> > > 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.
> > 
> > Just a minute, there's been no discussion or debate about this on the
> > list.  The rationale for using the NULL seed is clearly laid out here:
> > 
> > https://docs.kernel.org/security/tpm/tpm-security.html
> > 
> > But in brief it is the only way to detect reset attacks against the TPM
> > and a reset attack is the single simplest attack an interposer can do.
> > 
> > If you think there's a problem with the approach, by all means let's
> > have a debate, since TPM security is always a trade off, but you can't
> > simply come to your own opinion and try to impose it by fiat without at
> > least raising whatever issue you think you've found with the parties
> > who contributed the code in the first place.
> 
> Ok fair enough, it's quite context dependent what is not secure and
> what is secure.
> 
> What I've thought, or have planned to implement, is not to discard null
> seed but instead parmetrize the primary key as a kernel command-line
> parameter.
> 
> E.g. "tpm.integrity_key={off,null,handle}" and
> "tpm.integrity_key_handle" to specify an NV index. The default value is
> off and I think also that with this change and possibly with some
> additional polishing it can reappear in default config,
> 
> This out of context for the PR but I will take your comment into account
> in the pull request.
> 
> My main issue preventing sending a new pull request is that weird list
> of core TPM2 features that is claimed "not to be required" with zero
> references. Especially it is contraditory claim that TPM2_CreatePrimary
> would be optional feature as the whole chip standard is based on three
> random seeds from which primary keys are templated and used as root
> keys for other keys.
> 
> So I guess I cherry-pick the claims from Chris' patch that I can cope
> with, look what I wrote to my commit and adjust that accordingly and
> finally write a tag message with summarization of all this. I exactly
> drop the arguments with no quantitative evidence, which is probably
> a sane way to move forward.

Personally I think that once there's correctly implemented command-line
option, the feature flag is somewhat redundant (and we've never had one
for /dev/tpmrm0). And it will help a lot with kernel QA as you can run
tests with same kernel image without recompilation.

BR, Jarkko

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

[Jarkko Sakkinen]

On Mon, Oct 06, 2025 at 07:57:10PM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 06, 2025 at 07:51:51PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Oct 06, 2025 at 10:33:40AM -0400, James Bottomley wrote:
> > > On Mon, 2025-10-06 at 17:12 +0300, Jarkko Sakkinen wrote:
> > > > 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.
> > > 
> > > Just a minute, there's been no discussion or debate about this on the
> > > list.  The rationale for using the NULL seed is clearly laid out here:
> > > 
> > > https://docs.kernel.org/security/tpm/tpm-security.html
> > > 
> > > But in brief it is the only way to detect reset attacks against the TPM
> > > and a reset attack is the single simplest attack an interposer can do.
> > > 
> > > If you think there's a problem with the approach, by all means let's
> > > have a debate, since TPM security is always a trade off, but you can't
> > > simply come to your own opinion and try to impose it by fiat without at
> > > least raising whatever issue you think you've found with the parties
> > > who contributed the code in the first place.
> > 
> > Ok fair enough, it's quite context dependent what is not secure and
> > what is secure.
> > 
> > What I've thought, or have planned to implement, is not to discard null
> > seed but instead parmetrize the primary key as a kernel command-line
> > parameter.
> > 
> > E.g. "tpm.integrity_key={off,null,handle}" and
> > "tpm.integrity_key_handle" to specify an NV index. The default value is
> > off and I think also that with this change and possibly with some
> > additional polishing it can reappear in default config,
> > 
> > This out of context for the PR but I will take your comment into account
> > in the pull request.
> > 
> > My main issue preventing sending a new pull request is that weird list
> > of core TPM2 features that is claimed "not to be required" with zero
> > references. Especially it is contraditory claim that TPM2_CreatePrimary
> > would be optional feature as the whole chip standard is based on three
> > random seeds from which primary keys are templated and used as root
> > keys for other keys.
> > 
> > So I guess I cherry-pick the claims from Chris' patch that I can cope
> > with, look what I wrote to my commit and adjust that accordingly and
> > finally write a tag message with summarization of all this. I exactly
> > drop the arguments with no quantitative evidence, which is probably
> > a sane way to move forward.
> 
> Personally I think that once there's correctly implemented command-line
> option, the feature flag is somewhat redundant (and we've never had one
> for /dev/tpmrm0). And it will help a lot with kernel QA as you can run
> tests with same kernel image without recompilation.

I don't really see any possible security issues either with null seed.

It's no different as per remote attestation when compared storage keys.
In a power cycle it's like same as per TPM2_Certify is considered. It's
pretty much exactly performance issues but depending on deployment.
Sometimes storage key root would be probably a better choice.

I really tried to dig something else than exactly perf stuff but was
unsuccessful to find anything, and I've actually done a lot of work
at work on remote attestation so everything is also somewhat fresh
on my mind.

Still rooting to perf, immediate action being default option disable,
and long term action being replacing the compilation option with
kernel command-line options. I.e., I'll stay on track what I'e
been already doing for some time :-)

That said, my PR cover letter (or the tag message) did suck and
I'll just address next during exactly why something is or isn't
an issue. I think this is really good outcome for everyone in
the long run (because everyone will get the outcome they were
looking for).

BR, Jarkko

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

[Jarkko Sakkinen]

On Tue, Oct 07, 2025 at 05:32:24PM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 06, 2025 at 07:57:10PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Oct 06, 2025 at 07:51:51PM +0300, Jarkko Sakkinen wrote:
> > > On Mon, Oct 06, 2025 at 10:33:40AM -0400, James Bottomley wrote:
> > > > On Mon, 2025-10-06 at 17:12 +0300, Jarkko Sakkinen wrote:
> > > > > 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.
> > > > 
> > > > Just a minute, there's been no discussion or debate about this on the
> > > > list.  The rationale for using the NULL seed is clearly laid out here:
> > > > 
> > > > https://docs.kernel.org/security/tpm/tpm-security.html
> > > > 
> > > > But in brief it is the only way to detect reset attacks against the TPM
> > > > and a reset attack is the single simplest attack an interposer can do.
> > > > 
> > > > If you think there's a problem with the approach, by all means let's
> > > > have a debate, since TPM security is always a trade off, but you can't
> > > > simply come to your own opinion and try to impose it by fiat without at
> > > > least raising whatever issue you think you've found with the parties
> > > > who contributed the code in the first place.
> > > 
> > > Ok fair enough, it's quite context dependent what is not secure and
> > > what is secure.
> > > 
> > > What I've thought, or have planned to implement, is not to discard null
> > > seed but instead parmetrize the primary key as a kernel command-line
> > > parameter.
> > > 
> > > E.g. "tpm.integrity_key={off,null,handle}" and
> > > "tpm.integrity_key_handle" to specify an NV index. The default value is
> > > off and I think also that with this change and possibly with some
> > > additional polishing it can reappear in default config,
> > > 
> > > This out of context for the PR but I will take your comment into account
> > > in the pull request.
> > > 
> > > My main issue preventing sending a new pull request is that weird list
> > > of core TPM2 features that is claimed "not to be required" with zero
> > > references. Especially it is contraditory claim that TPM2_CreatePrimary
> > > would be optional feature as the whole chip standard is based on three
> > > random seeds from which primary keys are templated and used as root
> > > keys for other keys.
> > > 
> > > So I guess I cherry-pick the claims from Chris' patch that I can cope
> > > with, look what I wrote to my commit and adjust that accordingly and
> > > finally write a tag message with summarization of all this. I exactly
> > > drop the arguments with no quantitative evidence, which is probably
> > > a sane way to move forward.
> > 
> > Personally I think that once there's correctly implemented command-line
> > option, the feature flag is somewhat redundant (and we've never had one
> > for /dev/tpmrm0). And it will help a lot with kernel QA as you can run
> > tests with same kernel image without recompilation.
> 
> I don't really see any possible security issues either with null seed.
> 
> It's no different as per remote attestation when compared storage keys.
> In a power cycle it's like same as per TPM2_Certify is considered. It's
> pretty much exactly performance issues but depending on deployment.
> Sometimes storage key root would be probably a better choice.
> 
> I really tried to dig something else than exactly perf stuff but was
> unsuccessful to find anything, and I've actually done a lot of work
> at work on remote attestation so everything is also somewhat fresh
> on my mind.
> 
> Still rooting to perf, immediate action being default option disable,
> and long term action being replacing the compilation option with
> kernel command-line options. I.e., I'll stay on track what I'e
> been already doing for some time :-)
> 
> That said, my PR cover letter (or the tag message) did suck and
> I'll just address next during exactly why something is or isn't
> an issue. I think this is really good outcome for everyone in
> the long run (because everyone will get the outcome they were
> looking for).

And in the business I'm ATM i.e., covering like with 20-30% market share
Finnish high school IT deplyoment going extremes is feasible, as some
kids are smart and capable of hacking the systems so to speak ;-) I'd
likely enable this feature e.g., in that type of deployment together
with remote attesation just to max out defence in depth.

BR, Jarkko

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

[Jonathan McDowell]

On Mon, Oct 06, 2025 at 07:51:46PM +0300, Jarkko Sakkinen wrote:
> My main issue preventing sending a new pull request is that weird list
> of core TPM2 features that is claimed "not to be required" with zero
> references. Especially it is contraditory claim that TPM2_CreatePrimary
> would be optional feature as the whole chip standard is based on three
> random seeds from which primary keys are templated and used as root
> keys for other keys.

My understanding here is that the main specification about what's 
"required" for TPMs to implement is from the PC Client Platform TPM 
Profile. There's no specific server PTP (though there is talk about 
creating one), so _most_ vendors just implement the PC Client PTP. It 
doesn't mean a TPM that doesn't do so isn't TPM compliant, just not PC 
Client PTP compliant.

Google have taken the approach in their Titan based TPM implementation 
to avoid implementing features they don't need, to reduce attack 
surface.

I'm not aware of anyone else who has done this.

J.

-- 
... You non-conformists are all alike.

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

[Jarkko Sakkinen]

On Mon, Oct 06, 2025 at 06:02:05PM +0100, Jonathan McDowell wrote:
> On Mon, Oct 06, 2025 at 07:51:46PM +0300, Jarkko Sakkinen wrote:
> > My main issue preventing sending a new pull request is that weird list
> > of core TPM2 features that is claimed "not to be required" with zero
> > references. Especially it is contraditory claim that TPM2_CreatePrimary
> > would be optional feature as the whole chip standard is based on three
> > random seeds from which primary keys are templated and used as root
> > keys for other keys.
> 
> My understanding here is that the main specification about what's 
> "required" for TPMs to implement is from the PC Client Platform TPM 
> Profile. There's no specific server PTP (though there is talk about 
> creating one), so _most_ vendors just implement the PC Client PTP. It 
> doesn't mean a TPM that doesn't do so isn't TPM compliant, just not PC 
> Client PTP compliant.
> 
> Google have taken the approach in their Titan based TPM implementation 
> to avoid implementing features they don't need, to reduce attack 
> surface.

If it is an internal product, it does not qualify for as an argument but
good that you brought this detail out. I mean the action is the same
and debate is really what are correct preconditions for taking any
action.

So I'll cover the next trial for PR with:

1. I'll retain my existing commit with no changes.
2. In the cover letter I can address the details brough by Chris with
   the clause that Titan specific arguments are not basis for any
   decision making, as it is wrong and does not scale given that
   any possible company in this planet can have their own random
   incompatible partial TPM implementation (and most likely there
   are at least few of such).
3. I'll link Chris' message to the email. I.e. based on other
   claims in that mail we can already fully justify the action
   itself. I'm just knowingly ignoring that list of "incompatible"
   features, and remark that so that the process is transparent.

> 
> I'm not aware of anyone else who has done this.
> 
> J.
> 
> -- 
> ... You non-conformists are all alike.

BR, Jarkko

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

[Linus Torvalds]

On Sun, 5 Oct 2025 at 08:47, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> and apologies for this late pull request. This pull request disables
> TCG_TPM2_HMAC from the default configuration as it does not perform well
> enough

So having looked more at this, not only does it disable that
TCG_TPM2_HMAC, it does a lot of other things too.

I really am going to require a better pull request, and I have thrown
this one away.

The exclusive access looks debatable to me too. I think you should
also require that the open was done not only with O_EXCL, but as a
write too.

Exclusive reads do not make sense.

Now, I certainly *hope* that nobody has /dev/tmp being world-readable,
so it probably doesn't matter, but that new exclusive access thing is
very different than what the code used to do, and if I read it
correctly it will also disable the kernel doing certain operations. So
it needs to be as limited as possible.

And damn it, it needs to be *explained*. Not have a pull request where
one single line is explained badly.

             Linus

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

[Jarkko Sakkinen]

On Sun, Oct 05, 2025 at 11:24:09AM -0700, Linus Torvalds wrote:
> On Sun, 5 Oct 2025 at 08:47, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > and apologies for this late pull request. This pull request disables
> > TCG_TPM2_HMAC from the default configuration as it does not perform well
> > enough
> 
> So having looked more at this, not only does it disable that
> TCG_TPM2_HMAC, it does a lot of other things too.
> 
> I really am going to require a better pull request, and I have thrown
> this one away.

What I think I think it is good call.

> 
> The exclusive access looks debatable to me too. I think you should
> also require that the open was done not only with O_EXCL, but as a
> write too.
> 
> Exclusive reads do not make sense.

True, I agree with this.

After reading this email I realized also another issue with these patch
when I tested them sequentially building a VM for each commit ID.

Without "tpm: Require O_EXCL for exclusive /dev/tpm access" applied,
there's a regression: usually a daemon of some sort opens /dev/tpm0:

COMMAND      PID USER   FD   TYPE DEVICE SIZE/OFF NODE NAME
tpm2-abrm 771444  tss    5u   CHR 10,224      0t0   94 /dev/tpm0

Without top patch this leaves /dev/tpmrm0 unusuable, which is a huge
developer experience downgrade as it is nice and covenient device
to try and do things. I.e. tail patch needs to be squashed and
the whole patch set needs to be re-reviewed.

I don't know systemd source code too well, but after eading
documentation of systemd-cryptenroll, it can also use /dev/tpmrm0:

https://www.freedesktop.org/software/systemd/man/latest/systemd-cryptenroll.html

Based on this it's now like there's a breaking patch and the top
most patch fixes the bug.

And based on this I'm happy to postpone O_EXCL changes to 6.19.
Patch set just needs to be restructured better so that in-the
middle of the series patches don't break things. And also it'd
be better if this patch would be relocated as the first in the
series: "tpm: Remove tpm_find_get_ops".


> 
> Now, I certainly *hope* that nobody has /dev/tmp being world-readable,
> so it probably doesn't matter, but that new exclusive access thing is
> very different than what the code used to do, and if I read it
> correctly it will also disable the kernel doing certain operations. So
> it needs to be as limited as possible.

Not disagreeing with this.

> 
> And damn it, it needs to be *explained*. Not have a pull request where
> one single line is explained badly.

'll send tomorrow a new PR without O_EXCL patches.

As per hmac encryption, I think my decision was the right call
but cover letter did suck agreed (sorry about that).

> 
>              Linus

BR, Jarkko

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

[Jonathan McDowell]

On Mon, Oct 06, 2025 at 03:33:56PM +0300, Jarkko Sakkinen wrote:
> On Sun, Oct 05, 2025 at 11:24:09AM -0700, Linus Torvalds wrote:
> > On Sun, 5 Oct 2025 at 08:47, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > The exclusive access looks debatable to me too. I think you should
> > also require that the open was done not only with O_EXCL, but as a
> > write too.
> > 
> > Exclusive reads do not make sense.
> 
> True, I agree with this.

I'm not sure _reads_ make sense for the TPM device files at all. It's a 
command + response interface.

What should we do if we get O_EXCL and O_RDONLY? Return an error? Ignore 
the O_EXCL flag?

> After reading this email I realized also another issue with these patch
> when I tested them sequentially building a VM for each commit ID.
> 
> Without "tpm: Require O_EXCL for exclusive /dev/tpm access" applied,
> there's a regression: usually a daemon of some sort opens /dev/tpm0:
> 
> COMMAND      PID USER   FD   TYPE DEVICE SIZE/OFF NODE NAME
> tpm2-abrm 771444  tss    5u   CHR 10,224      0t0   94 /dev/tpm0
> 
> Without top patch this leaves /dev/tpmrm0 unusuable, which is a huge
> developer experience downgrade as it is nice and covenient device
> to try and do things. I.e. tail patch needs to be squashed and
> the whole patch set needs to be re-reviewed.

That's a fair point; I structured the patches in that fashion because I 
felt the O_EXCL patch was potentially contentious and might not be 
accepted.

> And based on this I'm happy to postpone O_EXCL changes to 6.19.
> Patch set just needs to be restructured better so that in-the
> middle of the series patches don't break things. And also it'd
> be better if this patch would be relocated as the first in the
> series: "tpm: Remove tpm_find_get_ops".

I'll spin a set with the tpm_find_get_ops removal first, then the O_EXCL 
patch, then the other two, which I think fixes all the ordering 
concerns.

J.

-- 
... Nice world. Let's make it weirder.

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

[Linus Torvalds]

On Mon, 6 Oct 2025 at 14:40, Jonathan McDowell <noodles@earth.li> wrote:
>
> I'm not sure _reads_ make sense for the TPM device files at all. It's a
> command + response interface.

Yeah, I won't disagree. My main worry was one of "existing users doing
odd things shouldn't then get new semantics"

I don't actually _expect_ people to do odd things wrt /dev/tpm and
friends, but I've so often been surprised by the odd things that do
happen that...

> What should we do if we get O_EXCL and O_RDONLY? Return an error? Ignore
> the O_EXCL flag?

Either of those likely is perfectly fine. Just ignoring it is the one
that adds the smallest possible ABI change, so in that sense it's the
"safer" option.

But you are probably right that there aren't any users that open that
thing read-only in the first place, much less then using O_EXCL.

Except I've seen programs that "work" by virtue of having done
entirely the wrong thing that just happened to work. Things like using
the wrong values entirely for 'flags' to open(), but the bits we cared
about happened to be ok, so...

And we have *traditionally* very much ignored O_EXCL when not paired
with O_CREAT (and even then it's iffy with NFS).

So people might just have random crud in the flags. That's why I
reacted originally: O_EXCL does make sense as a "open exclusively"
based on the name, but realistically we never actually *use* it that
way except for block devices.

So we *have* had those semantics for block devices, and they make
sense, and I don't mind the change at all per se.

But this is the kind of thing that I react to just because we have had
unexpected behavior in the past when a bit goes from "don't care" to
"meaningful".

Put another way: I'm probably just being unreasonably worried.

                     Linus

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

[James Bottomley]

On Mon, 2025-10-06 at 22:40 +0100, Jonathan McDowell wrote:
> What should we do if we get O_EXCL and O_RDONLY? Return an error?
> Ignore the O_EXCL flag?

As a command/response interface, it can't be operated read only ... so
I think we should just return -EINVAL (or something more appropriate)
if someone tries to open it O_RDONLY regardless of O_EXCL.

Regards,

James