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/[email protected]/

-- 
thanks,

Mimi

Reply via email to