Re: [PATCH v3 4/4] tpm: Allow for exclusive TPM access when using /dev/tpm

Jarkko Sakkinen
2025-10-27T19:38:59Z

On Mon, Oct 20, 2025 at 01:53:30PM +0200, Roberto Sassu wrote:
> On Mon, 2025-10-20 at 12:31 +0100, Jonathan McDowell wrote:
> > From: Jonathan McDowell <noodles@meta.com>
> >
> > There are situations where userspace might reasonably desire exclusive
> > access to the TPM, or the kernel's internal context saving + flushing
> > may cause issues, for example when performing firmware upgrades. Extend
> > the locking already used for avoiding concurrent userspace access to
> > prevent internal users of the TPM when /dev/tpm<n> is in use.
> >
> > The few internal users who already hold the open_lock are changed to use
> > tpm_internal_(try_get|put)_ops, with the old tpm_(try_get|put)_ops
> > functions changing to obtain read access to the open_lock. We return
> > -EBUSY when another user has exclusive access, rather than adding waits.
> >
> > Signed-off-by: Jonathan McDowell <noodles@meta.com>
> > ---
> > v2: Switch to _locked instead of _internal_ for function names.
> > v3: Move to end of patch series.
> >
> > drivers/char/tpm/tpm-chip.c | 53 +++++++++++++++++++++++++------
> > drivers/char/tpm/tpm-dev-common.c | 8 ++---
> > drivers/char/tpm/tpm.h | 2 ++
> > drivers/char/tpm/tpm2-space.c | 5 ++-
> > 4 files changed, 52 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> > index ba906966721a..687f6d8cd601 100644
> > --- a/drivers/char/tpm/tpm-chip.c
> > +++ b/drivers/char/tpm/tpm-chip.c
> > @@ -144,7 +144,7 @@ void tpm_chip_stop(struct tpm_chip *chip)
> > EXPORT_SYMBOL_GPL(tpm_chip_stop);
> >
> > /**
> > - * tpm_try_get_ops() - Get a ref to the tpm_chip
> > + * tpm_try_get_ops_locked() - Get a ref to the tpm_chip
> > * @chip: Chip to ref
> > *
> > * The caller must already have some kind of locking to ensure that chip is
> > @@ -154,7 +154,7 @@ EXPORT_SYMBOL_GPL(tpm_chip_stop);
> > *
> > * Returns -ERRNO if the chip could not be got.
> > */
> > -int tpm_try_get_ops(struct tpm_chip *chip)
> > +int tpm_try_get_ops_locked(struct tpm_chip *chip)
> > {
> > int rc = -EIO;
> >
> > @@ -185,22 +185,57 @@ int tpm_try_get_ops(struct tpm_chip *chip)
> > put_device(&chip->dev);
> > return rc;
> > }
> > -EXPORT_SYMBOL_GPL(tpm_try_get_ops);
> >
> > /**
> > - * tpm_put_ops() - Release a ref to the tpm_chip
> > + * tpm_put_ops_locked() - Release a ref to the tpm_chip
> > * @chip: Chip to put
> > *
> > - * This is the opposite pair to tpm_try_get_ops(). After this returns chip may
> > - * be kfree'd.
> > + * This is the opposite pair to tpm_try_get_ops_locked(). After this returns
> > + * chip may be kfree'd.
> > */
> > -void tpm_put_ops(struct tpm_chip *chip)
> > +void tpm_put_ops_locked(struct tpm_chip *chip)
> > {
> > tpm_chip_stop(chip);
> > mutex_unlock(&chip->tpm_mutex);
> > up_read(&chip->ops_sem);
> > put_device(&chip->dev);
> > }
> > +
> > +/**
> > + * tpm_try_get_ops() - Get a ref to the tpm_chip
> > + * @chip: Chip to ref
> > + *
> > + * The caller must already have some kind of locking to ensure that chip is
> > + * valid. This function will attempt to get the open_lock for the chip,
> > + * ensuring no other user is expecting exclusive access, before locking the
> > + * chip so that the ops member can be accessed safely. The locking prevents
> > + * tpm_chip_unregister from completing, so it should not be held for long
> > + * periods.
> > + *
> > + * Returns -ERRNO if the chip could not be got.
> > + */
> > +int tpm_try_get_ops(struct tpm_chip *chip)
> > +{
> > + if (!down_read_trylock(&chip->open_lock))
> > + return -EBUSY;
>
> Hi Jonathan
>
> do I understand it correctly, that a process might open the TPM with
> O_EXCL, and this will prevent IMA from extending a PCR until that
> process closes the file descriptor?
>
> If yes, this might be a concern, and I think an additional API to
> prevent such behavior would be needed (for example when IMA is active,
> i.e. there is a measurement policy loaded).

Also this would be a problem with hwrng.

This probably needs to be refined somehow. I don't have a solution at
hand but "invariant" is that in-kernel caller should override user space
exclusion, even when O_EXCL is used.

>
> Thanks
>
> Roberto

BR, Jarkko