linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hao Wu <hao.wu@rubrik.com>
To: hao.wu@rubrik.com, shrihari.kalkar@rubrik.com,
	seungyeop.han@rubrik.com, anish.jhaveri@rubrik.com,
	peterhuewe@gmx.de, jarkko@kernel.org, jgg@ziepe.ca,
	linux-integrity@vger.kernel.org, pmenzel@molgen.mpg.de,
	kgold@linux.ibm.com, zohar@linux.vnet.ibm.com,
	why2jjj.linux@gmail.com, hamza@hpe.com,
	gregkh@linuxfoundation.org, arnd@arndb.de,
	nayna@linux.vnet.ibm.com, James.Bottomley@hansenpartnership.com
Subject: [PATCH v3] tpm: fix Atmel TPM crash caused by too frequent queries
Date: Sun, 11 Jul 2021 00:51:22 -0700	[thread overview]
Message-ID: <20210711075122.30056-1-hao.wu@rubrik.com> (raw)
In-Reply-To: <20210709044028.77278-1-hao.wu@rubrik.com>

The Atmel TPM 1.2 chips crash with error
`tpm_try_transmit: send(): error -62` since kernel 4.14.
It is observed from the kernel log after running `tpm_sealdata -z`.
The error thrown from the command is as follows
```
$ tpm_sealdata -z
Tspi_Key_LoadKey failed: 0x00001087 - layer=tddl,
code=0087 (135), I/O error
```

The issue was reproduced with the following Atmel TPM chip:
```
$ tpm_version
T0  TPM 1.2 Version Info:
  Chip Version:        1.2.66.1
  Spec Level:          2
  Errata Revision:     3
  TPM Vendor ID:       ATML
  TPM Version:         01010000
  Manufacturer Info:   41544d4c
```

The root cause of the issue is due to the TPM calls to msleep()
were replaced with usleep_range() [1], which reduces
the actual timeout. Via experiments, it is observed that
the original msleep(5) actually sleeps for 15ms.
Because of a known timeout issue in Atmel TPM 1.2 chip,
the shorter timeout than 15ms can cause the error described above.

A few further changes in kernel 4.16 [2] and 4.18 [3, 4] further
reduced the timeout to less than 1ms. With experiments,
the problematic timeout in the latest kernel is the one
for `wait_for_tpm_stat`.

To fix it, the patch reverts the timeout of `wait_for_tpm_stat`
to 15ms for all Atmel TPM 1.2 chips, but leave it untouched
for Ateml TPM 2.0 chip, and chips from other vendors.
As explained above, the chosen 15ms timeout is
the actual timeout before this issue introduced,
thus the old value is used here.
Particularly, TPM_ATML_TIMEOUT_WAIT_STAT_MIN is set to 14700us,
TPM_ATML_TIMEOUT_WAIT_STAT_MIN is set to 15000us according to
the existing TPM_TIMEOUT_RANGE_US (300us).
The fixed has been tested in the system with the affected Atmel chip
with no issues observed after boot up.

References:
[1] 9f3fc7bcddcb tpm: replace msleep() with usleep_range() in TPM
1.2/2.0 generic drivers
[2] cf151a9a44d5 tpm: reduce tpm polling delay in tpm_tis_core
[3] 59f5a6b07f64 tpm: reduce poll sleep time in tpm_transmit()
[4] 424eaf910c32 tpm: reduce polling time to usecs for even finer
granularity

Fixes: 9f3fc7bcddcb ("tpm: replace msleep() with usleep_range() in TPM 1.2/2.0 generic drivers")
Link: https://patchwork.kernel.org/project/linux-integrity/patch/20200926223150.109645-1-hao.wu@rubrik.com/
Signed-off-by: Hao Wu <hao.wu@rubrik.com>
---
This version (v3) removes unnecessary condition check
in `wait_for_tpm_stat`.

Test Plan:
- Run fixed kernel with ATMEL TPM chips and see crash
has been fixed.
- Run fixed kernel with non-ATMEL TPM chips, and confirm
the timeout has not been changed.

drivers/char/tpm/tpm.h          |  6 ++++--
 drivers/char/tpm/tpm_tis_core.c | 14 ++++++++++++--
 include/linux/tpm.h             |  3 +++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 283f78211c3a..6de1b44c4aab 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -41,8 +41,10 @@ enum tpm_timeout {
 	TPM_TIMEOUT_RETRY = 100, /* msecs */
 	TPM_TIMEOUT_RANGE_US = 300,	/* usecs */
 	TPM_TIMEOUT_POLL = 1,	/* msecs */
-	TPM_TIMEOUT_USECS_MIN = 100,      /* usecs */
-	TPM_TIMEOUT_USECS_MAX = 500      /* usecs */
+	TPM_TIMEOUT_USECS_MIN = 100,	/* usecs */
+	TPM_TIMEOUT_USECS_MAX = 500,	/* usecs */
+	TPM_ATML_TIMEOUT_WAIT_STAT_MIN = 14700,	/* usecs */
+	TPM_ATML_TIMEOUT_WAIT_STAT_MAX = 15000	/* usecs */
 };
 
 /* TPM addresses */
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 55b9d3965ae1..2de1f71e8ae1 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -80,8 +80,8 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
 		}
 	} else {
 		do {
-			usleep_range(TPM_TIMEOUT_USECS_MIN,
-				     TPM_TIMEOUT_USECS_MAX);
+			usleep_range(chip->timeout_wait_stat_min,
+				     chip->timeout_wait_stat_max);
 			status = chip->ops->status(chip);
 			if ((status & mask) == mask)
 				return 0;
@@ -934,6 +934,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 	chip->timeout_b = msecs_to_jiffies(TIS_TIMEOUT_B_MAX);
 	chip->timeout_c = msecs_to_jiffies(TIS_TIMEOUT_C_MAX);
 	chip->timeout_d = msecs_to_jiffies(TIS_TIMEOUT_D_MAX);
+	/* init timeouts for wait_for_tpm_stat */
+	chip->timeout_wait_stat_min = TPM_TIMEOUT_USECS_MIN;
+	chip->timeout_wait_stat_max = TPM_TIMEOUT_USECS_MAX;
 	priv->phy_ops = phy_ops;
 	dev_set_drvdata(&chip->dev, priv);
 
@@ -983,6 +986,13 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
 
 	priv->manufacturer_id = vendor;
 
+	if (priv->manufacturer_id == TPM_VID_ATML &&
+		!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
+		/* If TPM chip is 1.2 ATMEL chip, timeout need to be relaxed*/
+		chip->timeout_wait_stat_min = TPM_ATML_TIMEOUT_WAIT_STAT_MIN;
+		chip->timeout_wait_stat_max = TPM_ATML_TIMEOUT_WAIT_STAT_MAX;
+	}
+
 	rc = tpm_tis_read8(priv, TPM_RID(0), &rid);
 	if (rc < 0)
 		goto out_err;
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index aa11fe323c56..171b9102c976 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -150,6 +150,8 @@ struct tpm_chip {
 	bool timeout_adjusted;
 	unsigned long duration[TPM_NUM_DURATIONS]; /* jiffies */
 	bool duration_adjusted;
+	unsigned int timeout_wait_stat_min; /* usecs */
+	unsigned int timeout_wait_stat_max; /* usecs */
 
 	struct dentry *bios_dir[TPM_NUM_EVENT_LOG_FILES];
 
@@ -269,6 +271,7 @@ enum tpm2_cc_attrs {
 #define TPM_VID_INTEL    0x8086
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
+#define TPM_VID_ATML     0x1114
 
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_TPM2		= BIT(1),
-- 
2.29.0.vfs.0.0


  parent reply	other threads:[~2021-07-11  7:51 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-20 23:18 [PATCH] Fix Atmel TPM crash caused by too frequent queries Hao Wu
2021-06-23 13:35 ` Jarkko Sakkinen
2021-06-24  5:49   ` Hao Wu
2021-06-29 20:06     ` Jarkko Sakkinen
2021-06-30  4:27       ` Hao Wu
2021-06-24  5:33 ` Hao Wu
2021-06-29 20:07   ` Jarkko Sakkinen
2021-06-30  4:22   ` [PATCH] tpm: fix ATMEL " Hao Wu
2021-07-02  6:35     ` Jarkko Sakkinen
2021-07-02  7:12       ` Greg KH
2021-07-02  7:33       ` Hao Wu
2021-07-02  7:35         ` Hao Wu
2021-07-02  7:45         ` Jarkko Sakkinen
2021-07-02  7:59           ` Hao Wu
2021-07-02  8:42             ` Jarkko Sakkinen
2021-07-02 11:57               ` Jarkko Sakkinen
2021-07-02 19:16                 ` Hao Wu
2021-07-05  5:19                   ` Jarkko Sakkinen
2021-07-05  5:29                     ` Hao Wu
2021-07-04  0:07     ` Hao Wu
2021-07-05  7:15       ` Jarkko Sakkinen
2021-07-05 23:09         ` Hao Wu
2021-07-06 12:34           ` Mimi Zohar
2021-07-07  4:18             ` Hao Wu
2021-07-07  4:34               ` Hao Wu
2021-07-07  4:31     ` [PATCH v2] " Hao Wu
2021-07-07  9:24       ` Jarkko Sakkinen
2021-07-07 18:28         ` Hao Wu
2021-07-07 21:10           ` Jarkko Sakkinen
2021-07-09  4:43             ` Hao Wu
2021-07-09  4:40     ` [PATCH v2] tpm: fix Atmel " Hao Wu
2021-07-09 17:47       ` Jarkko Sakkinen
2021-07-09 19:23         ` Hao Wu
2021-07-11  7:37           ` Hao Wu
2021-07-16  5:30             ` Hao Wu
2021-07-11  7:51       ` Hao Wu [this message]
2021-07-27  2:46         ` [PATCH v3] " Jarkko Sakkinen
2021-07-27  3:40           ` Hao Wu
2021-08-14 22:25         ` [PATCH v4] " Hao Wu
2021-08-26  5:38           ` Hao Wu
2021-08-26 16:24             ` Jarkko Sakkinen
2021-08-27  0:35               ` Hao Wu
2021-09-04 21:14                 ` Hao Wu
2021-09-04 23:15                   ` Hao Wu
2021-09-05  3:51           ` [PATCH v5] " Hao Wu
2021-09-07 17:43             ` Jarkko Sakkinen
2021-09-08  8:33               ` Hao Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210711075122.30056-1-hao.wu@rubrik.com \
    --to=hao.wu@rubrik.com \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=anish.jhaveri@rubrik.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=hamza@hpe.com \
    --cc=jarkko@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=kgold@linux.ibm.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=nayna@linux.vnet.ibm.com \
    --cc=peterhuewe@gmx.de \
    --cc=pmenzel@molgen.mpg.de \
    --cc=seungyeop.han@rubrik.com \
    --cc=shrihari.kalkar@rubrik.com \
    --cc=why2jjj.linux@gmail.com \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).