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 <[email protected]>
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 <[email protected]>
---
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).
Yes, this definitely provides a path where userspace could prevent a
measurement hitting the TPM from IMA. I did think about this when
working out what to do if the lock was held elsewhere, but punted on
making any changes because there are several other avenues where that
can already happen.
+
+ return tpm_try_get_ops_locked(chip);
+}
+EXPORT_SYMBOL_GPL(tpm_try_get_ops);
+
+/**
+ * tpm_put_ops() - 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.
+ */
+void tpm_put_ops(struct tpm_chip *chip)
+{
+ tpm_put_ops_locked(chip);
+
+ up_read(&chip->open_lock);
+}
EXPORT_SYMBOL_GPL(tpm_put_ops);
/**
@@ -644,10 +679,10 @@ void tpm_chip_unregister(struct tpm_chip *chip)
#ifdef CONFIG_TCG_TPM2_HMAC
int rc;
- rc = tpm_try_get_ops(chip);
+ rc = tpm_try_get_ops_locked(chip);
if (!rc) {
tpm2_end_auth_session(chip);
- tpm_put_ops(chip);
+ tpm_put_ops_locked(chip);
}
#endif
diff --git a/drivers/char/tpm/tpm-dev-common.c
b/drivers/char/tpm/tpm-dev-common.c
index f2a5e09257dd..0f5bc63411aa 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -65,7 +65,7 @@ static void tpm_dev_async_work(struct work_struct *work)
mutex_lock(&priv->buffer_mutex);
priv->command_enqueued = false;
- ret = tpm_try_get_ops(priv->chip);
+ ret = tpm_try_get_ops_locked(priv->chip);
if (ret) {
priv->response_length = ret;
goto out;
@@ -73,7 +73,7 @@ static void tpm_dev_async_work(struct work_struct *work)
ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer,
sizeof(priv->data_buffer));
- tpm_put_ops(priv->chip);
+ tpm_put_ops_locked(priv->chip);
/*
* If ret is > 0 then tpm_dev_transmit returned the size of the
@@ -220,14 +220,14 @@ ssize_t tpm_common_write(struct file *file, const char
__user *buf,
* lock during this period so that the tpm can be unregistered even if
* the char dev is held open.
*/
- if (tpm_try_get_ops(priv->chip)) {
+ if (tpm_try_get_ops_locked(priv->chip)) {
ret = -EPIPE;
goto out;
}
ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer,
sizeof(priv->data_buffer));
- tpm_put_ops(priv->chip);
+ tpm_put_ops_locked(priv->chip);
if (ret > 0) {
priv->response_length = ret;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 02c07fef41ba..57ef8589f5f5 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -272,6 +272,8 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev,
const struct tpm_class_ops *ops);
struct tpm_chip *tpmm_chip_alloc(struct device *pdev,
const struct tpm_class_ops *ops);
+int tpm_try_get_ops_locked(struct tpm_chip *chip);
+void tpm_put_ops_locked(struct tpm_chip *chip);
int tpm_chip_register(struct tpm_chip *chip);
void tpm_chip_unregister(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 60354cd53b5c..0ad5e18355e0 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -58,10 +58,9 @@ int tpm2_init_space(struct tpm_space *space, unsigned int
buf_size)
void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
{
-
- if (tpm_try_get_ops(chip) == 0) {
+ if (tpm_try_get_ops_locked(chip) == 0) {
tpm2_flush_sessions(chip, space);
- tpm_put_ops(chip);
+ tpm_put_ops_locked(chip);
}
kfree(space->context_buf);
J.
--
"I'm a compsci. I don't write code." -- Noodles. "I'm a DB coder.
Neither do I." -- Adrian.
