Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v4 0/4] tpm: add update_durations class op to allow override of chip supplied values
@ 2019-09-02 14:27 Jerry Snitselaar
  2019-09-02 14:27 ` [PATCH v4 1/3] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jerry Snitselaar
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jerry Snitselaar @ 2019-09-02 14:27 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-kernel, Alexey Klimov, Jarkko Sakkinen, Peter Huewe,
	Jason Gunthorpe

We've run into a case where a customer has an STM TPM 1.2 chip
(version 1.2.8.28), that is getting into an inconsistent state and
they end up getting tpm transmit errors.  In really old tpm code this
wasn't seen because the code that grabbed the duration values from the
chip could fail silently, and would proceed to just use default values
and move forward. More recent code though successfully gets the
duration values from the chip, and using those values this particular
chip version gets into the state seen by the customer.

The idea with this patchset is to provide a facility like the
update_timeouts operation to allow the override of chip supplied
values.

changes from v3:
    * Assign value to version when tpm1_getcap is successful for TPM 1.1 device
      not when it fails.

changes from v2:
    * Added patch 1/3
    * Rework tpm_tis_update_durations to make use of new version structs
      and pull tpm1_getcap calls out of loop.

changes from v1:
    * Remove unneeded newline
    * Formatting cleanups
    * Change tpm_tis_update_durations to be a void function, and
      use chip->duration_adjusted to track whether adjustment was
      made.

Jarkko Sakkinen (1):
      tpm: Remove duplicate code from caps_show() in tpm-sysfs.c

Jerry Snitselaar (2):
      tpm: provide a way to override the chip returned durations
      tpm_tis: override durations for STM tpm with firmware 1.2.8.28



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 1/3] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c
  2019-09-02 14:27 [PATCH v4 0/4] tpm: add update_durations class op to allow override of chip supplied values Jerry Snitselaar
@ 2019-09-02 14:27 ` Jerry Snitselaar
  2019-09-02 14:27 ` [PATCH v4 2/3] tpm: provide a way to override the chip returned durations Jerry Snitselaar
  2019-09-02 14:27 ` [PATCH v4 3/3] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Jerry Snitselaar
  2 siblings, 0 replies; 6+ messages in thread
From: Jerry Snitselaar @ 2019-09-02 14:27 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-kernel, Alexey Klimov, Jarkko Sakkinen, Peter Huewe,
	Jason Gunthorpe, Jerry Snitselaar

From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Replace existing TPM 1.x version structs with new structs that consolidate
the common parts into a single struct so that code duplication is no longer
needed in caps_show().

Cc: Peter Huewe <peterhuewe@gmx.de>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Alexey Klimov <aklimov@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Tested-by: Jerry Snitselaar <jsnitsel@redhat.com>
---
 drivers/char/tpm/tpm-sysfs.c | 45 ++++++++++++++++++------------------
 drivers/char/tpm/tpm.h       | 23 ++++++++----------
 2 files changed, 33 insertions(+), 35 deletions(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index d9caedda075b..eb05d5df4759 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -217,6 +217,7 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	struct tpm_chip *chip = to_tpm_chip(dev);
+	struct tpm1_version *version;
 	ssize_t rc = 0;
 	char *str = buf;
 	cap_t cap;
@@ -232,31 +233,31 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
 	str += sprintf(str, "Manufacturer: 0x%x\n",
 		       be32_to_cpu(cap.manufacturer_id));
 
-	/* Try to get a TPM version 1.2 TPM_CAP_VERSION_INFO */
-	rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+	/* TPM 1.2 */
+	if (!tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
 			 "attempting to determine the 1.2 version",
-			 sizeof(cap.tpm_version_1_2));
-	if (!rc) {
-		str += sprintf(str,
-			       "TCG version: %d.%d\nFirmware version: %d.%d\n",
-			       cap.tpm_version_1_2.Major,
-			       cap.tpm_version_1_2.Minor,
-			       cap.tpm_version_1_2.revMajor,
-			       cap.tpm_version_1_2.revMinor);
-	} else {
-		/* Otherwise just use TPM_STRUCT_VER */
-		if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
-				"attempting to determine the 1.1 version",
-				sizeof(cap.tpm_version)))
-			goto out_ops;
-		str += sprintf(str,
-			       "TCG version: %d.%d\nFirmware version: %d.%d\n",
-			       cap.tpm_version.Major,
-			       cap.tpm_version.Minor,
-			       cap.tpm_version.revMajor,
-			       cap.tpm_version.revMinor);
+			 sizeof(cap.version2))) {
+		version = &cap.version2.version;
+		goto out_print;
 	}
+
+	/* TPM 1.1 */
+	if (tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+			"attempting to determine the 1.1 version",
+			sizeof(cap.version1))) {
+		goto out_ops;
+	}
+
+	version = &cap.version1;
+
+out_print:
+	str += sprintf(str,
+		       "TCG version: %d.%d\nFirmware version: %d.%d\n",
+		       version->major, version->minor,
+		       version->rev_major, version->rev_minor);
+
 	rc = str - buf;
+
 out_ops:
 	tpm_put_ops(chip);
 	return rc;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index a7fea3e0ca86..a4f74dd02a35 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,19 +186,16 @@ struct	stclear_flags_t {
 	u8	bGlobalLock;
 } __packed;
 
-struct	tpm_version_t {
-	u8	Major;
-	u8	Minor;
-	u8	revMajor;
-	u8	revMinor;
+struct tpm1_version {
+	u8 major;
+	u8 minor;
+	u8 rev_major;
+	u8 rev_minor;
 } __packed;
 
-struct	tpm_version_1_2_t {
-	__be16	tag;
-	u8	Major;
-	u8	Minor;
-	u8	revMajor;
-	u8	revMinor;
+struct tpm1_version2 {
+	__be16 tag;
+	struct tpm1_version version;
 } __packed;
 
 struct	timeout_t {
@@ -243,8 +240,8 @@ typedef union {
 	struct	stclear_flags_t	stclear_flags;
 	__u8	owned;
 	__be32	num_pcrs;
-	struct	tpm_version_t	tpm_version;
-	struct	tpm_version_1_2_t tpm_version_1_2;
+	struct tpm1_version version1;
+	struct tpm1_version2 version2;
 	__be32	manufacturer_id;
 	struct timeout_t  timeout;
 	struct duration_t duration;
-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 2/3] tpm: provide a way to override the chip returned durations
  2019-09-02 14:27 [PATCH v4 0/4] tpm: add update_durations class op to allow override of chip supplied values Jerry Snitselaar
  2019-09-02 14:27 ` [PATCH v4 1/3] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jerry Snitselaar
@ 2019-09-02 14:27 ` Jerry Snitselaar
  2019-09-03 15:53   ` Jarkko Sakkinen
  2019-09-02 14:27 ` [PATCH v4 3/3] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Jerry Snitselaar
  2 siblings, 1 reply; 6+ messages in thread
From: Jerry Snitselaar @ 2019-09-02 14:27 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-kernel, Alexey Klimov, Jarkko Sakkinen, Peter Huewe,
	Jason Gunthorpe, Jerry Snitselaar

Patch adds method ->update_durations to override returned
durations in case TPM chip misbehaves for TPM 1.2 drivers.

Cc: Peter Huewe <peterhuewe@gmx.de>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: Alexey Klimov <aklimov@redhat.com>
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm1-cmd.c | 15 +++++++++++++++
 include/linux/tpm.h         |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 149e953ca369..ca7158fa6e6c 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -343,6 +343,7 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
 {
 	cap_t cap;
 	unsigned long timeout_old[4], timeout_chip[4], timeout_eff[4];
+	unsigned long durations[3];
 	ssize_t rc;
 
 	rc = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, NULL,
@@ -427,6 +428,20 @@ int tpm1_get_timeouts(struct tpm_chip *chip)
 		usecs_to_jiffies(be32_to_cpu(cap.duration.tpm_long));
 	chip->duration[TPM_LONG_LONG] = 0; /* not used under 1.2 */
 
+	/*
+	 * Provide the ability for vendor overrides of duration values in case
+	 * of misreporting.
+	 */
+	if (chip->ops->update_durations)
+		chip->ops->update_durations(chip, durations);
+
+	if (chip->duration_adjusted) {
+		dev_info(&chip->dev, HW_ERR "Adjusting reported durations.");
+		chip->duration[TPM_SHORT] = durations[0];
+		chip->duration[TPM_MEDIUM] = durations[1];
+		chip->duration[TPM_LONG] = durations[2];
+	}
+
 	/* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the above
 	 * value wrong and apparently reports msecs rather than usecs. So we
 	 * fix up the resulting too-small TPM_SHORT value to make things work.
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 53c0ea9ec9df..bb1d1ac7081d 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -67,6 +67,8 @@ struct tpm_class_ops {
 	u8 (*status) (struct tpm_chip *chip);
 	void (*update_timeouts)(struct tpm_chip *chip,
 				unsigned long *timeout_cap);
+	void (*update_durations)(struct tpm_chip *chip,
+				 unsigned long *duration_cap);
 	int (*go_idle)(struct tpm_chip *chip);
 	int (*cmd_ready)(struct tpm_chip *chip);
 	int (*request_locality)(struct tpm_chip *chip, int loc);
-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 3/3] tpm_tis: override durations for STM tpm with firmware 1.2.8.28
  2019-09-02 14:27 [PATCH v4 0/4] tpm: add update_durations class op to allow override of chip supplied values Jerry Snitselaar
  2019-09-02 14:27 ` [PATCH v4 1/3] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jerry Snitselaar
  2019-09-02 14:27 ` [PATCH v4 2/3] tpm: provide a way to override the chip returned durations Jerry Snitselaar
@ 2019-09-02 14:27 ` Jerry Snitselaar
  2019-09-03 15:54   ` Jarkko Sakkinen
  2 siblings, 1 reply; 6+ messages in thread
From: Jerry Snitselaar @ 2019-09-02 14:27 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-kernel, Alexey Klimov, Jarkko Sakkinen, Peter Huewe,
	Jason Gunthorpe, Jerry Snitselaar

There was revealed a bug in the STM TPM chipset used in Dell R415s.
Bug is observed so far only on chipset firmware 1.2.8.28
(1.2 TPM, device-id 0x0, rev-id 78). After some number of
operations chipset hangs and stays in inconsistent state:

tpm_tis 00:09: Operation Timed out
tpm_tis 00:09: tpm_transmit: tpm_send: error -5

Durations returned by the chip are the same like on other
firmware revisions but apparently with specifically 1.2.8.28 fw
durations should be reset to 2 minutes to enable tpm chip work
properly. No working way of updating firmware was found.

This patch adds implementation of ->update_durations method
that matches only STM devices with specific firmware version.

Cc: Peter Huewe <peterhuewe@gmx.de>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Signed-off-by: Alexey Klimov <aklimov@redhat.com>
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm_tis_core.c | 79 +++++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c3181ea9f271..27c6ca031e23 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -506,6 +506,84 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	return rc;
 }
 
+struct tis_vendor_durations_override {
+	u32 did_vid;
+	struct tpm1_version version;
+	unsigned long durations[3];
+};
+
+static const struct  tis_vendor_durations_override vendor_dur_overrides[] = {
+	/* STMicroelectronics 0x104a */
+	{ 0x0000104a,
+	  { 1, 2, 8, 28 },
+	  { (2 * 60 * HZ), (2 * 60 * HZ), (2 * 60 * HZ) } },
+};
+
+static void tpm_tis_update_durations(struct tpm_chip *chip,
+				     unsigned long *duration_cap)
+{
+	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+	struct tpm1_version *version;
+	u32 did_vid;
+	int i, rc;
+	cap_t cap;
+
+	chip->duration_adjusted = false;
+
+	if (chip->ops->clk_enable != NULL)
+		chip->ops->clk_enable(chip, true);
+
+	rc = tpm_tis_read32(priv, TPM_DID_VID(0), &did_vid);
+	if (rc < 0) {
+		dev_warn(&chip->dev, "%s: failed to read did_vid. %d\n",
+			 __func__, rc);
+		goto out;
+	}
+
+	/* Try to get a TPM version 1.2 or 1.1 TPM_CAP_VERSION_INFO */
+	rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_2, &cap,
+			 "attempting to determine the 1.2 version",
+			 sizeof(cap.version2));
+	if (!rc) {
+		version = &cap.version2.version;
+	} else {
+		rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
+				 "attempting to determine the 1.1 version",
+				 sizeof(cap.version1));
+
+		if (rc)
+			goto out;
+
+		version = &cap.version1;
+	}
+
+	for (i = 0; i != ARRAY_SIZE(vendor_dur_overrides); i++) {
+		if (vendor_dur_overrides[i].did_vid != did_vid)
+			continue;
+
+		if ((version->major ==
+		     vendor_dur_overrides[i].version.major) &&
+		    (version->minor ==
+		     vendor_dur_overrides[i].version.minor) &&
+		    (version->rev_major ==
+		     vendor_dur_overrides[i].version.rev_major) &&
+		    (version->rev_minor ==
+		     vendor_dur_overrides[i].version.rev_minor)) {
+
+			memcpy(duration_cap,
+			       vendor_dur_overrides[i].durations,
+			       sizeof(vendor_dur_overrides[i].durations));
+
+			chip->duration_adjusted = true;
+			goto out;
+		}
+	}
+
+out:
+	if (chip->ops->clk_enable != NULL)
+		chip->ops->clk_enable(chip, false);
+}
+
 struct tis_vendor_timeout_override {
 	u32 did_vid;
 	unsigned long timeout_us[4];
@@ -842,6 +920,7 @@ static const struct tpm_class_ops tpm_tis = {
 	.send = tpm_tis_send,
 	.cancel = tpm_tis_ready,
 	.update_timeouts = tpm_tis_update_timeouts,
+	.update_durations = tpm_tis_update_durations,
 	.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
 	.req_canceled = tpm_tis_req_canceled,
-- 
2.21.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/3] tpm: provide a way to override the chip returned durations
  2019-09-02 14:27 ` [PATCH v4 2/3] tpm: provide a way to override the chip returned durations Jerry Snitselaar
@ 2019-09-03 15:53   ` Jarkko Sakkinen
  0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2019-09-03 15:53 UTC (permalink / raw)
  To: Jerry Snitselaar, linux-integrity
  Cc: linux-kernel, Alexey Klimov, Peter Huewe, Jason Gunthorpe

On Mon, 2019-09-02 at 07:27 -0700, Jerry Snitselaar wrote:
> Patch adds method ->update_durations to override returned
> durations in case TPM chip misbehaves for TPM 1.2 drivers.
> 
> Cc: Peter Huewe <peterhuewe@gmx.de>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Signed-off-by: Alexey Klimov <aklimov@redhat.com>
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> (!update_durations path)

/Jarkko


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 3/3] tpm_tis: override durations for STM tpm with firmware 1.2.8.28
  2019-09-02 14:27 ` [PATCH v4 3/3] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Jerry Snitselaar
@ 2019-09-03 15:54   ` Jarkko Sakkinen
  0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2019-09-03 15:54 UTC (permalink / raw)
  To: Jerry Snitselaar, linux-integrity
  Cc: linux-kernel, Alexey Klimov, Peter Huewe, Jason Gunthorpe

On Mon, 2019-09-02 at 07:27 -0700, Jerry Snitselaar wrote:
> There was revealed a bug in the STM TPM chipset used in Dell R415s.
> Bug is observed so far only on chipset firmware 1.2.8.28
> (1.2 TPM, device-id 0x0, rev-id 78). After some number of
> operations chipset hangs and stays in inconsistent state:
> 
> tpm_tis 00:09: Operation Timed out
> tpm_tis 00:09: tpm_transmit: tpm_send: error -5
> 
> Durations returned by the chip are the same like on other
> firmware revisions but apparently with specifically 1.2.8.28 fw
> durations should be reset to 2 minutes to enable tpm chip work
> properly. No working way of updating firmware was found.
> 
> This patch adds implementation of ->update_durations method
> that matches only STM devices with specific firmware version.
> 
> Cc: Peter Huewe <peterhuewe@gmx.de>
> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Signed-off-by: Alexey Klimov <aklimov@redhat.com>
> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> (!update_durations path)

/Jarkko


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-02 14:27 [PATCH v4 0/4] tpm: add update_durations class op to allow override of chip supplied values Jerry Snitselaar
2019-09-02 14:27 ` [PATCH v4 1/3] tpm: Remove duplicate code from caps_show() in tpm-sysfs.c Jerry Snitselaar
2019-09-02 14:27 ` [PATCH v4 2/3] tpm: provide a way to override the chip returned durations Jerry Snitselaar
2019-09-03 15:53   ` Jarkko Sakkinen
2019-09-02 14:27 ` [PATCH v4 3/3] tpm_tis: override durations for STM tpm with firmware 1.2.8.28 Jerry Snitselaar
2019-09-03 15:54   ` Jarkko Sakkinen

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org linux-integrity@archiver.kernel.org
	public-inbox-index linux-integrity


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/ public-inbox