On Tue, Aug 09, 2022 at 07:39:18PM +0000, Sven van Ashbrook wrote:

Concurrent accesses to the tpm chip are prevented by allowing only a single thread at a time to obtain a tpm chip reference through tpm_try_get_ops(). However, the tpm's suspend function does not use this mechanism, so when the tpm api is called by a kthread which does not get frozen on suspend (such as the hw_random kthread) it's possible that the tpm is used when already in suspend, or in use while in the process of suspending.

This is seen on certain ChromeOS platforms - low-probability warnings are generated during suspend. In this case, the tpm attempted to read data from a tpm chip on an already-suspended bus.

i2c_designware i2c_designware.1: Transfer while suspended

Fix:

1. prevent concurrent execution of tpm accesses and suspend/ resume, by letting suspend/resume grab the tpm_mutex.
2. before commencing a tpm access, check if the tpm chip is already suspended. Fail with -EAGAIN if so.

Tested by running 6000 suspend/resume cycles back-to-back on a ChromeOS "brya" device. The intermittent warnings reliably disappear after applying this patch. No system issues were observed.

Cc: stable@vger.kernel.org Fixes: e891db1a18bf ("tpm: turn on TPM on suspend for TPM 1.x") Signed-off-by: Sven van Ashbrook svenva@chromium.org

---

drivers/char/tpm/tpm-interface.c | 16 ++++++++++++++++ include/linux/tpm.h | 2 ++ 2 files changed, 18 insertions(+)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 1621ce818705..16ca490fd483 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -82,6 +82,11 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz) return -E2BIG; }

• if (chip->is_suspended) {

• dev_info(&chip->dev, "blocking transmit while suspended\n");
  

• return -EAGAIN;
  

• }
• rc = chip->ops->send(chip, buf, count); if (rc < 0) { if (rc != -EPIPE)

@@ -394,6 +399,8 @@ int tpm_pm_suspend(struct device *dev) if (!chip) return -ENODEV;

• mutex_lock(&chip->tpm_mutex);
• if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED) goto suspended;

@@ -411,6 +418,11 @@ int tpm_pm_suspend(struct device *dev) } suspended:

• if (!rc)

• chip->is_suspended = true;
  

• mutex_unlock(&chip->tpm_mutex);
• return rc;

} EXPORT_SYMBOL_GPL(tpm_pm_suspend); @@ -426,6 +438,10 @@ int tpm_pm_resume(struct device *dev) if (chip == NULL) return -ENODEV;

• mutex_lock(&chip->tpm_mutex);
• chip->is_suspended = false;
• mutex_unlock(&chip->tpm_mutex);
• return 0;

} EXPORT_SYMBOL_GPL(tpm_pm_resume); diff --git a/include/linux/tpm.h b/include/linux/tpm.h index d7c67581929f..0fbc1a43ae80 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -131,6 +131,8 @@ struct tpm_chip { int dev_num; /* /dev/tpm# */ unsigned long is_open; /* only one allowed */

• bool is_suspended;
• char hwrng_name[64]; struct hwrng hwrng;

2.37.1.559.g78731f0fdb-goog

What about adding TPM_CHIP_FLAG_SUSPENDED instead?

BR, Jarkko