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

[Jarkko Sakkinen <jarkko@kernel.org>]

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