Re: [PATCH v3 0/4] pm: Ensure exclusive userspace access when using /dev/tpm

Jarkko Sakkinen
2025-10-27T19:41:06Z

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