I hit a problem where ~ 1% of TPM firmware upgrades were failing.
Investigating revealed the issue was that although the upgrade tool uses
/dev/tpm0 this does not actually prevent access via /dev/tpmrm0, nor
internal kernel users. It *does* prevent access to others via /dev/tpm0

So the upgrade process started, the HW RNG came in to get some
randomness in the middle, did the HMAC context dance, and confused
everything to the point the TPM was no longer visible to the OS even
after a reboot.

Thankfully I've been able to recover those devices, but really what I'd
like is the ability for a userspace tool to exclusively access the TPM
without something coming in behind it. Given the lightweight attempt at
locking that already exists I think this was the original intention.

I've reworked this series based on feedback received.

Firstly, it's been reordered TPM sharing functionality doesn't break
during bisection.

Secondly, the O_EXCL check has been tightend up to ensure the caller is
also opening the device O_RDWR. Callers shouldn't really be opening the
TPM except for reading + writing, but this should help guard against
unexpected flags usage a bit more.

Finally, this revision keeps the prohibition on more than one user of
/dev/tpm#, to avoid unexpected breakages for clients that expect this to
guard against multiple invocations. A client only then needs to use
O_EXCL if it wants to prevent *all* other access, even with
ContextSaves, such as the firmware upgrade case.

(I've sent a separate standalone patch that allows the TPM HW RNG to be
disabled at run time, and it's now in -next, but even with that I think
something like this is a good idea as well.)

Jonathan McDowell (4):
  tpm: Remove tpm_find_get_ops
  tpm: Add O_EXCL for exclusive /dev/tpm access
  tpm: Include /dev/tpmrm<n> when checking exclusive userspace TPM
    access
  tpm: Allow for exclusive TPM access when using /dev/tpm<n>

 drivers/char/tpm/tpm-chip.c       | 90 +++++++++++++++----------------
 drivers/char/tpm/tpm-dev-common.c |  8 +--
 drivers/char/tpm/tpm-dev.c        | 35 ++++++++++--
 drivers/char/tpm/tpm-dev.h        |  1 +
 drivers/char/tpm/tpm-interface.c  | 20 +++++--
 drivers/char/tpm/tpm.h            |  3 +-
 drivers/char/tpm/tpm2-space.c     |  5 +-
 drivers/char/tpm/tpm_tis_core.c   |  3 +-
 drivers/char/tpm/tpmrm-dev.c      | 20 ++++++-
 include/linux/tpm.h               |  4 +-
 10 files changed, 124 insertions(+), 65 deletions(-)

-- 
2.51.0

On Mon, Oct 20, 2025 at 12:30:32PM +0100, Jonathan McDowell wrote:
> I hit a problem where ~ 1% of TPM firmware upgrades were failing.
> Investigating revealed the issue was that although the upgrade tool uses
> /dev/tpm0 this does not actually prevent access via /dev/tpmrm0, nor
> internal kernel users. It *does* prevent access to others via /dev/tpm0
> 
> So the upgrade process started, the HW RNG came in to get some
> randomness in the middle, did the HMAC context dance, and confused
> everything to the point the TPM was no longer visible to the OS even
> after a reboot.
> 
> Thankfully I've been able to recover those devices, but really what I'd
> like is the ability for a userspace tool to exclusively access the TPM
> without something coming in behind it. Given the lightweight attempt at
> locking that already exists I think this was the original intention.
> 
> I've reworked this series based on feedback received.
> 
> Firstly, it's been reordered TPM sharing functionality doesn't break
> during bisection.
> 
> Secondly, the O_EXCL check has been tightend up to ensure the caller is
> also opening the device O_RDWR. Callers shouldn't really be opening the
> TPM except for reading + writing, but this should help guard against
> unexpected flags usage a bit more.
> 
> Finally, this revision keeps the prohibition on more than one user of
> /dev/tpm#, to avoid unexpected breakages for clients that expect this to
> guard against multiple invocations. A client only then needs to use
> O_EXCL if it wants to prevent *all* other access, even with
> ContextSaves, such as the firmware upgrade case.
> 
> (I've sent a separate standalone patch that allows the TPM HW RNG to be
> disabled at run time, and it's now in -next, but even with that I think
> something like this is a good idea as well.)
> 
> Jonathan McDowell (4):
>   tpm: Remove tpm_find_get_ops
>   tpm: Add O_EXCL for exclusive /dev/tpm access
>   tpm: Include /dev/tpmrm<n> when checking exclusive userspace TPM
>     access
>   tpm: Allow for exclusive TPM access when using /dev/tpm<n>
> 
>  drivers/char/tpm/tpm-chip.c       | 90 +++++++++++++++----------------
>  drivers/char/tpm/tpm-dev-common.c |  8 +--
>  drivers/char/tpm/tpm-dev.c        | 35 ++++++++++--
>  drivers/char/tpm/tpm-dev.h        |  1 +
>  drivers/char/tpm/tpm-interface.c  | 20 +++++--
>  drivers/char/tpm/tpm.h            |  3 +-
>  drivers/char/tpm/tpm2-space.c     |  5 +-
>  drivers/char/tpm/tpm_tis_core.c   |  3 +-
>  drivers/char/tpm/tpmrm-dev.c      | 20 ++++++-
>  include/linux/tpm.h               |  4 +-
>  10 files changed, 124 insertions(+), 65 deletions(-)
> 
> -- 
> 2.51.0
> 

I will put to queue with my tags but I just want to make first sure that we do not
break anything.

I'll upgrade my test suite first to have TPM 1.2 tests (which is also
needed for my own series) and run it in bunch of configurations.  And on
TPM2 I check the behavior with TSS2 daemon on / off.

I have no doubts on the code changes, and it is most importantly a
security improvement, given that "who has the access and how long" 
can be deduced for a system configuration. I just feel that with
this code change it is better to check and verify everything :-)

BR, Jarkko

On Fri, 2025-10-24 at 21:55 +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 20, 2025 at 12:30:32PM +0100, Jonathan McDowell wrote:
> > I hit a problem where ~ 1% of TPM firmware upgrades were failing.
> > Investigating revealed the issue was that although the upgrade tool uses
> > /dev/tpm0 this does not actually prevent access via /dev/tpmrm0, nor
> > internal kernel users. It *does* prevent access to others via /dev/tpm0
> > 
> > So the upgrade process started, the HW RNG came in to get some
> > randomness in the middle, did the HMAC context dance, and confused
> > everything to the point the TPM was no longer visible to the OS even
> > after a reboot.
> > 
> > Thankfully I've been able to recover those devices, but really what I'd
> > like is the ability for a userspace tool to exclusively access the TPM
> > without something coming in behind it. Given the lightweight attempt at
> > locking that already exists I think this was the original intention.
> > 
> > I've reworked this series based on feedback received.
> > 
> > Firstly, it's been reordered TPM sharing functionality doesn't break
> > during bisection.
> > 
> > Secondly, the O_EXCL check has been tightend up to ensure the caller is
> > also opening the device O_RDWR. Callers shouldn't really be opening the
> > TPM except for reading + writing, but this should help guard against
> > unexpected flags usage a bit more.
> > 
> > Finally, this revision keeps the prohibition on more than one user of
> > /dev/tpm#, to avoid unexpected breakages for clients that expect this to
> > guard against multiple invocations. A client only then needs to use
> > O_EXCL if it wants to prevent *all* other access, even with
> > ContextSaves, such as the firmware upgrade case.
> > 
> > (I've sent a separate standalone patch that allows the TPM HW RNG to be
> > disabled at run time, and it's now in -next, but even with that I think
> > something like this is a good idea as well.)
> > 
> > Jonathan McDowell (4):
> >   tpm: Remove tpm_find_get_ops
> >   tpm: Add O_EXCL for exclusive /dev/tpm access
> >   tpm: Include /dev/tpmrm<n> when checking exclusive userspace TPM
> >     access
> >   tpm: Allow for exclusive TPM access when using /dev/tpm<n>
> > 
> >  drivers/char/tpm/tpm-chip.c       | 90 +++++++++++++++----------------
> >  drivers/char/tpm/tpm-dev-common.c |  8 +--
> >  drivers/char/tpm/tpm-dev.c        | 35 ++++++++++--
> >  drivers/char/tpm/tpm-dev.h        |  1 +
> >  drivers/char/tpm/tpm-interface.c  | 20 +++++--
> >  drivers/char/tpm/tpm.h            |  3 +-
> >  drivers/char/tpm/tpm2-space.c     |  5 +-
> >  drivers/char/tpm/tpm_tis_core.c   |  3 +-
> >  drivers/char/tpm/tpmrm-dev.c      | 20 ++++++-
> >  include/linux/tpm.h               |  4 +-
> >  10 files changed, 124 insertions(+), 65 deletions(-)
> > 
> > -- 
> > 2.51.0
> > 
> 
> I will put to queue with my tags but I just want to make first sure that we do not
> break anything.
> 
> I'll upgrade my test suite first to have TPM 1.2 tests (which is also
> needed for my own series) and run it in bunch of configurations.  And on
> TPM2 I check the behavior with TSS2 daemon on / off.
> 
> I have no doubts on the code changes, and it is most importantly a
> security improvement, given that "who has the access and how long" 
> can be deduced for a system configuration. I just feel that with
> this code change it is better to check and verify everything :-)

Roberto has already commented on this patch set, saying it would affect IMA[1].
I still need to look at the patch set, but please don't break IMA.

[1]https://lore.kernel.org/linux-integrity/cec499d5130f37a7887d39b44efd8538dd361fe3.camel@huaweicloud.com/

-- 
thanks,

Mimi

On Mon, Oct 27, 2025 at 07:50:46AM -0400, Mimi Zohar wrote:
> On Fri, 2025-10-24 at 21:55 +0300, Jarkko Sakkinen wrote:
> > On Mon, Oct 20, 2025 at 12:30:32PM +0100, Jonathan McDowell wrote:
> > > I hit a problem where ~ 1% of TPM firmware upgrades were failing.
> > > Investigating revealed the issue was that although the upgrade tool uses
> > > /dev/tpm0 this does not actually prevent access via /dev/tpmrm0, nor
> > > internal kernel users. It *does* prevent access to others via /dev/tpm0
> > > 
> > > So the upgrade process started, the HW RNG came in to get some
> > > randomness in the middle, did the HMAC context dance, and confused
> > > everything to the point the TPM was no longer visible to the OS even
> > > after a reboot.
> > > 
> > > Thankfully I've been able to recover those devices, but really what I'd
> > > like is the ability for a userspace tool to exclusively access the TPM
> > > without something coming in behind it. Given the lightweight attempt at
> > > locking that already exists I think this was the original intention.
> > > 
> > > I've reworked this series based on feedback received.
> > > 
> > > Firstly, it's been reordered TPM sharing functionality doesn't break
> > > during bisection.
> > > 
> > > Secondly, the O_EXCL check has been tightend up to ensure the caller is
> > > also opening the device O_RDWR. Callers shouldn't really be opening the
> > > TPM except for reading + writing, but this should help guard against
> > > unexpected flags usage a bit more.
> > > 
> > > Finally, this revision keeps the prohibition on more than one user of
> > > /dev/tpm#, to avoid unexpected breakages for clients that expect this to
> > > guard against multiple invocations. A client only then needs to use
> > > O_EXCL if it wants to prevent *all* other access, even with
> > > ContextSaves, such as the firmware upgrade case.
> > > 
> > > (I've sent a separate standalone patch that allows the TPM HW RNG to be
> > > disabled at run time, and it's now in -next, but even with that I think
> > > something like this is a good idea as well.)
> > > 
> > > Jonathan McDowell (4):
> > >   tpm: Remove tpm_find_get_ops
> > >   tpm: Add O_EXCL for exclusive /dev/tpm access
> > >   tpm: Include /dev/tpmrm<n> when checking exclusive userspace TPM
> > >     access
> > >   tpm: Allow for exclusive TPM access when using /dev/tpm<n>
> > > 
> > >  drivers/char/tpm/tpm-chip.c       | 90 +++++++++++++++----------------
> > >  drivers/char/tpm/tpm-dev-common.c |  8 +--
> > >  drivers/char/tpm/tpm-dev.c        | 35 ++++++++++--
> > >  drivers/char/tpm/tpm-dev.h        |  1 +
> > >  drivers/char/tpm/tpm-interface.c  | 20 +++++--
> > >  drivers/char/tpm/tpm.h            |  3 +-
> > >  drivers/char/tpm/tpm2-space.c     |  5 +-
> > >  drivers/char/tpm/tpm_tis_core.c   |  3 +-
> > >  drivers/char/tpm/tpmrm-dev.c      | 20 ++++++-
> > >  include/linux/tpm.h               |  4 +-
> > >  10 files changed, 124 insertions(+), 65 deletions(-)
> > > 
> > > -- 
> > > 2.51.0
> > > 
> > 
> > I will put to queue with my tags but I just want to make first sure that we do not
> > break anything.
> > 
> > I'll upgrade my test suite first to have TPM 1.2 tests (which is also
> > needed for my own series) and run it in bunch of configurations.  And on
> > TPM2 I check the behavior with TSS2 daemon on / off.
> > 
> > I have no doubts on the code changes, and it is most importantly a
> > security improvement, given that "who has the access and how long" 
> > can be deduced for a system configuration. I just feel that with
> > this code change it is better to check and verify everything :-)
> 
> Roberto has already commented on this patch set, saying it would affect IMA[1].
> I still need to look at the patch set, but please don't break IMA.

See my response in that thread.

> 
> [1]https://lore.kernel.org/linux-integrity/cec499d5130f37a7887d39b44efd8538dd361fe3.camel@huaweicloud.com/
> 
> -- 
> thanks,
> 
> Mimi

BR, Jarkko