linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/16] Remove nested TPM operations
@ 2018-11-05  1:45 Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter Jarkko Sakkinen
                   ` (15 more replies)
  0 siblings, 16 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Jason Gunthorpe, open list

[was Detach TPM space code out of the tpm_transmit() flow but the scope
 expanded a bit.]

Make the changes necessary to detach TPM space code and TPM activation
code out of the tpm_transmit() flow because of both of these can cause
nested tpm_transmit() calls. The nesteds calls make the whole flow hard
to maintain, and thus, it is better to just fix things now before this
turns into a bigger mess.

The commits are available in a branch called 'nested' in

git://git.infradead.org/users/jjs/linux-tpmdd.git

v3:
* Encapsulate power gating code to tpm_chip_start() and tpm_chip_stop().
* Move TPM power gating code and locking to tpm_try_get_ops() and
  tpm_put_ops().
* Call power gating code directly in tpm_chip_register() and
  tpm2_del_space().

v2:
* Print tpm2_commit_space() error inside tpm2_commit_space()
* Error code was not printed when recv() callback failed. It is
  fixed in this version.
* Added a patch that removes @space from tpm_transmit().
* Fixed a regression in earlier series. Forgot to amend the change
  from the staging area that renames NESTED to UNLOCKED in tpm2-space.c.

Jarkko Sakkinen (16):
  tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter
  tpm: fix invalid return value in pubek_show()
  tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails
  tpm: call tpm2_flush_space() on error in tpm_try_transmit()
  tpm: print tpm2_commit_space() error inside tpm2_commit_space()
  tpm: clean up tpm_try_transmit() error handling flow
  tpm: access command header through struct in  tpm_try_transmit()
  tpm: move tpm_validate_commmand() to tpm2-space.c
  tpm: encapsulate tpm_dev_transmit()
  tpm: move TPM space code out of tpm_transmit()
  tpm: remove @space from tpm_transmit()
  tpm: use tpm_try_get_ops() in tpm-sysfs.c.
  tpm: remove TPM_TRANSMIT_UNLOCKED flag
  tpm: introduce tpm_chip_start() and tpm_chip_stop()
  tpm: take TPM chip power gating out of tpm_transmit()
  tpm: remove @flags from tpm_transmit()

 drivers/char/tpm/tpm-chip.c       | 105 +++++++++++-
 drivers/char/tpm/tpm-dev-common.c |  45 ++++-
 drivers/char/tpm/tpm-interface.c  | 262 ++++++------------------------
 drivers/char/tpm/tpm-sysfs.c      | 141 +++++++++-------
 drivers/char/tpm/tpm.h            |  37 ++---
 drivers/char/tpm/tpm1-cmd.c       |  28 +---
 drivers/char/tpm/tpm2-cmd.c       |  68 +++-----
 drivers/char/tpm/tpm2-space.c     |  87 +++++++---
 drivers/char/tpm/tpm_vtpm_proxy.c |   4 +-
 9 files changed, 382 insertions(+), 395 deletions(-)

-- 
2.19.1


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

* [PATCH v3 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05 21:48   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 02/16] tpm: fix invalid return value in pubek_show() Jarkko Sakkinen
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Since we pass an initialized struct tpm_buf instance in every call site
now, it is cleaner to pass that directly to the tpm_transmit_cmd() as
the TPM command/response buffer.

Fine-tune a little bit tpm_transmit() and tpm_transmit_cmd() comments
while doing this.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c  | 67 +++++++++++++++++--------------
 drivers/char/tpm/tpm-sysfs.c      |  2 +-
 drivers/char/tpm/tpm.h            |  5 +--
 drivers/char/tpm/tpm1-cmd.c       | 26 ++++--------
 drivers/char/tpm/tpm2-cmd.c       | 37 +++++++----------
 drivers/char/tpm/tpm2-space.c     |  4 +-
 drivers/char/tpm/tpm_vtpm_proxy.c |  3 +-
 7 files changed, 64 insertions(+), 80 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d9439f9abe78..64510ed81b46 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -298,23 +298,22 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 
 /**
  * tpm_transmit - Internal kernel interface to transmit TPM commands.
+ * @chip:	a TPM chip to use
+ * @space:	a TPM space
+ * @buf:	a TPM command buffer
+ * @bufsiz:	length of the TPM command buffer
+ * @flags:	TPM transmit flags
  *
- * @chip: TPM chip to use
- * @space: tpm space
- * @buf: TPM command buffer
- * @bufsiz: length of the TPM command buffer
- * @flags: tpm transmit flags - bitmap
+ * A wrapper around tpm_try_transmit() that handles TPM2_RC_RETRY returns from
+ * the TPM and retransmits the command after a delay up to a maximum wait of
+ * TPM2_DURATION_LONG.
  *
- * A wrapper around tpm_try_transmit that handles TPM2_RC_RETRY
- * returns from the TPM and retransmits the command after a delay up
- * to a maximum wait of TPM2_DURATION_LONG.
- *
- * Note: TPM1 never returns TPM2_RC_RETRY so the retry logic is TPM2
- * only
+ * Note that TPM 1.x never returns TPM2_RC_RETRY so the retry logic is TPM 2.0
+ * only.
  *
  * Return:
- *     the length of the return when the operation is successful.
- *     A negative number for system errors (errno).
+ * * The response length	- OK
+ * * -errno			- A system error
  */
 ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 		     u8 *buf, size_t bufsiz, unsigned int flags)
@@ -365,33 +364,31 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	}
 	return ret;
 }
+
 /**
  * tpm_transmit_cmd - send a tpm command to the device
- *    The function extracts tpm out header return code
- *
- * @chip: TPM chip to use
- * @space: tpm space
- * @buf: TPM command buffer
- * @bufsiz: length of the buffer
- * @min_rsp_body_length: minimum expected length of response body
- * @flags: tpm transmit flags - bitmap
- * @desc: command description used in the error message
+ * @chip:			a TPM chip to use
+ * @space:			a TPM space
+ * @buf:			a TPM command buffer
+ * @min_rsp_body_length:	minimum expected length of response body
+ * @flags:			TPM transmit flags
+ * @desc:			command description used in the error message
  *
  * Return:
- *     0 when the operation is successful.
- *     A negative number for system errors (errno).
- *     A positive number for a TPM error.
+ * * 0		- OK
+ * * -errno	- A system error
+ * * TPM_RC	- A TPM error
  */
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
-			 void *buf, size_t bufsiz,
-			 size_t min_rsp_body_length, unsigned int flags,
-			 const char *desc)
+			 struct tpm_buf *buf, size_t min_rsp_body_length,
+			 unsigned int flags, const char *desc)
 {
-	const struct tpm_output_header *header = buf;
+	const struct tpm_output_header *header =
+		(struct tpm_output_header *)buf->data;
 	int err;
 	ssize_t len;
 
-	len = tpm_transmit(chip, space, buf, bufsiz, flags);
+	len = tpm_transmit(chip, space, buf->data, PAGE_SIZE, flags);
 	if (len <  0)
 		return len;
 
@@ -528,14 +525,22 @@ EXPORT_SYMBOL_GPL(tpm_pcr_extend);
  */
 int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
 {
+	struct tpm_buf buf;
 	int rc;
 
 	chip = tpm_find_get_ops(chip);
 	if (!chip)
 		return -ENODEV;
 
-	rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
+	rc = tpm_buf_init(&buf, 0, 0);
+	if (rc)
+		goto out;
+
+	memcpy(buf.data, cmd, buflen);
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
 			      "attempting to a send a command");
+	tpm_buf_destroy(&buf);
+out:
 	tpm_put_ops(chip);
 	return rc;
 }
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index b88e08ec2c59..c2769e55cb6c 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -53,7 +53,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 
 	tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
 
-	rc = tpm_transmit_cmd(chip, NULL, tpm_buf.data, PAGE_SIZE,
+	rc = tpm_transmit_cmd(chip, NULL, &tpm_buf,
 			      READ_PUBEK_RESULT_MIN_BODY_SIZE, 0,
 			      "attempting to read the PUBEK");
 	if (rc) {
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index f27d1f38a93d..49bca4d1e786 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -503,9 +503,8 @@ enum tpm_transmit_flags {
 ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 		     u8 *buf, size_t bufsiz, unsigned int flags);
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
-			 void *buf, size_t bufsiz,
-			 size_t min_rsp_body_length, unsigned int flags,
-			 const char *desc);
+			 struct tpm_buf *buf, size_t min_rsp_body_length,
+			 unsigned int flags, const char *desc);
 int tpm_get_timeouts(struct tpm_chip *);
 int tpm_auto_startup(struct tpm_chip *chip);
 
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 6f306338953b..f19b7c1ff800 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -334,11 +334,9 @@ static int tpm1_startup(struct tpm_chip *chip)
 
 	tpm_buf_append_u16(&buf, TPM_ST_CLEAR);
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
 			      "attempting to start the TPM");
-
 	tpm_buf_destroy(&buf);
-
 	return rc;
 }
 
@@ -462,9 +460,7 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
 	tpm_buf_append_u32(&buf, pcr_idx);
 	tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
-			      TPM_DIGEST_SIZE, 0, log_msg);
-
+	rc = tpm_transmit_cmd(chip, NULL, &buf, TPM_DIGEST_SIZE, 0, log_msg);
 	tpm_buf_destroy(&buf);
 	return rc;
 }
@@ -494,11 +490,9 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		tpm_buf_append_u32(&buf, 4);
 		tpm_buf_append_u32(&buf, subcap_id);
 	}
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
-			      min_cap_length, 0, desc);
+	rc = tpm_transmit_cmd(chip, NULL, &buf, min_cap_length, 0, desc);
 	if (!rc)
 		*cap = *(cap_t *)&buf.data[TPM_HEADER_SIZE + 4];
-
 	tpm_buf_destroy(&buf);
 	return rc;
 }
@@ -537,7 +531,7 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 	do {
 		tpm_buf_append_u32(&buf, num_bytes);
 
-		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
+		rc = tpm_transmit_cmd(chip, NULL, &buf,
 				      sizeof(out->rng_data_len), 0,
 				      "attempting get random");
 		if (rc)
@@ -583,8 +577,7 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
 
 	tpm_buf_append_u32(&buf, pcr_idx);
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
-			      TPM_DIGEST_SIZE, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &buf, TPM_DIGEST_SIZE, 0,
 			      "attempting to read a pcr value");
 	if (rc)
 		goto out;
@@ -618,11 +611,8 @@ static int tpm1_continue_selftest(struct tpm_chip *chip)
 	if (rc)
 		return rc;
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
-			      0, 0, "continue selftest");
-
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, "continue selftest");
 	tpm_buf_destroy(&buf);
-
 	return rc;
 }
 
@@ -747,9 +737,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
 		return rc;
 	/* now do the actual savestate */
 	for (try = 0; try < TPM_RETRY; try++) {
-		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
-				      0, 0, NULL);
-
+		rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, NULL);
 		/*
 		 * If the TPM indicates that it is too busy to respond to
 		 * this command then retry before giving up.  It can take
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index a6bec13afa69..2bcf470c8e5d 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -197,8 +197,8 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
 	tpm_buf_append(&buf, (const unsigned char *)pcr_select,
 		       sizeof(pcr_select));
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
-			res_buf ? "attempting to read a pcr value" : NULL);
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, res_buf ?
+			      "attempting to read a pcr value" : NULL);
 	if (rc == 0 && res_buf) {
 		out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
 		memcpy(res_buf, out->digest, SHA1_DIGEST_SIZE);
@@ -264,7 +264,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
 		}
 	}
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
 			      "attempting extend a PCR value");
 
 	tpm_buf_destroy(&buf);
@@ -309,7 +309,7 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 	do {
 		tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
 		tpm_buf_append_u16(&buf, num_bytes);
-		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
+		err = tpm_transmit_cmd(chip, NULL, &buf,
 				       offsetof(struct tpm2_get_random_out,
 						buffer),
 				       0, "attempting get random");
@@ -362,9 +362,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 
 	tpm_buf_append_u32(&buf, handle);
 
-	(void) tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, flags,
-				"flushing context");
-
+	tpm_transmit_cmd(chip, NULL, &buf, 0, flags, "flushing context");
 	tpm_buf_destroy(&buf);
 }
 
@@ -478,8 +476,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
-			      "sealing data");
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 4, 0, "sealing data");
 	if (rc)
 		goto out;
 
@@ -561,8 +558,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, flags,
-			      "loading blob");
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 4, flags, "loading blob");
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -612,8 +608,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 6, flags,
-			      "unsealing");
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 6, flags, "unsealing");
 	if (rc > 0)
 		rc = -EPERM;
 
@@ -703,7 +698,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
 	tpm_buf_append_u32(&buf, property_id);
 	tpm_buf_append_u32(&buf, 1);
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, NULL);
 	if (!rc) {
 		out = (struct tpm2_get_cap_out *)
 			&buf.data[TPM_HEADER_SIZE];
@@ -733,8 +728,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 	if (rc)
 		return;
 	tpm_buf_append_u16(&buf, shutdown_type);
-	tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
-			 "stopping the TPM");
+	tpm_transmit_cmd(chip, NULL, &buf, 0, 0, "stopping the TPM");
 	tpm_buf_destroy(&buf);
 }
 
@@ -763,7 +757,7 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
 			return rc;
 
 		tpm_buf_append_u8(&buf, full);
-		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+		rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
 				      "attempting the self test");
 		tpm_buf_destroy(&buf);
 
@@ -800,7 +794,7 @@ int tpm2_probe(struct tpm_chip *chip)
 	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
 	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
 	tpm_buf_append_u32(&buf, 1);
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, NULL);
 	/* We ignore TPM return codes on purpose. */
 	if (rc >=  0) {
 		out = (struct tpm_output_header *)buf.data;
@@ -839,7 +833,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 	tpm_buf_append_u32(&buf, 0);
 	tpm_buf_append_u32(&buf, 1);
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 9, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 9, 0,
 			      "get tpm pcr allocation");
 	if (rc)
 		goto out;
@@ -911,8 +905,7 @@ static int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
 	tpm_buf_append_u32(&buf, TPM2_CC_FIRST);
 	tpm_buf_append_u32(&buf, nr_commands);
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
-			      9 + 4 * nr_commands, 0, NULL);
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 9 + 4 * nr_commands, 0, NULL);
 	if (rc) {
 		tpm_buf_destroy(&buf);
 		goto out;
@@ -969,7 +962,7 @@ static int tpm2_startup(struct tpm_chip *chip)
 		return rc;
 
 	tpm_buf_append_u16(&buf, TPM2_SU_CLEAR);
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
 			      "attempting to start the TPM");
 	tpm_buf_destroy(&buf);
 
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index dcdfde3c253e..1131a8e7b79b 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -83,7 +83,7 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
 	body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
 	tpm_buf_append(&tbuf, &buf[*offset], body_size);
 
-	rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 4,
+	rc = tpm_transmit_cmd(chip, NULL, &tbuf, 4,
 			      TPM_TRANSMIT_NESTED, NULL);
 	if (rc < 0) {
 		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
@@ -132,7 +132,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 
 	tpm_buf_append_u32(&tbuf, handle);
 
-	rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &tbuf, 0,
 			      TPM_TRANSMIT_NESTED, NULL);
 	if (rc < 0) {
 		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 87a0ce47f201..5f95fbfb7f6b 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -417,8 +417,7 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
 
 	proxy_dev->state |= STATE_DRIVER_COMMAND;
 
-	rc = tpm_transmit_cmd(chip, NULL, buf.data, tpm_buf_length(&buf), 0,
-			      TPM_TRANSMIT_NESTED,
+	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, TPM_TRANSMIT_NESTED,
 			      "attempting to set locality");
 
 	proxy_dev->state &= ~STATE_DRIVER_COMMAND;
-- 
2.19.1


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

* [PATCH v3 02/16] tpm: fix invalid return value in pubek_show()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05 21:51   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 03/16] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails Jarkko Sakkinen
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Return zero when tpm_buf_init() fails as we do for other functions in
tpm-sysfs.c.

Fixes: da379f3c1db0c ("tpm: migrate pubek_show to struct tpm_buf")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-sysfs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index c2769e55cb6c..7ed7eb6f906a 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -47,9 +47,8 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 
 	memset(&anti_replay, 0, sizeof(anti_replay));
 
-	rc = tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK);
-	if (rc)
-		return rc;
+	if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK))
+		return 0;
 
 	tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
 
-- 
2.19.1


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

* [PATCH v3 03/16] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 02/16] tpm: fix invalid return value in pubek_show() Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05 21:54   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 04/16] tpm: call tpm2_flush_space() on error in tpm_try_transmit() Jarkko Sakkinen
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Do not print partial list of PCRs when tpm1_pcr_read() fails but instead
return 0 from pcrs_show(). This is consistent behavior with other sysfs
functions.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-sysfs.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 7ed7eb6f906a..d3b05c7526c8 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -115,13 +115,15 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
 	for (i = 0; i < num_pcrs; i++) {
 		rc = tpm1_pcr_read(chip, i, digest);
 		if (rc)
-			break;
+			goto out;
 		str += sprintf(str, "PCR-%02d: ", i);
 		for (j = 0; j < TPM_DIGEST_SIZE; j++)
 			str += sprintf(str, "%02X ", digest[j]);
 		str += sprintf(str, "\n");
 	}
-	return str - buf;
+	rc = str - buf;
+out:
+	return rc;
 }
 static DEVICE_ATTR_RO(pcrs);
 
-- 
2.19.1


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

* [PATCH v3 04/16] tpm: call tpm2_flush_space() on error in tpm_try_transmit()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (2 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 03/16] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05 22:01   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 05/16] tpm: print tpm2_commit_space() error inside tpm2_commit_space() Jarkko Sakkinen
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	stable, Peter Huewe, Jason Gunthorpe, Arnd Bergmann,
	Greg Kroah-Hartman, open list

Always call tpm2_flush_space() on failure in tpm_try_transmit() so that
the volatile memory of the TPM gets cleared. If /dev/tpm0 does not have
sufficient permissions (usually it has), this could leak to the leakage
of TPM objects. Through /dev/tpmrm0 this issue does not raise new
security concerns.

Cc: stable@vger.kernel.org
Fixes: 745b361e989a ("tpm:tpm: infrastructure for TPM spaces")
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++-----------
 drivers/char/tpm/tpm.h           |  1 +
 drivers/char/tpm/tpm2-space.c    |  2 +-
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 64510ed81b46..ecda6f96cde0 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -224,14 +224,14 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 
 	rc = tpm2_prepare_space(chip, space, ordinal, buf);
 	if (rc)
-		goto out;
+		goto out_idle;
 
 	rc = chip->ops->send(chip, buf, count);
 	if (rc < 0) {
 		if (rc != -EPIPE)
 			dev_err(&chip->dev,
 				"%s: tpm_send: error %d\n", __func__, rc);
-		goto out;
+		goto out_space;
 	}
 
 	if (chip->flags & TPM_CHIP_FLAG_IRQ)
@@ -247,7 +247,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 		if (chip->ops->req_canceled(chip, status)) {
 			dev_err(&chip->dev, "Operation Canceled\n");
 			rc = -ECANCELED;
-			goto out;
+			goto out_space;
 		}
 
 		tpm_msleep(TPM_TIMEOUT_POLL);
@@ -257,7 +257,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 	chip->ops->cancel(chip);
 	dev_err(&chip->dev, "Operation Timed out\n");
 	rc = -ETIME;
-	goto out;
+	goto out_space;
 
 out_recv:
 	len = chip->ops->recv(chip, buf, bufsiz);
@@ -265,22 +265,28 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 		rc = len;
 		dev_err(&chip->dev,
 			"tpm_transmit: tpm_recv: error %d\n", rc);
-		goto out;
+		goto out_idle;
 	} else if (len < TPM_HEADER_SIZE) {
 		rc = -EFAULT;
-		goto out;
+		goto out_idle;
 	}
 
 	if (len != be32_to_cpu(header->length)) {
 		rc = -EFAULT;
-		goto out;
+		goto out_idle;
 	}
 
-	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
-	if (rc)
-		dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
+out_space:
+	if (rc) {
+		tpm2_flush_space(chip);
+	} else {
+		rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
+		if (rc)
+			dev_err(&chip->dev, "tpm2_commit_space: error %d\n",
+				rc);
+	}
 
-out:
+out_idle:
 	/* may fail but do not override previous error value in rc */
 	tpm_go_idle(chip, flags);
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 49bca4d1e786..229ac42b644e 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -579,6 +579,7 @@ int tpm2_probe(struct tpm_chip *chip);
 int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
 int tpm2_init_space(struct tpm_space *space);
 void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
+void tpm2_flush_space(struct tpm_chip *chip);
 int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 		       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 1131a8e7b79b..d53c882268ff 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -162,7 +162,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 	return 0;
 }
 
-static void tpm2_flush_space(struct tpm_chip *chip)
+void tpm2_flush_space(struct tpm_chip *chip)
 {
 	struct tpm_space *space = &chip->work_space;
 	int i;
-- 
2.19.1


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

* [PATCH v3 05/16] tpm: print tpm2_commit_space() error inside tpm2_commit_space()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (3 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 04/16] tpm: call tpm2_flush_space() on error in tpm_try_transmit() Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05 22:04   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 06/16] tpm: clean up tpm_try_transmit() error handling flow Jarkko Sakkinen
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

The error logging for tpm2_commit_space() is in a wrong place. This
commit moves it inside that function.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm2-space.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index d53c882268ff..3d5f9577e5de 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -501,19 +501,19 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 	rc = tpm2_map_response_header(chip, cc, buf, *bufsiz);
 	if (rc) {
 		tpm2_flush_space(chip);
-		return rc;
+		goto out;
 	}
 
 	rc = tpm2_map_response_body(chip, cc, buf, *bufsiz);
 	if (rc) {
 		tpm2_flush_space(chip);
-		return rc;
+		goto out;
 	}
 
 	rc = tpm2_save_space(chip);
 	if (rc) {
 		tpm2_flush_space(chip);
-		return rc;
+		goto out;
 	}
 
 	*bufsiz = be32_to_cpu(header->length);
@@ -526,4 +526,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 	memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE);
 
 	return 0;
+out:
+	dev_err(&chip->dev, "%s: error %d\n", __func__, rc);
+	return rc;
 }
-- 
2.19.1


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

* [PATCH v3 06/16] tpm: clean up tpm_try_transmit() error handling flow
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (4 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 05/16] tpm: print tpm2_commit_space() error inside tpm2_commit_space() Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05 22:20   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 07/16] tpm: access command header through struct in tpm_try_transmit() Jarkko Sakkinen
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Move locking, locality handling and power management to tpm_transmit()
in order to simplify the flow.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 90 ++++++++++++++------------------
 1 file changed, 39 insertions(+), 51 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index ecda6f96cde0..0f343407daf8 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -171,7 +171,6 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 	ssize_t len = 0;
 	u32 count, ordinal;
 	unsigned long stop;
-	bool need_locality;
 
 	rc = tpm_validate_command(chip, space, buf, bufsiz);
 	if (rc == -EINVAL)
@@ -201,30 +200,9 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 		return -E2BIG;
 	}
 
-	if (!(flags & TPM_TRANSMIT_UNLOCKED) && !(flags & TPM_TRANSMIT_NESTED))
-		mutex_lock(&chip->tpm_mutex);
-
-	if (chip->ops->clk_enable != NULL)
-		chip->ops->clk_enable(chip, true);
-
-	/* Store the decision as chip->locality will be changed. */
-	need_locality = chip->locality == -1;
-
-	if (need_locality) {
-		rc = tpm_request_locality(chip, flags);
-		if (rc < 0) {
-			need_locality = false;
-			goto out_locality;
-		}
-	}
-
-	rc = tpm_cmd_ready(chip, flags);
-	if (rc)
-		goto out_locality;
-
 	rc = tpm2_prepare_space(chip, space, ordinal, buf);
 	if (rc)
-		goto out_idle;
+		return rc;
 
 	rc = chip->ops->send(chip, buf, count);
 	if (rc < 0) {
@@ -265,40 +243,16 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 		rc = len;
 		dev_err(&chip->dev,
 			"tpm_transmit: tpm_recv: error %d\n", rc);
-		goto out_idle;
-	} else if (len < TPM_HEADER_SIZE) {
-		rc = -EFAULT;
-		goto out_idle;
-	}
-
-	if (len != be32_to_cpu(header->length)) {
+		tpm2_flush_space(chip);
+	} else if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
 		rc = -EFAULT;
-		goto out_idle;
-	}
 
 out_space:
-	if (rc) {
+	if (rc)
 		tpm2_flush_space(chip);
-	} else {
+	else
 		rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
-		if (rc)
-			dev_err(&chip->dev, "tpm2_commit_space: error %d\n",
-				rc);
-	}
-
-out_idle:
-	/* may fail but do not override previous error value in rc */
-	tpm_go_idle(chip, flags);
-
-out_locality:
-	if (need_locality)
-		tpm_relinquish_locality(chip, flags);
-
-	if (chip->ops->clk_enable != NULL)
-		chip->ops->clk_enable(chip, false);
 
-	if (!(flags & TPM_TRANSMIT_UNLOCKED) && !(flags & TPM_TRANSMIT_NESTED))
-		mutex_unlock(&chip->tpm_mutex);
 	return rc ? rc : len;
 }
 
@@ -328,6 +282,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	/* space for header and handles */
 	u8 save[TPM_HEADER_SIZE + 3*sizeof(u32)];
 	unsigned int delay_msec = TPM2_DURATION_SHORT;
+	bool has_locality = false;
 	u32 rc = 0;
 	ssize_t ret;
 	const size_t save_size = min(space ? sizeof(save) : TPM_HEADER_SIZE,
@@ -343,7 +298,40 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	memcpy(save, buf, save_size);
 
 	for (;;) {
+		if (!(flags & TPM_TRANSMIT_UNLOCKED) &&
+		    !(flags & TPM_TRANSMIT_NESTED))
+			mutex_lock(&chip->tpm_mutex);
+
+		if (chip->ops->clk_enable != NULL)
+			chip->ops->clk_enable(chip, true);
+
+		if (chip->locality == -1) {
+			ret = tpm_request_locality(chip, flags);
+			if (ret)
+				goto out_locality;
+			has_locality = true;
+		}
+
+		ret = tpm_cmd_ready(chip, flags);
+		if (ret)
+			goto out_locality;
+
 		ret = tpm_try_transmit(chip, space, buf, bufsiz, flags);
+
+		/* This may fail but do not override ret. */
+		tpm_go_idle(chip, flags);
+
+out_locality:
+		if (has_locality)
+			tpm_relinquish_locality(chip, flags);
+
+		if (chip->ops->clk_enable != NULL)
+			chip->ops->clk_enable(chip, false);
+
+		if (!(flags & TPM_TRANSMIT_UNLOCKED) &&
+		    !(flags & TPM_TRANSMIT_NESTED))
+			mutex_unlock(&chip->tpm_mutex);
+
 		if (ret < 0)
 			break;
 		rc = be32_to_cpu(header->return_code);
-- 
2.19.1


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

* [PATCH v3 07/16] tpm: access command header through struct in  tpm_try_transmit()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (5 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 06/16] tpm: clean up tpm_try_transmit() error handling flow Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05 22:26   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 08/16] tpm: move tpm_validate_commmand() to tpm2-space.c Jarkko Sakkinen
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Instead of accessing fields of the command header through offsets to
the raw buffer, it is a better idea to use the header struct pointer
that is already used elsewhere in the function.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 0f343407daf8..422e3bb0bd3d 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -190,8 +190,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 	if (bufsiz > TPM_BUFSIZE)
 		bufsiz = TPM_BUFSIZE;
 
-	count = be32_to_cpu(*((__be32 *) (buf + 2)));
-	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
+	count = be32_to_cpu(header->length);
+	ordinal = be32_to_cpu(header->return_code);
 	if (count == 0)
 		return -ENODATA;
 	if (count > bufsiz) {
-- 
2.19.1


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

* [PATCH v3 08/16] tpm: move tpm_validate_commmand() to tpm2-space.c
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (6 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 07/16] tpm: access command header through struct in tpm_try_transmit() Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05 22:36   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 09/16] tpm: encapsulate tpm_dev_transmit() Jarkko Sakkinen
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Move tpm_validate_command() to tpm2-space.c and make it part of the
tpm2_prepare_space() flow. Make cc resolution as part of the TPM space
functionality in order to detach it from rest of the tpm_transmit()
flow.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 72 +++++++-------------------------
 drivers/char/tpm/tpm.h           |  9 ++--
 drivers/char/tpm/tpm2-space.c    | 54 +++++++++++++++++++++---
 3 files changed, 68 insertions(+), 67 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 422e3bb0bd3d..3bf0c51b7b4f 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -62,47 +62,6 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 }
 EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 
-static int tpm_validate_command(struct tpm_chip *chip,
-				 struct tpm_space *space,
-				 const u8 *cmd,
-				 size_t len)
-{
-	const struct tpm_input_header *header = (const void *)cmd;
-	int i;
-	u32 cc;
-	u32 attrs;
-	unsigned int nr_handles;
-
-	if (len < TPM_HEADER_SIZE)
-		return -EINVAL;
-
-	if (!space)
-		return 0;
-
-	if (chip->flags & TPM_CHIP_FLAG_TPM2 && chip->nr_commands) {
-		cc = be32_to_cpu(header->ordinal);
-
-		i = tpm2_find_cc(chip, cc);
-		if (i < 0) {
-			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
-				cc);
-			return -EOPNOTSUPP;
-		}
-
-		attrs = chip->cc_attrs_tbl[i];
-		nr_handles =
-			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
-		if (len < TPM_HEADER_SIZE + 4 * nr_handles)
-			goto err_len;
-	}
-
-	return 0;
-err_len:
-	dev_dbg(&chip->dev,
-		"%s: insufficient command length %zu", __func__, len);
-	return -EINVAL;
-}
-
 static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
 {
 	int rc;
@@ -172,20 +131,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 	u32 count, ordinal;
 	unsigned long stop;
 
-	rc = tpm_validate_command(chip, space, buf, bufsiz);
-	if (rc == -EINVAL)
-		return rc;
-	/*
-	 * If the command is not implemented by the TPM, synthesize a
-	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
-	 */
-	if (rc == -EOPNOTSUPP) {
-		header->length = cpu_to_be32(sizeof(*header));
-		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
-		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE |
-						  TSS2_RESMGR_TPM_RC_LAYER);
-		return sizeof(*header);
-	}
+	if (bufsiz < TPM_HEADER_SIZE)
+		return -EINVAL;
 
 	if (bufsiz > TPM_BUFSIZE)
 		bufsiz = TPM_BUFSIZE;
@@ -200,7 +147,18 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 		return -E2BIG;
 	}
 
-	rc = tpm2_prepare_space(chip, space, ordinal, buf);
+	rc = tpm2_prepare_space(chip, space, buf, bufsiz);
+	/*
+	 * If the command is not implemented by the TPM, synthesize a
+	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
+	 */
+	if (rc == -EOPNOTSUPP) {
+		header->length = cpu_to_be32(sizeof(*header));
+		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE |
+						  TSS2_RESMGR_TPM_RC_LAYER);
+		return sizeof(*header);
+	}
 	if (rc)
 		return rc;
 
@@ -251,7 +209,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 	if (rc)
 		tpm2_flush_space(chip);
 	else
-		rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
+		rc = tpm2_commit_space(chip, space, buf, &len);
 
 	return rc ? rc : len;
 }
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 229ac42b644e..8503dd261897 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -264,6 +264,7 @@ struct tpm_chip {
 #endif /* CONFIG_ACPI */
 
 	struct tpm_space work_space;
+	u32 last_cc;
 	u32 nr_commands;
 	u32 *cc_attrs_tbl;
 
@@ -580,10 +581,10 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
 int tpm2_init_space(struct tpm_space *space);
 void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
 void tpm2_flush_space(struct tpm_chip *chip);
-int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
-		       u8 *cmd);
-int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
-		      u32 cc, u8 *buf, size_t *bufsiz);
+int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u8 *cmd,
+		       size_t cmdsiz);
+int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, u8 *buf,
+		       size_t *bufsiz);
 
 int tpm_bios_log_setup(struct tpm_chip *chip);
 void tpm_bios_log_teardown(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 3d5f9577e5de..20c295fadd50 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -264,14 +264,55 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd)
 	return 0;
 }
 
-int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
-		       u8 *cmd)
+static int tpm_validate_command(struct tpm_chip *chip, struct tpm_space *space,
+				const u8 *cmd, size_t len)
+{
+	const struct tpm_input_header *header = (const void *)cmd;
+	int i;
+	u32 cc;
+	u32 attrs;
+	unsigned int nr_handles;
+
+	if (len < TPM_HEADER_SIZE)
+		return -EINVAL;
+
+	if (chip->nr_commands) {
+		cc = be32_to_cpu(header->ordinal);
+
+		i = tpm2_find_cc(chip, cc);
+		if (i < 0) {
+			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
+				cc);
+			return -EOPNOTSUPP;
+		}
+
+		attrs = chip->cc_attrs_tbl[i];
+		nr_handles =
+			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
+		if (len < TPM_HEADER_SIZE + 4 * nr_handles)
+			goto err_len;
+	}
+
+	return cc;
+err_len:
+	dev_dbg(&chip->dev, "%s: insufficient command length %zu", __func__,
+		len);
+	return -EINVAL;
+}
+
+int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u8 *cmd,
+		       size_t cmdsiz)
 {
 	int rc;
+	int cc;
 
 	if (!space)
 		return 0;
 
+	cc = tpm_validate_command(chip, space, cmd, cmdsiz);
+	if (cc < 0)
+		return cc;
+
 	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
 	       sizeof(space->context_tbl));
 	memcpy(&chip->work_space.session_tbl, &space->session_tbl,
@@ -291,6 +332,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 		return rc;
 	}
 
+	chip->last_cc = cc;
 	return 0;
 }
 
@@ -489,8 +531,8 @@ static int tpm2_save_space(struct tpm_chip *chip)
 	return 0;
 }
 
-int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
-		      u32 cc, u8 *buf, size_t *bufsiz)
+int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, u8 *buf,
+		      size_t *bufsiz)
 {
 	struct tpm_output_header *header = (void *)buf;
 	int rc;
@@ -498,13 +540,13 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 	if (!space)
 		return 0;
 
-	rc = tpm2_map_response_header(chip, cc, buf, *bufsiz);
+	rc = tpm2_map_response_header(chip, chip->last_cc, buf, *bufsiz);
 	if (rc) {
 		tpm2_flush_space(chip);
 		goto out;
 	}
 
-	rc = tpm2_map_response_body(chip, cc, buf, *bufsiz);
+	rc = tpm2_map_response_body(chip, chip->last_cc, buf, *bufsiz);
 	if (rc) {
 		tpm2_flush_space(chip);
 		goto out;
-- 
2.19.1


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

* [PATCH v3 09/16] tpm: encapsulate tpm_dev_transmit()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (7 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 08/16] tpm: move tpm_validate_commmand() to tpm2-space.c Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-06 15:17   ` Stefan Berger
  2018-11-05  1:45 ` [PATCH v3 10/16] tpm: move TPM space code out of tpm_transmit() Jarkko Sakkinen
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Encapsulate tpm_transmit() call pattern to tpm_dev_transmit() because it
is identically used from two places. Use unlocked version of
tpm_transmit() so that we are able to move the calls to
tpm2_prepare_space() and tpm2_commit_space() later on to this new
function.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-dev-common.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 99b5133a9d05..cbb0ee30b511 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -27,7 +27,19 @@
 static struct workqueue_struct *tpm_dev_wq;
 static DEFINE_MUTEX(tpm_dev_wq_lock);
 
-static void tpm_async_work(struct work_struct *work)
+static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
+				u8 *buf, size_t bufsiz)
+{
+	ssize_t ret;
+
+	mutex_lock(&chip->tpm_mutex);
+	ret = tpm_transmit(chip, space, buf, bufsiz, TPM_TRANSMIT_UNLOCKED);
+	mutex_unlock(&chip->tpm_mutex);
+
+	return ret;
+}
+
+static void tpm_dev_async_work(struct work_struct *work)
 {
 	struct file_priv *priv =
 			container_of(work, struct file_priv, async_work);
@@ -35,9 +47,8 @@ static void tpm_async_work(struct work_struct *work)
 
 	mutex_lock(&priv->buffer_mutex);
 	priv->command_enqueued = false;
-	ret = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
-			   sizeof(priv->data_buffer), 0);
-
+	ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer,
+			       sizeof(priv->data_buffer));
 	tpm_put_ops(priv->chip);
 	if (ret > 0) {
 		priv->data_pending = ret;
@@ -78,7 +89,7 @@ void tpm_common_open(struct file *file, struct tpm_chip *chip,
 	mutex_init(&priv->buffer_mutex);
 	timer_setup(&priv->user_read_timer, user_reader_timeout, 0);
 	INIT_WORK(&priv->timeout_work, tpm_timeout_work);
-	INIT_WORK(&priv->async_work, tpm_async_work);
+	INIT_WORK(&priv->async_work, tpm_dev_async_work);
 	init_waitqueue_head(&priv->async_wait);
 	file->private_data = priv;
 }
@@ -163,8 +174,8 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
 		return size;
 	}
 
-	ret = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
-			   sizeof(priv->data_buffer), 0);
+	ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer,
+			       sizeof(priv->data_buffer));
 	tpm_put_ops(priv->chip);
 
 	if (ret > 0) {
-- 
2.19.1


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

* [PATCH v3 10/16] tpm: move TPM space code out of tpm_transmit()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (8 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 09/16] tpm: encapsulate tpm_dev_transmit() Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 12/16] tpm: use tpm_try_get_ops() in tpm-sysfs.c Jarkko Sakkinen
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Prepare and commit TPM space before and after calling tpm_transmit()
instead of doing that inside tpm_transmit(). After this change we can
remove TPM_TRANSMIT_NESTED flag from tpm2_prepare_space() and
tpm2_commit_space() and replace it with TPM_TRANSMIT_UNLOCKED.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-dev-common.c | 30 ++++++++++++++++++++++++++----
 drivers/char/tpm/tpm-interface.c  | 29 +++--------------------------
 drivers/char/tpm/tpm2-space.c     | 12 ++++++------
 3 files changed, 35 insertions(+), 36 deletions(-)

diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index cbb0ee30b511..87b0d0b37bc0 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -30,13 +30,35 @@ static DEFINE_MUTEX(tpm_dev_wq_lock);
 static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
 				u8 *buf, size_t bufsiz)
 {
-	ssize_t ret;
+	struct tpm_output_header *header = (void *)buf;
+	ssize_t ret, len;
 
 	mutex_lock(&chip->tpm_mutex);
-	ret = tpm_transmit(chip, space, buf, bufsiz, TPM_TRANSMIT_UNLOCKED);
-	mutex_unlock(&chip->tpm_mutex);
+	ret = tpm2_prepare_space(chip, space, buf, bufsiz);
+	/* If the command is not implemented by the TPM, synthesize a
+	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
+	 */
+	if (ret == -EOPNOTSUPP) {
+		header->length = cpu_to_be32(sizeof(*header));
+		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
+		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE |
+						  TSS2_RESMGR_TPM_RC_LAYER);
+		ret = sizeof(*header);
+	}
+	if (ret)
+		goto out_lock;
 
-	return ret;
+	len = tpm_transmit(chip, space, buf, bufsiz, TPM_TRANSMIT_UNLOCKED);
+	if (len < 0)
+		ret = len;
+
+	if (ret)
+		tpm2_flush_space(chip);
+	else
+		ret = tpm2_commit_space(chip, space, buf, &len);
+out_lock:
+	mutex_unlock(&chip->tpm_mutex);
+	return ret ? ret : len;
 }
 
 static void tpm_dev_async_work(struct work_struct *work)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 3bf0c51b7b4f..50cc46c6bfff 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -147,27 +147,12 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 		return -E2BIG;
 	}
 
-	rc = tpm2_prepare_space(chip, space, buf, bufsiz);
-	/*
-	 * If the command is not implemented by the TPM, synthesize a
-	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
-	 */
-	if (rc == -EOPNOTSUPP) {
-		header->length = cpu_to_be32(sizeof(*header));
-		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
-		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE |
-						  TSS2_RESMGR_TPM_RC_LAYER);
-		return sizeof(*header);
-	}
-	if (rc)
-		return rc;
-
 	rc = chip->ops->send(chip, buf, count);
 	if (rc < 0) {
 		if (rc != -EPIPE)
 			dev_err(&chip->dev,
 				"%s: tpm_send: error %d\n", __func__, rc);
-		goto out_space;
+		return rc;
 	}
 
 	if (chip->flags & TPM_CHIP_FLAG_IRQ)
@@ -182,8 +167,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 
 		if (chip->ops->req_canceled(chip, status)) {
 			dev_err(&chip->dev, "Operation Canceled\n");
-			rc = -ECANCELED;
-			goto out_space;
+			return -ECANCELED;
 		}
 
 		tpm_msleep(TPM_TIMEOUT_POLL);
@@ -192,8 +176,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 
 	chip->ops->cancel(chip);
 	dev_err(&chip->dev, "Operation Timed out\n");
-	rc = -ETIME;
-	goto out_space;
+	return -ETIME;
 
 out_recv:
 	len = chip->ops->recv(chip, buf, bufsiz);
@@ -205,12 +188,6 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
 	} else if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
 		rc = -EFAULT;
 
-out_space:
-	if (rc)
-		tpm2_flush_space(chip);
-	else
-		rc = tpm2_commit_space(chip, space, buf, &len);
-
 	return rc ? rc : len;
 }
 
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 20c295fadd50..942f307eedd6 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -39,7 +39,7 @@ static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space)
 	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
 		if (space->session_tbl[i])
 			tpm2_flush_context_cmd(chip, space->session_tbl[i],
-					       TPM_TRANSMIT_NESTED);
+					       TPM_TRANSMIT_UNLOCKED);
 	}
 }
 
@@ -84,7 +84,7 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
 	tpm_buf_append(&tbuf, &buf[*offset], body_size);
 
 	rc = tpm_transmit_cmd(chip, NULL, &tbuf, 4,
-			      TPM_TRANSMIT_NESTED, NULL);
+			      TPM_TRANSMIT_UNLOCKED, NULL);
 	if (rc < 0) {
 		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
 			 __func__, rc);
@@ -133,7 +133,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 	tpm_buf_append_u32(&tbuf, handle);
 
 	rc = tpm_transmit_cmd(chip, NULL, &tbuf, 0,
-			      TPM_TRANSMIT_NESTED, NULL);
+			      TPM_TRANSMIT_UNLOCKED, NULL);
 	if (rc < 0) {
 		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
 			 __func__, rc);
@@ -170,7 +170,7 @@ void tpm2_flush_space(struct tpm_chip *chip)
 	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
 		if (space->context_tbl[i] && ~space->context_tbl[i])
 			tpm2_flush_context_cmd(chip, space->context_tbl[i],
-					       TPM_TRANSMIT_NESTED);
+					       TPM_TRANSMIT_UNLOCKED);
 
 	tpm2_flush_sessions(chip, space);
 }
@@ -419,7 +419,7 @@ static int tpm2_map_response_header(struct tpm_chip *chip, u32 cc, u8 *rsp,
 
 	return 0;
 out_no_slots:
-	tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_NESTED);
+	tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_UNLOCKED);
 	dev_warn(&chip->dev, "%s: out of slots for 0x%08X\n", __func__,
 		 phandle);
 	return -ENOMEM;
@@ -507,7 +507,7 @@ static int tpm2_save_space(struct tpm_chip *chip)
 			return rc;
 
 		tpm2_flush_context_cmd(chip, space->context_tbl[i],
-				       TPM_TRANSMIT_NESTED);
+				       TPM_TRANSMIT_UNLOCKED);
 		space->context_tbl[i] = ~0;
 	}
 
-- 
2.19.1


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

* [PATCH v3 12/16] tpm: use tpm_try_get_ops() in tpm-sysfs.c.
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (9 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 10/16] tpm: move TPM space code out of tpm_transmit() Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 13/16] tpm: remove TPM_TRANSMIT_UNLOCKED flag Jarkko Sakkinen
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Use tpm_try_get_ops() in tpm-sysfs.c so that we can consider moving
other decorations (locking, localities, power management for example)
inside it. This direction can be of course taken only after other call
sites for tpm_transmit() have been treated in the same way.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-sysfs.c | 135 +++++++++++++++++++++--------------
 1 file changed, 82 insertions(+), 53 deletions(-)

diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 7126b0b04ee6..02521c4d631b 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -39,7 +39,6 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 {
 	struct tpm_buf tpm_buf;
 	struct tpm_readpubek_out *out;
-	ssize_t rc;
 	int i;
 	char *str = buf;
 	struct tpm_chip *chip = to_tpm_chip(dev);
@@ -47,17 +46,17 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 
 	memset(&anti_replay, 0, sizeof(anti_replay));
 
-	if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK))
+	if (tpm_try_get_ops(chip))
 		return 0;
 
+	if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK))
+		goto out_ops;
+
 	tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
 
-	rc = tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE,
-			      0, "attempting to read the PUBEK");
-	if (rc) {
-		tpm_buf_destroy(&tpm_buf);
-		return 0;
-	}
+	if (tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE,
+			      0, "attempting to read the PUBEK"))
+		goto out_buf;
 
 	out = (struct tpm_readpubek_out *)&tpm_buf.data[10];
 	str +=
@@ -88,9 +87,11 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 			str += sprintf(str, "\n");
 	}
 
-	rc = str - buf;
+out_buf:
 	tpm_buf_destroy(&tpm_buf);
-	return rc;
+out_ops:
+	tpm_put_ops(chip);
+	return str - buf;
 }
 static DEVICE_ATTR_RO(pubek);
 
@@ -99,29 +100,31 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
 {
 	cap_t cap;
 	u8 digest[TPM_DIGEST_SIZE];
-	ssize_t rc;
 	u32 i, j, num_pcrs;
 	char *str = buf;
+	ssize_t rc = 0;
 	struct tpm_chip *chip = to_tpm_chip(dev);
 
-	rc = tpm1_getcap(chip, TPM_CAP_PROP_PCR, &cap,
-			 "attempting to determine the number of PCRS",
-			 sizeof(cap.num_pcrs));
-	if (rc)
+	if (tpm_try_get_ops(chip))
 		return 0;
 
+	if (tpm1_getcap(chip, TPM_CAP_PROP_PCR, &cap,
+			"attempting to determine the number of PCRS",
+			sizeof(cap.num_pcrs)))
+		goto out_ops;
+
 	num_pcrs = be32_to_cpu(cap.num_pcrs);
 	for (i = 0; i < num_pcrs; i++) {
-		rc = tpm1_pcr_read(chip, i, digest);
-		if (rc)
-			goto out;
+		if (tpm1_pcr_read(chip, i, digest))
+			goto out_ops;
 		str += sprintf(str, "PCR-%02d: ", i);
 		for (j = 0; j < TPM_DIGEST_SIZE; j++)
 			str += sprintf(str, "%02X ", digest[j]);
 		str += sprintf(str, "\n");
 	}
 	rc = str - buf;
-out:
+out_ops:
+	tpm_put_ops(chip);
 	return rc;
 }
 static DEVICE_ATTR_RO(pcrs);
@@ -129,16 +132,21 @@ static DEVICE_ATTR_RO(pcrs);
 static ssize_t enabled_show(struct device *dev, struct device_attribute *attr,
 		     char *buf)
 {
+	struct tpm_chip *chip = to_tpm_chip(dev);
+	ssize_t rc = 0;
 	cap_t cap;
-	ssize_t rc;
 
-	rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
-			 "attempting to determine the permanent enabled state",
-			 sizeof(cap.perm_flags));
-	if (rc)
+	if (tpm_try_get_ops(chip))
 		return 0;
 
+	if (tpm1_getcap(chip, TPM_CAP_FLAG_PERM, &cap,
+			"attempting to determine the permanent enabled state",
+			sizeof(cap.perm_flags)))
+		goto out_ops;
+
 	rc = sprintf(buf, "%d\n", !cap.perm_flags.disable);
+out_ops:
+	tpm_put_ops(chip);
 	return rc;
 }
 static DEVICE_ATTR_RO(enabled);
@@ -146,16 +154,21 @@ static DEVICE_ATTR_RO(enabled);
 static ssize_t active_show(struct device *dev, struct device_attribute *attr,
 		    char *buf)
 {
+	struct tpm_chip *chip = to_tpm_chip(dev);
+	ssize_t rc = 0;
 	cap_t cap;
-	ssize_t rc;
 
-	rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_PERM, &cap,
-			 "attempting to determine the permanent active state",
-			 sizeof(cap.perm_flags));
-	if (rc)
+	if (tpm_try_get_ops(chip))
 		return 0;
 
+	if (tpm1_getcap(chip, TPM_CAP_FLAG_PERM, &cap,
+			"attempting to determine the permanent active state",
+			sizeof(cap.perm_flags)))
+		goto out_ops;
+
 	rc = sprintf(buf, "%d\n", !cap.perm_flags.deactivated);
+out_ops:
+	tpm_put_ops(chip);
 	return rc;
 }
 static DEVICE_ATTR_RO(active);
@@ -163,16 +176,21 @@ static DEVICE_ATTR_RO(active);
 static ssize_t owned_show(struct device *dev, struct device_attribute *attr,
 			  char *buf)
 {
+	struct tpm_chip *chip = to_tpm_chip(dev);
+	ssize_t rc = 0;
 	cap_t cap;
-	ssize_t rc;
 
-	rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
-			 "attempting to determine the owner state",
-			 sizeof(cap.owned));
-	if (rc)
+	if (tpm_try_get_ops(chip))
 		return 0;
 
+	if (tpm1_getcap(to_tpm_chip(dev), TPM_CAP_PROP_OWNER, &cap,
+			"attempting to determine the owner state",
+			sizeof(cap.owned)))
+		goto out_ops;
+
 	rc = sprintf(buf, "%d\n", cap.owned);
+out_ops:
+	tpm_put_ops(chip);
 	return rc;
 }
 static DEVICE_ATTR_RO(owned);
@@ -180,16 +198,21 @@ static DEVICE_ATTR_RO(owned);
 static ssize_t temp_deactivated_show(struct device *dev,
 				     struct device_attribute *attr, char *buf)
 {
+	struct tpm_chip *chip = to_tpm_chip(dev);
+	ssize_t rc = 0;
 	cap_t cap;
-	ssize_t rc;
 
-	rc = tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
-			 "attempting to determine the temporary state",
-			 sizeof(cap.stclear_flags));
-	if (rc)
+	if (tpm_try_get_ops(chip))
 		return 0;
 
+	if (tpm1_getcap(to_tpm_chip(dev), TPM_CAP_FLAG_VOL, &cap,
+			"attempting to determine the temporary state",
+			sizeof(cap.stclear_flags)))
+		goto out_ops;
+
 	rc = sprintf(buf, "%d\n", cap.stclear_flags.deactivated);
+out_ops:
+	tpm_put_ops(chip);
 	return rc;
 }
 static DEVICE_ATTR_RO(temp_deactivated);
@@ -198,15 +221,18 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	struct tpm_chip *chip = to_tpm_chip(dev);
-	cap_t cap;
-	ssize_t rc;
+	ssize_t rc = 0;
 	char *str = buf;
+	cap_t cap;
 
-	rc = tpm1_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
-			 "attempting to determine the manufacturer",
-			 sizeof(cap.manufacturer_id));
-	if (rc)
+	if (tpm_try_get_ops(chip))
 		return 0;
+
+	if (tpm1_getcap(chip, TPM_CAP_PROP_MANUFACTURER, &cap,
+			"attempting to determine the manufacturer",
+			sizeof(cap.manufacturer_id)))
+		goto out_ops;
+
 	str += sprintf(str, "Manufacturer: 0x%x\n",
 		       be32_to_cpu(cap.manufacturer_id));
 
@@ -223,11 +249,10 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
 			       cap.tpm_version_1_2.revMinor);
 	} else {
 		/* Otherwise just use TPM_STRUCT_VER */
-		rc = tpm1_getcap(chip, TPM_CAP_VERSION_1_1, &cap,
-				 "attempting to determine the 1.1 version",
-				 sizeof(cap.tpm_version));
-		if (rc)
-			return 0;
+		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,
@@ -235,8 +260,10 @@ static ssize_t caps_show(struct device *dev, struct device_attribute *attr,
 			       cap.tpm_version.revMajor,
 			       cap.tpm_version.revMinor);
 	}
-
-	return str - buf;
+	rc = str - buf;
+out_ops:
+	tpm_put_ops(chip);
+	return rc;
 }
 static DEVICE_ATTR_RO(caps);
 
@@ -244,10 +271,12 @@ static ssize_t cancel_store(struct device *dev, struct device_attribute *attr,
 			    const char *buf, size_t count)
 {
 	struct tpm_chip *chip = to_tpm_chip(dev);
-	if (chip == NULL)
+
+	if (tpm_try_get_ops(chip))
 		return 0;
 
 	chip->ops->cancel(chip);
+	tpm_put_ops(chip);
 	return count;
 }
 static DEVICE_ATTR_WO(cancel);
-- 
2.19.1


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

* [PATCH v3 13/16] tpm: remove TPM_TRANSMIT_UNLOCKED flag
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (10 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 12/16] tpm: use tpm_try_get_ops() in tpm-sysfs.c Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 14/16] tpm: introduce tpm_chip_start() and tpm_chip_stop() Jarkko Sakkinen
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Added locking as part of tpm_try_get_ops() and tpm_put_ops() as they are
anyway used in most of the call sites except in tpmrm_release() where we
take the locks manually.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-chip.c       |  2 ++
 drivers/char/tpm/tpm-dev-common.c |  4 +---
 drivers/char/tpm/tpm-interface.c  |  8 --------
 drivers/char/tpm/tpm.h            |  8 ++------
 drivers/char/tpm/tpm2-cmd.c       | 13 ++++---------
 drivers/char/tpm/tpm2-space.c     | 15 ++++++---------
 6 files changed, 15 insertions(+), 35 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 32db84683c40..157505b0f755 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -58,6 +58,7 @@ int tpm_try_get_ops(struct tpm_chip *chip)
 	if (!chip->ops)
 		goto out_lock;
 
+	mutex_lock(&chip->tpm_mutex);
 	return 0;
 out_lock:
 	up_read(&chip->ops_sem);
@@ -75,6 +76,7 @@ EXPORT_SYMBOL_GPL(tpm_try_get_ops);
  */
 void tpm_put_ops(struct tpm_chip *chip)
 {
+	mutex_unlock(&chip->tpm_mutex);
 	up_read(&chip->ops_sem);
 	put_device(&chip->dev);
 }
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index 4f109cdbac45..d09c13d5c12e 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -33,7 +33,6 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	struct tpm_output_header *header = (void *)buf;
 	ssize_t ret, len;
 
-	mutex_lock(&chip->tpm_mutex);
 	ret = tpm2_prepare_space(chip, space, buf, bufsiz);
 	/* If the command is not implemented by the TPM, synthesize a
 	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
@@ -48,7 +47,7 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	if (ret)
 		goto out_lock;
 
-	len = tpm_transmit(chip, buf, bufsiz, TPM_TRANSMIT_UNLOCKED);
+	len = tpm_transmit(chip, buf, bufsiz, 0);
 	if (len < 0)
 		ret = len;
 
@@ -57,7 +56,6 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	else
 		ret = tpm2_commit_space(chip, space, buf, &len);
 out_lock:
-	mutex_unlock(&chip->tpm_mutex);
 	return ret ? ret : len;
 }
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 81b3cbde9901..8456162e1260 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -229,10 +229,6 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 	memcpy(save, buf, save_size);
 
 	for (;;) {
-		if (!(flags & TPM_TRANSMIT_UNLOCKED) &&
-		    !(flags & TPM_TRANSMIT_NESTED))
-			mutex_lock(&chip->tpm_mutex);
-
 		if (chip->ops->clk_enable != NULL)
 			chip->ops->clk_enable(chip, true);
 
@@ -259,10 +255,6 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 		if (chip->ops->clk_enable != NULL)
 			chip->ops->clk_enable(chip, false);
 
-		if (!(flags & TPM_TRANSMIT_UNLOCKED) &&
-		    !(flags & TPM_TRANSMIT_NESTED))
-			mutex_unlock(&chip->tpm_mutex);
-
 		if (ret < 0)
 			break;
 		rc = be32_to_cpu(header->return_code);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 5a9e1aac2b7e..addd699317e4 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -491,14 +491,10 @@ extern struct idr dev_nums_idr;
 /**
  * enum tpm_transmit_flags - flags for tpm_transmit()
  *
- * @TPM_TRANSMIT_UNLOCKED:	do not lock the chip
- * @TPM_TRANSMIT_NESTED:	discard setup steps (power management,
- *				locality) including locking (i.e. implicit
- *				UNLOCKED)
+ * %TPM_TRANSMIT_NESTED:	discard setup steps (power management, locality)
  */
 enum tpm_transmit_flags {
-	TPM_TRANSMIT_UNLOCKED	= BIT(0),
-	TPM_TRANSMIT_NESTED      = BIT(1),
+	TPM_TRANSMIT_NESTED      = BIT(0),
 };
 
 ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 10072da19269..8fdd835ecc42 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -652,17 +652,12 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	u32 blob_handle;
 	int rc;
 
-	mutex_lock(&chip->tpm_mutex);
-	rc = tpm2_load_cmd(chip, payload, options, &blob_handle,
-			   TPM_TRANSMIT_UNLOCKED);
+	rc = tpm2_load_cmd(chip, payload, options, &blob_handle, 0);
 	if (rc)
-		goto out;
+		return rc;
 
-	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle,
-			     TPM_TRANSMIT_UNLOCKED);
-	tpm2_flush_context_cmd(chip, blob_handle, TPM_TRANSMIT_UNLOCKED);
-out:
-	mutex_unlock(&chip->tpm_mutex);
+	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle, 0);
+	tpm2_flush_context_cmd(chip, blob_handle, 0);
 	return rc;
 }
 
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 9c8195ae1cc5..9ced3acdc8f7 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -38,8 +38,7 @@ static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space)
 
 	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
 		if (space->session_tbl[i])
-			tpm2_flush_context_cmd(chip, space->session_tbl[i],
-					       TPM_TRANSMIT_UNLOCKED);
+			tpm2_flush_context_cmd(chip, space->session_tbl[i], 0);
 	}
 }
 
@@ -83,7 +82,7 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
 	body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
 	tpm_buf_append(&tbuf, &buf[*offset], body_size);
 
-	rc = tpm_transmit_cmd(chip, &tbuf, 4, TPM_TRANSMIT_UNLOCKED, NULL);
+	rc = tpm_transmit_cmd(chip, &tbuf, 4, 0, NULL);
 	if (rc < 0) {
 		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
 			 __func__, rc);
@@ -131,7 +130,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 
 	tpm_buf_append_u32(&tbuf, handle);
 
-	rc = tpm_transmit_cmd(chip, &tbuf, 0, TPM_TRANSMIT_UNLOCKED, NULL);
+	rc = tpm_transmit_cmd(chip, &tbuf, 0, 0, NULL);
 	if (rc < 0) {
 		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
 			 __func__, rc);
@@ -167,8 +166,7 @@ void tpm2_flush_space(struct tpm_chip *chip)
 
 	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
 		if (space->context_tbl[i] && ~space->context_tbl[i])
-			tpm2_flush_context_cmd(chip, space->context_tbl[i],
-					       TPM_TRANSMIT_UNLOCKED);
+			tpm2_flush_context_cmd(chip, space->context_tbl[i], 0);
 
 	tpm2_flush_sessions(chip, space);
 }
@@ -417,7 +415,7 @@ static int tpm2_map_response_header(struct tpm_chip *chip, u32 cc, u8 *rsp,
 
 	return 0;
 out_no_slots:
-	tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_UNLOCKED);
+	tpm2_flush_context_cmd(chip, phandle, 0);
 	dev_warn(&chip->dev, "%s: out of slots for 0x%08X\n", __func__,
 		 phandle);
 	return -ENOMEM;
@@ -504,8 +502,7 @@ static int tpm2_save_space(struct tpm_chip *chip)
 		} else if (rc)
 			return rc;
 
-		tpm2_flush_context_cmd(chip, space->context_tbl[i],
-				       TPM_TRANSMIT_UNLOCKED);
+		tpm2_flush_context_cmd(chip, space->context_tbl[i], 0);
 		space->context_tbl[i] = ~0;
 	}
 
-- 
2.19.1


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

* [PATCH v3 14/16] tpm: introduce tpm_chip_start() and tpm_chip_stop()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (11 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 13/16] tpm: remove TPM_TRANSMIT_UNLOCKED flag Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 15/16] tpm: take TPM chip power gating out of tpm_transmit() Jarkko Sakkinen
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Encapsulate power gating and locality functionality to tpm_chip_start()
and tpm_chip_stop() in order to clean up the branching mess in
tpm_transmit().

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-chip.c      | 110 +++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm-interface.c |  87 +-----------------------
 drivers/char/tpm/tpm.h           |   2 +
 3 files changed, 115 insertions(+), 84 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 157505b0f755..65f1561eba81 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -37,6 +37,116 @@ struct class *tpm_class;
 struct class *tpmrm_class;
 dev_t tpm_devt;
 
+static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
+{
+	int rc;
+
+	if (flags & TPM_TRANSMIT_NESTED)
+		return 0;
+
+	if (!chip->ops->request_locality)
+		return 0;
+
+	rc = chip->ops->request_locality(chip, 0);
+	if (rc < 0)
+		return rc;
+
+	chip->locality = rc;
+	return 0;
+}
+
+static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
+{
+	int rc;
+
+	if (flags & TPM_TRANSMIT_NESTED)
+		return;
+
+	if (!chip->ops->relinquish_locality)
+		return;
+
+	rc = chip->ops->relinquish_locality(chip, chip->locality);
+	if (rc)
+		dev_err(&chip->dev, "%s: : error %d\n", __func__, rc);
+
+	chip->locality = -1;
+}
+
+static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
+{
+	if (flags & TPM_TRANSMIT_NESTED)
+		return 0;
+
+	if (!chip->ops->cmd_ready)
+		return 0;
+
+	return chip->ops->cmd_ready(chip);
+}
+
+static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
+{
+	if (flags & TPM_TRANSMIT_NESTED)
+		return 0;
+
+	if (!chip->ops->go_idle)
+		return 0;
+
+	return chip->ops->go_idle(chip);
+}
+
+/**
+ * tpm_chip_start() - power on the TPM
+ * @chip:	a TPM chip to use
+ * @flags:	TPM transmit flags
+ *
+ * Return:
+ * * The response length	- OK
+ * * -errno			- A system error
+ */
+int tpm_chip_start(struct tpm_chip *chip, unsigned int flags)
+{
+	int ret;
+
+	if (chip->ops->clk_enable)
+		chip->ops->clk_enable(chip, true);
+
+	if (chip->locality == -1) {
+		ret = tpm_request_locality(chip, flags);
+		if (ret) {
+			chip->ops->clk_enable(chip, false);
+			return ret;
+		}
+	}
+
+	ret = tpm_cmd_ready(chip, flags);
+	if (ret) {
+		tpm_relinquish_locality(chip, flags);
+		if (chip->ops->clk_enable)
+			chip->ops->clk_enable(chip, false);
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * tpm_chip_stop() - power off the TPM
+ * @chip:	a TPM chip to use
+ * @flags:	TPM transmit flags
+ *
+ * Return:
+ * * The response length	- OK
+ * * -errno			- A system error
+ */
+void tpm_chip_stop(struct tpm_chip *chip, unsigned int flags)
+{
+	tpm_go_idle(chip, flags);
+	tpm_relinquish_locality(chip, flags);
+	if (chip->ops->clk_enable)
+		chip->ops->clk_enable(chip, false);
+}
+
+
 /**
  * tpm_try_get_ops() - Get a ref to the tpm_chip
  * @chip: Chip to ref
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 8456162e1260..866e5405ca46 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -62,64 +62,6 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 }
 EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 
-static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
-{
-	int rc;
-
-	if (flags & TPM_TRANSMIT_NESTED)
-		return 0;
-
-	if (!chip->ops->request_locality)
-		return 0;
-
-	rc = chip->ops->request_locality(chip, 0);
-	if (rc < 0)
-		return rc;
-
-	chip->locality = rc;
-
-	return 0;
-}
-
-static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
-{
-	int rc;
-
-	if (flags & TPM_TRANSMIT_NESTED)
-		return;
-
-	if (!chip->ops->relinquish_locality)
-		return;
-
-	rc = chip->ops->relinquish_locality(chip, chip->locality);
-	if (rc)
-		dev_err(&chip->dev, "%s: : error %d\n", __func__, rc);
-
-	chip->locality = -1;
-}
-
-static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
-{
-	if (flags & TPM_TRANSMIT_NESTED)
-		return 0;
-
-	if (!chip->ops->cmd_ready)
-		return 0;
-
-	return chip->ops->cmd_ready(chip);
-}
-
-static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
-{
-	if (flags & TPM_TRANSMIT_NESTED)
-		return 0;
-
-	if (!chip->ops->go_idle)
-		return 0;
-
-	return chip->ops->go_idle(chip);
-}
-
 static ssize_t tpm_try_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 				unsigned int flags)
 {
@@ -214,7 +156,6 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 	/* space for header and handles */
 	u8 save[TPM_HEADER_SIZE + 3*sizeof(u32)];
 	unsigned int delay_msec = TPM2_DURATION_SHORT;
-	bool has_locality = false;
 	u32 rc = 0;
 	ssize_t ret;
 	const size_t save_size = min(sizeof(save), bufsiz);
@@ -229,34 +170,12 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 	memcpy(save, buf, save_size);
 
 	for (;;) {
-		if (chip->ops->clk_enable != NULL)
-			chip->ops->clk_enable(chip, true);
-
-		if (chip->locality == -1) {
-			ret = tpm_request_locality(chip, flags);
-			if (ret)
-				goto out_locality;
-			has_locality = true;
-		}
-
-		ret = tpm_cmd_ready(chip, flags);
+		ret = tpm_chip_start(chip, flags);
 		if (ret)
-			goto out_locality;
-
+			return ret;
 		ret = tpm_try_transmit(chip, buf, bufsiz, flags);
+		tpm_chip_stop(chip, flags);
 
-		/* This may fail but do not override ret. */
-		tpm_go_idle(chip, flags);
-
-out_locality:
-		if (has_locality)
-			tpm_relinquish_locality(chip, flags);
-
-		if (chip->ops->clk_enable != NULL)
-			chip->ops->clk_enable(chip, false);
-
-		if (ret < 0)
-			break;
 		rc = be32_to_cpu(header->return_code);
 		if (rc != TPM2_RC_RETRY && rc != TPM2_RC_TESTING)
 			break;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index addd699317e4..9d4065804477 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -526,6 +526,8 @@ static inline void tpm_msleep(unsigned int delay_msec)
 		     delay_msec * 1000);
 };
 
+int tpm_chip_start(struct tpm_chip *chip, unsigned int flags);
+void tpm_chip_stop(struct tpm_chip *chip, unsigned int flags);
 struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
 __must_check int tpm_try_get_ops(struct tpm_chip *chip);
 void tpm_put_ops(struct tpm_chip *chip);
-- 
2.19.1


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

* [PATCH v3 15/16] tpm: take TPM chip power gating out of tpm_transmit()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (12 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 14/16] tpm: introduce tpm_chip_start() and tpm_chip_stop() Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
  2018-11-05  1:45 ` [PATCH v3 16/16] tpm: remove @flags from tpm_transmit() Jarkko Sakkinen
       [not found] ` <20181105014552.20262-12-jarkko.sakkinen@linux.intel.com>
  15 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Call tpm_chip_start() and tpm_chip_stop() in

* tpm_try_get_ops() and tpm_put_ops()
* tpm_chip_register()
* tpm2_del_space()

And remove these calls from tpm_transmit(). The core reason for this
change is that in tpm_vtpm_proxy a locality change requires a virtual
TPM command (a command made up just for that driver).

The consequence of this is that this commit removes the remaining nested
calls.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-chip.c       | 19 ++++++-------------
 drivers/char/tpm/tpm-interface.c  |  4 ----
 drivers/char/tpm/tpm.h            |  9 ---------
 drivers/char/tpm/tpm2-space.c     |  5 ++++-
 drivers/char/tpm/tpm_vtpm_proxy.c |  3 +--
 5 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 65f1561eba81..87570182f75e 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -41,9 +41,6 @@ static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
 {
 	int rc;
 
-	if (flags & TPM_TRANSMIT_NESTED)
-		return 0;
-
 	if (!chip->ops->request_locality)
 		return 0;
 
@@ -59,9 +56,6 @@ static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
 {
 	int rc;
 
-	if (flags & TPM_TRANSMIT_NESTED)
-		return;
-
 	if (!chip->ops->relinquish_locality)
 		return;
 
@@ -74,9 +68,6 @@ static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
 
 static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
 {
-	if (flags & TPM_TRANSMIT_NESTED)
-		return 0;
-
 	if (!chip->ops->cmd_ready)
 		return 0;
 
@@ -85,9 +76,6 @@ static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
 
 static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
 {
-	if (flags & TPM_TRANSMIT_NESTED)
-		return 0;
-
 	if (!chip->ops->go_idle)
 		return 0;
 
@@ -169,7 +157,7 @@ int tpm_try_get_ops(struct tpm_chip *chip)
 		goto out_lock;
 
 	mutex_lock(&chip->tpm_mutex);
-	return 0;
+	return tpm_chip_start(chip, 0);
 out_lock:
 	up_read(&chip->ops_sem);
 	put_device(&chip->dev);
@@ -186,6 +174,7 @@ EXPORT_SYMBOL_GPL(tpm_try_get_ops);
  */
 void tpm_put_ops(struct tpm_chip *chip)
 {
+	tpm_chip_stop(chip, 0);
 	mutex_unlock(&chip->tpm_mutex);
 	up_read(&chip->ops_sem);
 	put_device(&chip->dev);
@@ -563,7 +552,11 @@ int tpm_chip_register(struct tpm_chip *chip)
 {
 	int rc;
 
+	rc = tpm_chip_start(chip, 0);
+	if (rc)
+		return rc;
 	rc = tpm_auto_startup(chip);
+	tpm_chip_stop(chip, 0);
 	if (rc)
 		return rc;
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 866e5405ca46..697091f37057 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -170,11 +170,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 	memcpy(save, buf, save_size);
 
 	for (;;) {
-		ret = tpm_chip_start(chip, flags);
-		if (ret)
-			return ret;
 		ret = tpm_try_transmit(chip, buf, bufsiz, flags);
-		tpm_chip_stop(chip, flags);
 
 		rc = be32_to_cpu(header->return_code);
 		if (rc != TPM2_RC_RETRY && rc != TPM2_RC_TESTING)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 9d4065804477..8555fcc1ac0a 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -488,15 +488,6 @@ extern const struct file_operations tpm_fops;
 extern const struct file_operations tpmrm_fops;
 extern struct idr dev_nums_idr;
 
-/**
- * enum tpm_transmit_flags - flags for tpm_transmit()
- *
- * %TPM_TRANSMIT_NESTED:	discard setup steps (power management, locality)
- */
-enum tpm_transmit_flags {
-	TPM_TRANSMIT_NESTED      = BIT(0),
-};
-
 ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 		     unsigned int flags);
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 9ced3acdc8f7..763c93f4d950 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -60,7 +60,10 @@ int tpm2_init_space(struct tpm_space *space)
 void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
 {
 	mutex_lock(&chip->tpm_mutex);
-	tpm2_flush_sessions(chip, space);
+	if (!tpm_chip_start(chip, 0)) {
+		tpm2_flush_sessions(chip, space);
+		tpm_chip_stop(chip, 0);
+	}
 	mutex_unlock(&chip->tpm_mutex);
 	kfree(space->context_buf);
 	kfree(space->session_buf);
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 2c09bcfd0cc9..bd489d0c3cd7 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -417,8 +417,7 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
 
 	proxy_dev->state |= STATE_DRIVER_COMMAND;
 
-	rc = tpm_transmit_cmd(chip, &buf, 0, TPM_TRANSMIT_NESTED,
-			      "attempting to set locality");
+	rc = tpm_transmit_cmd(chip, &buf, 0, 0, "attempting to set locality");
 
 	proxy_dev->state &= ~STATE_DRIVER_COMMAND;
 
-- 
2.19.1


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

* [PATCH v3 16/16] tpm: remove @flags from tpm_transmit()
  2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
                   ` (13 preceding siblings ...)
  2018-11-05  1:45 ` [PATCH v3 15/16] tpm: take TPM chip power gating out of tpm_transmit() Jarkko Sakkinen
@ 2018-11-05  1:45 ` Jarkko Sakkinen
       [not found] ` <20181105014552.20262-12-jarkko.sakkinen@linux.intel.com>
  15 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-05  1:45 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Jarkko Sakkinen,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

Remove @flags from tpm_transmit() API. It is no longer used for
anything.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-chip.c       | 32 ++++++++++-----------
 drivers/char/tpm/tpm-dev-common.c |  2 +-
 drivers/char/tpm/tpm-interface.c  | 18 ++++--------
 drivers/char/tpm/tpm-sysfs.c      |  2 +-
 drivers/char/tpm/tpm.h            | 13 ++++-----
 drivers/char/tpm/tpm1-cmd.c       | 14 ++++-----
 drivers/char/tpm/tpm2-cmd.c       | 48 ++++++++++++++-----------------
 drivers/char/tpm/tpm2-space.c     | 16 +++++------
 drivers/char/tpm/tpm_vtpm_proxy.c |  2 +-
 9 files changed, 65 insertions(+), 82 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 87570182f75e..46aa68756bac 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -37,7 +37,7 @@ struct class *tpm_class;
 struct class *tpmrm_class;
 dev_t tpm_devt;
 
-static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
+static int tpm_request_locality(struct tpm_chip *chip)
 {
 	int rc;
 
@@ -52,7 +52,7 @@ static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
 	return 0;
 }
 
-static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
+static void tpm_relinquish_locality(struct tpm_chip *chip)
 {
 	int rc;
 
@@ -66,7 +66,7 @@ static void tpm_relinquish_locality(struct tpm_chip *chip, unsigned int flags)
 	chip->locality = -1;
 }
 
-static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
+static int tpm_cmd_ready(struct tpm_chip *chip)
 {
 	if (!chip->ops->cmd_ready)
 		return 0;
@@ -74,7 +74,7 @@ static int tpm_cmd_ready(struct tpm_chip *chip, unsigned int flags)
 	return chip->ops->cmd_ready(chip);
 }
 
-static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
+static int tpm_go_idle(struct tpm_chip *chip)
 {
 	if (!chip->ops->go_idle)
 		return 0;
@@ -85,13 +85,12 @@ static int tpm_go_idle(struct tpm_chip *chip, unsigned int flags)
 /**
  * tpm_chip_start() - power on the TPM
  * @chip:	a TPM chip to use
- * @flags:	TPM transmit flags
  *
  * Return:
  * * The response length	- OK
  * * -errno			- A system error
  */
-int tpm_chip_start(struct tpm_chip *chip, unsigned int flags)
+int tpm_chip_start(struct tpm_chip *chip)
 {
 	int ret;
 
@@ -99,16 +98,16 @@ int tpm_chip_start(struct tpm_chip *chip, unsigned int flags)
 		chip->ops->clk_enable(chip, true);
 
 	if (chip->locality == -1) {
-		ret = tpm_request_locality(chip, flags);
+		ret = tpm_request_locality(chip);
 		if (ret) {
 			chip->ops->clk_enable(chip, false);
 			return ret;
 		}
 	}
 
-	ret = tpm_cmd_ready(chip, flags);
+	ret = tpm_cmd_ready(chip);
 	if (ret) {
-		tpm_relinquish_locality(chip, flags);
+		tpm_relinquish_locality(chip);
 		if (chip->ops->clk_enable)
 			chip->ops->clk_enable(chip, false);
 		return ret;
@@ -120,16 +119,15 @@ int tpm_chip_start(struct tpm_chip *chip, unsigned int flags)
 /**
  * tpm_chip_stop() - power off the TPM
  * @chip:	a TPM chip to use
- * @flags:	TPM transmit flags
  *
  * Return:
  * * The response length	- OK
  * * -errno			- A system error
  */
-void tpm_chip_stop(struct tpm_chip *chip, unsigned int flags)
+void tpm_chip_stop(struct tpm_chip *chip)
 {
-	tpm_go_idle(chip, flags);
-	tpm_relinquish_locality(chip, flags);
+	tpm_go_idle(chip);
+	tpm_relinquish_locality(chip);
 	if (chip->ops->clk_enable)
 		chip->ops->clk_enable(chip, false);
 }
@@ -157,7 +155,7 @@ int tpm_try_get_ops(struct tpm_chip *chip)
 		goto out_lock;
 
 	mutex_lock(&chip->tpm_mutex);
-	return tpm_chip_start(chip, 0);
+	return tpm_chip_start(chip);
 out_lock:
 	up_read(&chip->ops_sem);
 	put_device(&chip->dev);
@@ -174,7 +172,7 @@ EXPORT_SYMBOL_GPL(tpm_try_get_ops);
  */
 void tpm_put_ops(struct tpm_chip *chip)
 {
-	tpm_chip_stop(chip, 0);
+	tpm_chip_stop(chip);
 	mutex_unlock(&chip->tpm_mutex);
 	up_read(&chip->ops_sem);
 	put_device(&chip->dev);
@@ -552,11 +550,11 @@ int tpm_chip_register(struct tpm_chip *chip)
 {
 	int rc;
 
-	rc = tpm_chip_start(chip, 0);
+	rc = tpm_chip_start(chip);
 	if (rc)
 		return rc;
 	rc = tpm_auto_startup(chip);
-	tpm_chip_stop(chip, 0);
+	tpm_chip_stop(chip);
 	if (rc)
 		return rc;
 
diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
index d09c13d5c12e..1ed2d62a79ae 100644
--- a/drivers/char/tpm/tpm-dev-common.c
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -47,7 +47,7 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
 	if (ret)
 		goto out_lock;
 
-	len = tpm_transmit(chip, buf, bufsiz, 0);
+	len = tpm_transmit(chip, buf, bufsiz);
 	if (len < 0)
 		ret = len;
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 697091f37057..669ba1587294 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -62,8 +62,7 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
 }
 EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 
-static ssize_t tpm_try_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
-				unsigned int flags)
+static ssize_t tpm_try_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz)
 {
 	struct tpm_output_header *header = (void *)buf;
 	int rc;
@@ -136,7 +135,6 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
  * @chip:	a TPM chip to use
  * @buf:	a TPM command buffer
  * @bufsiz:	length of the TPM command buffer
- * @flags:	TPM transmit flags
  *
  * A wrapper around tpm_try_transmit() that handles TPM2_RC_RETRY returns from
  * the TPM and retransmits the command after a delay up to a maximum wait of
@@ -149,8 +147,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
  * * The response length	- OK
  * * -errno			- A system error
  */
-ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
-		     unsigned int flags)
+ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz)
 {
 	struct tpm_output_header *header = (struct tpm_output_header *)buf;
 	/* space for header and handles */
@@ -170,7 +167,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
 	memcpy(save, buf, save_size);
 
 	for (;;) {
-		ret = tpm_try_transmit(chip, buf, bufsiz, flags);
+		ret = tpm_try_transmit(chip, buf, bufsiz);
 
 		rc = be32_to_cpu(header->return_code);
 		if (rc != TPM2_RC_RETRY && rc != TPM2_RC_TESTING)
@@ -202,7 +199,6 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
  * @chip:			a TPM chip to use
  * @buf:			a TPM command buffer
  * @min_rsp_body_length:	minimum expected length of response body
- * @flags:			TPM transmit flags
  * @desc:			command description used in the error message
  *
  * Return:
@@ -211,15 +207,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
  * * TPM_RC	- A TPM error
  */
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
-			 size_t min_rsp_body_length, unsigned int flags,
-			 const char *desc)
+			 size_t min_rsp_body_length, const char *desc)
 {
 	const struct tpm_output_header *header =
 		(struct tpm_output_header *)buf->data;
 	int err;
 	ssize_t len;
 
-	len = tpm_transmit(chip, buf->data, PAGE_SIZE, flags);
+	len = tpm_transmit(chip, buf->data, PAGE_SIZE);
 	if (len <  0)
 		return len;
 
@@ -368,8 +363,7 @@ int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
 		goto out;
 
 	memcpy(buf.data, cmd, buflen);
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0,
-			      "attempting to a send a command");
+	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to a send a command");
 	tpm_buf_destroy(&buf);
 out:
 	tpm_put_ops(chip);
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 02521c4d631b..e3acd69481ce 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -55,7 +55,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 	tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
 
 	if (tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE,
-			      0, "attempting to read the PUBEK"))
+			     "attempting to read the PUBEK"))
 		goto out_buf;
 
 	out = (struct tpm_readpubek_out *)&tpm_buf.data[10];
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8555fcc1ac0a..541c06fa982e 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -488,11 +488,9 @@ extern const struct file_operations tpm_fops;
 extern const struct file_operations tpmrm_fops;
 extern struct idr dev_nums_idr;
 
-ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz,
-		     unsigned int flags);
+ssize_t tpm_transmit(struct tpm_chip *chip, u8 *buf, size_t bufsiz);
 ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_buf *buf,
-			 size_t min_rsp_body_length, unsigned int flags,
-			 const char *desc);
+			 size_t min_rsp_body_length, const char *desc);
 int tpm_get_timeouts(struct tpm_chip *);
 int tpm_auto_startup(struct tpm_chip *chip);
 
@@ -517,8 +515,8 @@ static inline void tpm_msleep(unsigned int delay_msec)
 		     delay_msec * 1000);
 };
 
-int tpm_chip_start(struct tpm_chip *chip, unsigned int flags);
-void tpm_chip_stop(struct tpm_chip *chip, unsigned int flags);
+int tpm_chip_start(struct tpm_chip *chip);
+void tpm_chip_stop(struct tpm_chip *chip);
 struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
 __must_check int tpm_try_get_ops(struct tpm_chip *chip);
 void tpm_put_ops(struct tpm_chip *chip);
@@ -551,8 +549,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
 		    struct tpm2_digest *digests);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
-void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
-			    unsigned int flags);
+void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
 int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options);
diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
index 162c255dd131..438c14106e19 100644
--- a/drivers/char/tpm/tpm1-cmd.c
+++ b/drivers/char/tpm/tpm1-cmd.c
@@ -334,7 +334,7 @@ static int tpm1_startup(struct tpm_chip *chip)
 
 	tpm_buf_append_u16(&buf, TPM_ST_CLEAR);
 
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0, "attempting to start the TPM");
+	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM");
 	tpm_buf_destroy(&buf);
 	return rc;
 }
@@ -459,7 +459,7 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
 	tpm_buf_append_u32(&buf, pcr_idx);
 	tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, 0, log_msg);
+	rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, log_msg);
 	tpm_buf_destroy(&buf);
 	return rc;
 }
@@ -489,7 +489,7 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		tpm_buf_append_u32(&buf, 4);
 		tpm_buf_append_u32(&buf, subcap_id);
 	}
-	rc = tpm_transmit_cmd(chip, &buf, min_cap_length, 0, desc);
+	rc = tpm_transmit_cmd(chip, &buf, min_cap_length, desc);
 	if (!rc)
 		*cap = *(cap_t *)&buf.data[TPM_HEADER_SIZE + 4];
 	tpm_buf_destroy(&buf);
@@ -530,7 +530,7 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 	do {
 		tpm_buf_append_u32(&buf, num_bytes);
 
-		rc = tpm_transmit_cmd(chip, &buf, sizeof(out->rng_data_len), 0,
+		rc = tpm_transmit_cmd(chip, &buf, sizeof(out->rng_data_len),
 				      "attempting get random");
 		if (rc)
 			goto out;
@@ -575,7 +575,7 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
 
 	tpm_buf_append_u32(&buf, pcr_idx);
 
-	rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, 0,
+	rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE,
 			      "attempting to read a pcr value");
 	if (rc)
 		goto out;
@@ -609,7 +609,7 @@ static int tpm1_continue_selftest(struct tpm_chip *chip)
 	if (rc)
 		return rc;
 
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0, "continue selftest");
+	rc = tpm_transmit_cmd(chip, &buf, 0, "continue selftest");
 	tpm_buf_destroy(&buf);
 	return rc;
 }
@@ -735,7 +735,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
 		return rc;
 	/* now do the actual savestate */
 	for (try = 0; try < TPM_RETRY; try++) {
-		rc = tpm_transmit_cmd(chip, &buf, 0, 0, NULL);
+		rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
 		/*
 		 * If the TPM indicates that it is too busy to respond to
 		 * this command then retry before giving up.  It can take
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 8fdd835ecc42..cdb5f5d5d2a1 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -197,7 +197,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
 	tpm_buf_append(&buf, (const unsigned char *)pcr_select,
 		       sizeof(pcr_select));
 
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0, res_buf ?
+	rc = tpm_transmit_cmd(chip, &buf, 0, res_buf ?
 			      "attempting to read a pcr value" : NULL);
 	if (rc == 0 && res_buf) {
 		out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
@@ -264,8 +264,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
 		}
 	}
 
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0,
-			      "attempting extend a PCR value");
+	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value");
 
 	tpm_buf_destroy(&buf);
 
@@ -312,7 +311,7 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 		err = tpm_transmit_cmd(chip, &buf,
 				       offsetof(struct tpm2_get_random_out,
 						buffer),
-				       0, "attempting get random");
+				       "attempting get random");
 		if (err)
 			goto out;
 
@@ -341,14 +340,11 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 }
 
 /**
- * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
+ * tpm2_flush_context() - execute a TPM2_FlushContext command
  * @chip:	TPM chip to use
  * @handle:	context handle
- * @flags:	tpm transmit flags - bitmap
- *
  */
-void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
-			    unsigned int flags)
+void tpm2_flush_context(struct tpm_chip *chip, u32 handle)
 {
 	struct tpm_buf buf;
 	int rc;
@@ -362,7 +358,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 
 	tpm_buf_append_u32(&buf, handle);
 
-	tpm_transmit_cmd(chip, &buf, 0, flags, "flushing context");
+	tpm_transmit_cmd(chip, &buf, 0, "flushing context");
 	tpm_buf_destroy(&buf);
 }
 
@@ -476,7 +472,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, &buf, 4, 0, "sealing data");
+	rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data");
 	if (rc)
 		goto out;
 
@@ -513,7 +509,6 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
  * @payload: the key data in clear and encrypted form
  * @options: authentication values and other options
  * @blob_handle: returned blob handle
- * @flags: tpm transmit flags
  *
  * Return: 0 on success.
  *        -E2BIG on wrong payload size.
@@ -523,7 +518,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 static int tpm2_load_cmd(struct tpm_chip *chip,
 			 struct trusted_key_payload *payload,
 			 struct trusted_key_options *options,
-			 u32 *blob_handle, unsigned int flags)
+			 u32 *blob_handle)
 {
 	struct tpm_buf buf;
 	unsigned int private_len;
@@ -558,7 +553,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, &buf, 4, flags, "loading blob");
+	rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob");
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -579,7 +574,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
  * @payload: the key data in clear and encrypted form
  * @options: authentication values and other options
  * @blob_handle: blob handle
- * @flags: tpm_transmit_cmd flags
  *
  * Return: 0 on success
  *         -EPERM on tpm error status
@@ -588,7 +582,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			   struct trusted_key_payload *payload,
 			   struct trusted_key_options *options,
-			   u32 blob_handle, unsigned int flags)
+			   u32 blob_handle)
 {
 	struct tpm_buf buf;
 	u16 data_len;
@@ -608,7 +602,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, &buf, 6, flags, "unsealing");
+	rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing");
 	if (rc > 0)
 		rc = -EPERM;
 
@@ -652,12 +646,12 @@ int tpm2_unseal_trusted(struct tpm_chip *chip,
 	u32 blob_handle;
 	int rc;
 
-	rc = tpm2_load_cmd(chip, payload, options, &blob_handle, 0);
+	rc = tpm2_load_cmd(chip, payload, options, &blob_handle);
 	if (rc)
 		return rc;
 
-	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle, 0);
-	tpm2_flush_context_cmd(chip, blob_handle, 0);
+	rc = tpm2_unseal_cmd(chip, payload, options, blob_handle);
+	tpm2_flush_context(chip, blob_handle);
 	return rc;
 }
 
@@ -693,7 +687,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
 	tpm_buf_append_u32(&buf, property_id);
 	tpm_buf_append_u32(&buf, 1);
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0, NULL);
+	rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
 	if (!rc) {
 		out = (struct tpm2_get_cap_out *)
 			&buf.data[TPM_HEADER_SIZE];
@@ -723,7 +717,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 	if (rc)
 		return;
 	tpm_buf_append_u16(&buf, shutdown_type);
-	tpm_transmit_cmd(chip, &buf, 0, 0, "stopping the TPM");
+	tpm_transmit_cmd(chip, &buf, 0, "stopping the TPM");
 	tpm_buf_destroy(&buf);
 }
 
@@ -752,7 +746,7 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
 			return rc;
 
 		tpm_buf_append_u8(&buf, full);
-		rc = tpm_transmit_cmd(chip, &buf, 0, 0,
+		rc = tpm_transmit_cmd(chip, &buf, 0,
 				      "attempting the self test");
 		tpm_buf_destroy(&buf);
 
@@ -789,7 +783,7 @@ int tpm2_probe(struct tpm_chip *chip)
 	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
 	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
 	tpm_buf_append_u32(&buf, 1);
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0, NULL);
+	rc = tpm_transmit_cmd(chip, &buf, 0, NULL);
 	/* We ignore TPM return codes on purpose. */
 	if (rc >=  0) {
 		out = (struct tpm_output_header *)buf.data;
@@ -828,7 +822,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 	tpm_buf_append_u32(&buf, 0);
 	tpm_buf_append_u32(&buf, 1);
 
-	rc = tpm_transmit_cmd(chip, &buf, 9, 0, "get tpm pcr allocation");
+	rc = tpm_transmit_cmd(chip, &buf, 9, "get tpm pcr allocation");
 	if (rc)
 		goto out;
 
@@ -899,7 +893,7 @@ static int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
 	tpm_buf_append_u32(&buf, TPM2_CC_FIRST);
 	tpm_buf_append_u32(&buf, nr_commands);
 
-	rc = tpm_transmit_cmd(chip, &buf, 9 + 4 * nr_commands, 0, NULL);
+	rc = tpm_transmit_cmd(chip, &buf, 9 + 4 * nr_commands, NULL);
 	if (rc) {
 		tpm_buf_destroy(&buf);
 		goto out;
@@ -956,7 +950,7 @@ static int tpm2_startup(struct tpm_chip *chip)
 		return rc;
 
 	tpm_buf_append_u16(&buf, TPM2_SU_CLEAR);
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0, "attempting to start the TPM");
+	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM");
 	tpm_buf_destroy(&buf);
 
 	return rc;
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 763c93f4d950..cd88e700aed7 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -38,7 +38,7 @@ static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space)
 
 	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
 		if (space->session_tbl[i])
-			tpm2_flush_context_cmd(chip, space->session_tbl[i], 0);
+			tpm2_flush_context(chip, space->session_tbl[i]);
 	}
 }
 
@@ -60,9 +60,9 @@ int tpm2_init_space(struct tpm_space *space)
 void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
 {
 	mutex_lock(&chip->tpm_mutex);
-	if (!tpm_chip_start(chip, 0)) {
+	if (!tpm_chip_start(chip)) {
 		tpm2_flush_sessions(chip, space);
-		tpm_chip_stop(chip, 0);
+		tpm_chip_stop(chip);
 	}
 	mutex_unlock(&chip->tpm_mutex);
 	kfree(space->context_buf);
@@ -85,7 +85,7 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
 	body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
 	tpm_buf_append(&tbuf, &buf[*offset], body_size);
 
-	rc = tpm_transmit_cmd(chip, &tbuf, 4, 0, NULL);
+	rc = tpm_transmit_cmd(chip, &tbuf, 4, NULL);
 	if (rc < 0) {
 		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
 			 __func__, rc);
@@ -133,7 +133,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 
 	tpm_buf_append_u32(&tbuf, handle);
 
-	rc = tpm_transmit_cmd(chip, &tbuf, 0, 0, NULL);
+	rc = tpm_transmit_cmd(chip, &tbuf, 0, NULL);
 	if (rc < 0) {
 		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
 			 __func__, rc);
@@ -169,7 +169,7 @@ void tpm2_flush_space(struct tpm_chip *chip)
 
 	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
 		if (space->context_tbl[i] && ~space->context_tbl[i])
-			tpm2_flush_context_cmd(chip, space->context_tbl[i], 0);
+			tpm2_flush_context(chip, space->context_tbl[i]);
 
 	tpm2_flush_sessions(chip, space);
 }
@@ -418,7 +418,7 @@ static int tpm2_map_response_header(struct tpm_chip *chip, u32 cc, u8 *rsp,
 
 	return 0;
 out_no_slots:
-	tpm2_flush_context_cmd(chip, phandle, 0);
+	tpm2_flush_context(chip, phandle);
 	dev_warn(&chip->dev, "%s: out of slots for 0x%08X\n", __func__,
 		 phandle);
 	return -ENOMEM;
@@ -505,7 +505,7 @@ static int tpm2_save_space(struct tpm_chip *chip)
 		} else if (rc)
 			return rc;
 
-		tpm2_flush_context_cmd(chip, space->context_tbl[i], 0);
+		tpm2_flush_context(chip, space->context_tbl[i]);
 		space->context_tbl[i] = ~0;
 	}
 
diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index bd489d0c3cd7..82e867eb3ad5 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -417,7 +417,7 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
 
 	proxy_dev->state |= STATE_DRIVER_COMMAND;
 
-	rc = tpm_transmit_cmd(chip, &buf, 0, 0, "attempting to set locality");
+	rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to set locality");
 
 	proxy_dev->state &= ~STATE_DRIVER_COMMAND;
 
-- 
2.19.1


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

* Re: [PATCH v3 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter
  2018-11-05  1:45 ` [PATCH v3 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter Jarkko Sakkinen
@ 2018-11-05 21:48   ` Stefan Berger
  2018-11-06  5:52     ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2018-11-05 21:48 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Since we pass an initialized struct tpm_buf instance in every call site
> now, it is cleaner to pass that directly to the tpm_transmit_cmd() as
> the TPM command/response buffer.
>
> Fine-tune a little bit tpm_transmit() and tpm_transmit_cmd() comments
> while doing this.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm-interface.c  | 67 +++++++++++++++++--------------
>   drivers/char/tpm/tpm-sysfs.c      |  2 +-
>   drivers/char/tpm/tpm.h            |  5 +--
>   drivers/char/tpm/tpm1-cmd.c       | 26 ++++--------
>   drivers/char/tpm/tpm2-cmd.c       | 37 +++++++----------
>   drivers/char/tpm/tpm2-space.c     |  4 +-
>   drivers/char/tpm/tpm_vtpm_proxy.c |  3 +-
>   7 files changed, 64 insertions(+), 80 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index d9439f9abe78..64510ed81b46 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -298,23 +298,22 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>
>   /**
>    * tpm_transmit - Internal kernel interface to transmit TPM commands.
> + * @chip:	a TPM chip to use
> + * @space:	a TPM space
> + * @buf:	a TPM command buffer
> + * @bufsiz:	length of the TPM command buffer
> + * @flags:	TPM transmit flags
>    *
> - * @chip: TPM chip to use
> - * @space: tpm space
> - * @buf: TPM command buffer
> - * @bufsiz: length of the TPM command buffer
> - * @flags: tpm transmit flags - bitmap
> + * A wrapper around tpm_try_transmit() that handles TPM2_RC_RETRY returns from
> + * the TPM and retransmits the command after a delay up to a maximum wait of
> + * TPM2_DURATION_LONG.
>    *
> - * A wrapper around tpm_try_transmit that handles TPM2_RC_RETRY
> - * returns from the TPM and retransmits the command after a delay up
> - * to a maximum wait of TPM2_DURATION_LONG.
> - *
> - * Note: TPM1 never returns TPM2_RC_RETRY so the retry logic is TPM2
> - * only
> + * Note that TPM 1.x never returns TPM2_RC_RETRY so the retry logic is TPM 2.0
> + * only.
>    *
>    * Return:
> - *     the length of the return when the operation is successful.
> - *     A negative number for system errors (errno).
> + * * The response length	- OK
> + * * -errno			- A system error
>    */
>   ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>   		     u8 *buf, size_t bufsiz, unsigned int flags)
> @@ -365,33 +364,31 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>   	}
>   	return ret;
>   }
> +
>   /**
>    * tpm_transmit_cmd - send a tpm command to the device
> - *    The function extracts tpm out header return code
> - *
> - * @chip: TPM chip to use
> - * @space: tpm space
> - * @buf: TPM command buffer
> - * @bufsiz: length of the buffer
> - * @min_rsp_body_length: minimum expected length of response body
> - * @flags: tpm transmit flags - bitmap
> - * @desc: command description used in the error message
> + * @chip:			a TPM chip to use
> + * @space:			a TPM space
> + * @buf:			a TPM command buffer
> + * @min_rsp_body_length:	minimum expected length of response body
> + * @flags:			TPM transmit flags
> + * @desc:			command description used in the error message
>    *
>    * Return:
> - *     0 when the operation is successful.
> - *     A negative number for system errors (errno).
> - *     A positive number for a TPM error.
> + * * 0		- OK
> + * * -errno	- A system error
> + * * TPM_RC	- A TPM error
>    */
>   ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> -			 void *buf, size_t bufsiz,
> -			 size_t min_rsp_body_length, unsigned int flags,
> -			 const char *desc)
> +			 struct tpm_buf *buf, size_t min_rsp_body_length,
> +			 unsigned int flags, const char *desc)
>   {
> -	const struct tpm_output_header *header = buf;
> +	const struct tpm_output_header *header =
> +		(struct tpm_output_header *)buf->data;
>   	int err;
>   	ssize_t len;
>
> -	len = tpm_transmit(chip, space, buf, bufsiz, flags);
> +	len = tpm_transmit(chip, space, buf->data, PAGE_SIZE, flags);
>   	if (len <  0)
>   		return len;
>
> @@ -528,14 +525,22 @@ EXPORT_SYMBOL_GPL(tpm_pcr_extend);
>    */
>   int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
>   {
> +	struct tpm_buf buf;
>   	int rc;
>
>   	chip = tpm_find_get_ops(chip);
>   	if (!chip)
>   		return -ENODEV;
>
> -	rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
> +	rc = tpm_buf_init(&buf, 0, 0);
> +	if (rc)
> +		goto out;
> +
> +	memcpy(buf.data, cmd, buflen);


Nit: buflen -> cmd_len


> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
>   			      "attempting to a send a command");
> +	tpm_buf_destroy(&buf);
> +out:
>   	tpm_put_ops(chip);
>   	return rc;
>   }
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index b88e08ec2c59..c2769e55cb6c 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -53,7 +53,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
>
>   	tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
>
> -	rc = tpm_transmit_cmd(chip, NULL, tpm_buf.data, PAGE_SIZE,
> +	rc = tpm_transmit_cmd(chip, NULL, &tpm_buf,
>   			      READ_PUBEK_RESULT_MIN_BODY_SIZE, 0,
>   			      "attempting to read the PUBEK");
>   	if (rc) {
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index f27d1f38a93d..49bca4d1e786 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -503,9 +503,8 @@ enum tpm_transmit_flags {
>   ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>   		     u8 *buf, size_t bufsiz, unsigned int flags);
>   ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> -			 void *buf, size_t bufsiz,
> -			 size_t min_rsp_body_length, unsigned int flags,
> -			 const char *desc);
> +			 struct tpm_buf *buf, size_t min_rsp_body_length,
> +			 unsigned int flags, const char *desc);
>   int tpm_get_timeouts(struct tpm_chip *);
>   int tpm_auto_startup(struct tpm_chip *chip);
>
> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
> index 6f306338953b..f19b7c1ff800 100644
> --- a/drivers/char/tpm/tpm1-cmd.c
> +++ b/drivers/char/tpm/tpm1-cmd.c
> @@ -334,11 +334,9 @@ static int tpm1_startup(struct tpm_chip *chip)
>
>   	tpm_buf_append_u16(&buf, TPM_ST_CLEAR);
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
>   			      "attempting to start the TPM");
> -
>   	tpm_buf_destroy(&buf);
> -
>   	return rc;
>   }
>
> @@ -462,9 +460,7 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash,
>   	tpm_buf_append_u32(&buf, pcr_idx);
>   	tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE);
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> -			      TPM_DIGEST_SIZE, 0, log_msg);
> -
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, TPM_DIGEST_SIZE, 0, log_msg);
>   	tpm_buf_destroy(&buf);
>   	return rc;
>   }
> @@ -494,11 +490,9 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>   		tpm_buf_append_u32(&buf, 4);
>   		tpm_buf_append_u32(&buf, subcap_id);
>   	}
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> -			      min_cap_length, 0, desc);
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, min_cap_length, 0, desc);
>   	if (!rc)
>   		*cap = *(cap_t *)&buf.data[TPM_HEADER_SIZE + 4];
> -
>   	tpm_buf_destroy(&buf);
>   	return rc;
>   }
> @@ -537,7 +531,7 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>   	do {
>   		tpm_buf_append_u32(&buf, num_bytes);
>
> -		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> +		rc = tpm_transmit_cmd(chip, NULL, &buf,
>   				      sizeof(out->rng_data_len), 0,
>   				      "attempting get random");
>   		if (rc)
> @@ -583,8 +577,7 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
>
>   	tpm_buf_append_u32(&buf, pcr_idx);
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> -			      TPM_DIGEST_SIZE, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, TPM_DIGEST_SIZE, 0,
>   			      "attempting to read a pcr value");
>   	if (rc)
>   		goto out;
> @@ -618,11 +611,8 @@ static int tpm1_continue_selftest(struct tpm_chip *chip)
>   	if (rc)
>   		return rc;
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> -			      0, 0, "continue selftest");
> -
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, "continue selftest");
>   	tpm_buf_destroy(&buf);
> -
>   	return rc;
>   }
>
> @@ -747,9 +737,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr)
>   		return rc;
>   	/* now do the actual savestate */
>   	for (try = 0; try < TPM_RETRY; try++) {
> -		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> -				      0, 0, NULL);
> -
> +		rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, NULL);
>   		/*
>   		 * If the TPM indicates that it is too busy to respond to
>   		 * this command then retry before giving up.  It can take
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index a6bec13afa69..2bcf470c8e5d 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -197,8 +197,8 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf)
>   	tpm_buf_append(&buf, (const unsigned char *)pcr_select,
>   		       sizeof(pcr_select));
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> -			res_buf ? "attempting to read a pcr value" : NULL);
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, res_buf ?
> +			      "attempting to read a pcr value" : NULL);
>   	if (rc == 0 && res_buf) {
>   		out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE];
>   		memcpy(res_buf, out->digest, SHA1_DIGEST_SIZE);
> @@ -264,7 +264,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, u32 count,
>   		}
>   	}
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
>   			      "attempting extend a PCR value");
>
>   	tpm_buf_destroy(&buf);
> @@ -309,7 +309,7 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>   	do {
>   		tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
>   		tpm_buf_append_u16(&buf, num_bytes);
> -		err = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> +		err = tpm_transmit_cmd(chip, NULL, &buf,
>   				       offsetof(struct tpm2_get_random_out,
>   						buffer),
>   				       0, "attempting get random");
> @@ -362,9 +362,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>
>   	tpm_buf_append_u32(&buf, handle);
>
> -	(void) tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, flags,
> -				"flushing context");
> -
> +	tpm_transmit_cmd(chip, NULL, &buf, 0, flags, "flushing context");
>   	tpm_buf_destroy(&buf);
>   }
>
> @@ -478,8 +476,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>   		goto out;
>   	}
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
> -			      "sealing data");
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 4, 0, "sealing data");
>   	if (rc)
>   		goto out;
>
> @@ -561,8 +558,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>   		goto out;
>   	}
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, flags,
> -			      "loading blob");
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 4, flags, "loading blob");
>   	if (!rc)
>   		*blob_handle = be32_to_cpup(
>   			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
> @@ -612,8 +608,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>   			     options->blobauth /* hmac */,
>   			     TPM_DIGEST_SIZE);
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 6, flags,
> -			      "unsealing");
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 6, flags, "unsealing");
>   	if (rc > 0)
>   		rc = -EPERM;
>
> @@ -703,7 +698,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
>   	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
>   	tpm_buf_append_u32(&buf, property_id);
>   	tpm_buf_append_u32(&buf, 1);
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, NULL);
>   	if (!rc) {
>   		out = (struct tpm2_get_cap_out *)
>   			&buf.data[TPM_HEADER_SIZE];
> @@ -733,8 +728,7 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
>   	if (rc)
>   		return;
>   	tpm_buf_append_u16(&buf, shutdown_type);
> -	tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> -			 "stopping the TPM");
> +	tpm_transmit_cmd(chip, NULL, &buf, 0, 0, "stopping the TPM");
>   	tpm_buf_destroy(&buf);
>   }
>
> @@ -763,7 +757,7 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
>   			return rc;
>
>   		tpm_buf_append_u8(&buf, full);
> -		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> +		rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
>   				      "attempting the self test");
>   		tpm_buf_destroy(&buf);
>
> @@ -800,7 +794,7 @@ int tpm2_probe(struct tpm_chip *chip)
>   	tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES);
>   	tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS);
>   	tpm_buf_append_u32(&buf, 1);
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0, NULL);
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0, NULL);
>   	/* We ignore TPM return codes on purpose. */
>   	if (rc >=  0) {
>   		out = (struct tpm_output_header *)buf.data;
> @@ -839,7 +833,7 @@ static ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>   	tpm_buf_append_u32(&buf, 0);
>   	tpm_buf_append_u32(&buf, 1);
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 9, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 9, 0,
>   			      "get tpm pcr allocation");
>   	if (rc)
>   		goto out;
> @@ -911,8 +905,7 @@ static int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
>   	tpm_buf_append_u32(&buf, TPM2_CC_FIRST);
>   	tpm_buf_append_u32(&buf, nr_commands);
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> -			      9 + 4 * nr_commands, 0, NULL);
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 9 + 4 * nr_commands, 0, NULL);
>   	if (rc) {
>   		tpm_buf_destroy(&buf);
>   		goto out;
> @@ -969,7 +962,7 @@ static int tpm2_startup(struct tpm_chip *chip)
>   		return rc;
>
>   	tpm_buf_append_u16(&buf, TPM2_SU_CLEAR);
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, 0,
>   			      "attempting to start the TPM");
>   	tpm_buf_destroy(&buf);
>
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index dcdfde3c253e..1131a8e7b79b 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -83,7 +83,7 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
>   	body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
>   	tpm_buf_append(&tbuf, &buf[*offset], body_size);
>
> -	rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 4,
> +	rc = tpm_transmit_cmd(chip, NULL, &tbuf, 4,
>   			      TPM_TRANSMIT_NESTED, NULL);
>   	if (rc < 0) {
>   		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
> @@ -132,7 +132,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
>
>   	tpm_buf_append_u32(&tbuf, handle);
>
> -	rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &tbuf, 0,
>   			      TPM_TRANSMIT_NESTED, NULL);
>   	if (rc < 0) {
>   		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
> diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
> index 87a0ce47f201..5f95fbfb7f6b 100644
> --- a/drivers/char/tpm/tpm_vtpm_proxy.c
> +++ b/drivers/char/tpm/tpm_vtpm_proxy.c
> @@ -417,8 +417,7 @@ static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality)
>
>   	proxy_dev->state |= STATE_DRIVER_COMMAND;
>
> -	rc = tpm_transmit_cmd(chip, NULL, buf.data, tpm_buf_length(&buf), 0,
> -			      TPM_TRANSMIT_NESTED,
> +	rc = tpm_transmit_cmd(chip, NULL, &buf, 0, TPM_TRANSMIT_NESTED,
>   			      "attempting to set locality");
>
>   	proxy_dev->state &= ~STATE_DRIVER_COMMAND;


Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>



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

* Re: [PATCH v3 02/16] tpm: fix invalid return value in pubek_show()
  2018-11-05  1:45 ` [PATCH v3 02/16] tpm: fix invalid return value in pubek_show() Jarkko Sakkinen
@ 2018-11-05 21:51   ` Stefan Berger
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Berger @ 2018-11-05 21:51 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Return zero when tpm_buf_init() fails as we do for other functions in
> tpm-sysfs.c.
>
> Fixes: da379f3c1db0c ("tpm: migrate pubek_show to struct tpm_buf")
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>


> ---
>   drivers/char/tpm/tpm-sysfs.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index c2769e55cb6c..7ed7eb6f906a 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -47,9 +47,8 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
>
>   	memset(&anti_replay, 0, sizeof(anti_replay));
>
> -	rc = tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK);
> -	if (rc)
> -		return rc;
> +	if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK))
> +		return 0;
>
>   	tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay));
>


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

* Re: [PATCH v3 03/16] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails
  2018-11-05  1:45 ` [PATCH v3 03/16] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails Jarkko Sakkinen
@ 2018-11-05 21:54   ` Stefan Berger
  2018-11-06  5:54     ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2018-11-05 21:54 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Do not print partial list of PCRs when tpm1_pcr_read() fails but instead
> return 0 from pcrs_show(). This is consistent behavior with other sysfs
> functions.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm-sysfs.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 7ed7eb6f906a..d3b05c7526c8 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -115,13 +115,15 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
>   	for (i = 0; i < num_pcrs; i++) {
>   		rc = tpm1_pcr_read(chip, i, digest);
>   		if (rc)
> -			break;
> +			goto out;

Previous patch said "Return zero when tpm_buf_init() fails as we do for 
other functions in tpm-sysfs.c.". Why not return 0 in this case as well?


>   		str += sprintf(str, "PCR-%02d: ", i);
>   		for (j = 0; j < TPM_DIGEST_SIZE; j++)
>   			str += sprintf(str, "%02X ", digest[j]);
>   		str += sprintf(str, "\n");
>   	}
> -	return str - buf;
> +	rc = str - buf;
> +out:
> +	return rc;
>   }
>   static DEVICE_ATTR_RO(pcrs);
>


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

* Re: [PATCH v3 04/16] tpm: call tpm2_flush_space() on error in tpm_try_transmit()
  2018-11-05  1:45 ` [PATCH v3 04/16] tpm: call tpm2_flush_space() on error in tpm_try_transmit() Jarkko Sakkinen
@ 2018-11-05 22:01   ` Stefan Berger
  2018-11-06  5:55     ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2018-11-05 22:01 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, stable, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Always call tpm2_flush_space() on failure in tpm_try_transmit() so that
> the volatile memory of the TPM gets cleared. If /dev/tpm0 does not have
> sufficient permissions (usually it has), this could leak to the leakage

leak ->lead


> of TPM objects. Through /dev/tpmrm0 this issue does not raise new
> security concerns.
>
> Cc: stable@vger.kernel.org
> Fixes: 745b361e989a ("tpm:tpm: infrastructure for TPM spaces")
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++-----------
>   drivers/char/tpm/tpm.h           |  1 +
>   drivers/char/tpm/tpm2-space.c    |  2 +-
>   3 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 64510ed81b46..ecda6f96cde0 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -224,14 +224,14 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>
>   	rc = tpm2_prepare_space(chip, space, ordinal, buf);
>   	if (rc)
> -		goto out;
> +		goto out_idle;
>
>   	rc = chip->ops->send(chip, buf, count);
>   	if (rc < 0) {
>   		if (rc != -EPIPE)
>   			dev_err(&chip->dev,
>   				"%s: tpm_send: error %d\n", __func__, rc);
> -		goto out;
> +		goto out_space;
>   	}
>
>   	if (chip->flags & TPM_CHIP_FLAG_IRQ)
> @@ -247,7 +247,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   		if (chip->ops->req_canceled(chip, status)) {
>   			dev_err(&chip->dev, "Operation Canceled\n");
>   			rc = -ECANCELED;
> -			goto out;
> +			goto out_space;
>   		}
>
>   		tpm_msleep(TPM_TIMEOUT_POLL);
> @@ -257,7 +257,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   	chip->ops->cancel(chip);
>   	dev_err(&chip->dev, "Operation Timed out\n");
>   	rc = -ETIME;
> -	goto out;
> +	goto out_space;
>
>   out_recv:
>   	len = chip->ops->recv(chip, buf, bufsiz);
> @@ -265,22 +265,28 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   		rc = len;
>   		dev_err(&chip->dev,
>   			"tpm_transmit: tpm_recv: error %d\n", rc);
> -		goto out;
> +		goto out_idle;

Why not jump to out_space here and in 2 instances below?


>   	} else if (len < TPM_HEADER_SIZE) {
>   		rc = -EFAULT;
> -		goto out;
> +		goto out_idle;
>   	}
>
>   	if (len != be32_to_cpu(header->length)) {
>   		rc = -EFAULT;
> -		goto out;
> +		goto out_idle;
>   	}
>
> -	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
> -	if (rc)
> -		dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
> +out_space:
> +	if (rc) {
> +		tpm2_flush_space(chip);
> +	} else {
> +		rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
> +		if (rc)
> +			dev_err(&chip->dev, "tpm2_commit_space: error %d\n",
> +				rc);
> +	}
>
> -out:
> +out_idle:
>   	/* may fail but do not override previous error value in rc */
>   	tpm_go_idle(chip, flags);
>
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 49bca4d1e786..229ac42b644e 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -579,6 +579,7 @@ int tpm2_probe(struct tpm_chip *chip);
>   int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
>   int tpm2_init_space(struct tpm_space *space);
>   void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
> +void tpm2_flush_space(struct tpm_chip *chip);
>   int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
>   		       u8 *cmd);
>   int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 1131a8e7b79b..d53c882268ff 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -162,7 +162,7 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
>   	return 0;
>   }
>
> -static void tpm2_flush_space(struct tpm_chip *chip)
> +void tpm2_flush_space(struct tpm_chip *chip)
>   {
>   	struct tpm_space *space = &chip->work_space;
>   	int i;



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

* Re: [PATCH v3 05/16] tpm: print tpm2_commit_space() error inside tpm2_commit_space()
  2018-11-05  1:45 ` [PATCH v3 05/16] tpm: print tpm2_commit_space() error inside tpm2_commit_space() Jarkko Sakkinen
@ 2018-11-05 22:04   ` Stefan Berger
  2018-11-06  5:58     ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2018-11-05 22:04 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> The error logging for tpm2_commit_space() is in a wrong place. This
> commit moves it inside that function.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm2-space.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index d53c882268ff..3d5f9577e5de 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -501,19 +501,19 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
>   	rc = tpm2_map_response_header(chip, cc, buf, *bufsiz);
>   	if (rc) {
>   		tpm2_flush_space(chip);
> -		return rc;
> +		goto out;
>   	}
>
>   	rc = tpm2_map_response_body(chip, cc, buf, *bufsiz);
>   	if (rc) {
>   		tpm2_flush_space(chip);
> -		return rc;
> +		goto out;
>   	}
>
>   	rc = tpm2_save_space(chip);
>   	if (rc) {
>   		tpm2_flush_space(chip);
> -		return rc;
> +		goto out;
>   	}
>
>   	*bufsiz = be32_to_cpu(header->length);
> @@ -526,4 +526,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
>   	memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE);
>
>   	return 0;
> +out:
> +	dev_err(&chip->dev, "%s: error %d\n", __func__, rc);
> +	return rc;
>   }


This is in tpm-interface.c:

     rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
     if (rc)
         dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);

I suppose you will remove if(rc) ... ?


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

* Re: [PATCH v3 06/16] tpm: clean up tpm_try_transmit() error handling flow
  2018-11-05  1:45 ` [PATCH v3 06/16] tpm: clean up tpm_try_transmit() error handling flow Jarkko Sakkinen
@ 2018-11-05 22:20   ` Stefan Berger
  2018-11-06  6:01     ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2018-11-05 22:20 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Move locking, locality handling and power management to tpm_transmit()
> in order to simplify the flow.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm-interface.c | 90 ++++++++++++++------------------
>   1 file changed, 39 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index ecda6f96cde0..0f343407daf8 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -171,7 +171,6 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   	ssize_t len = 0;
>   	u32 count, ordinal;
>   	unsigned long stop;
> -	bool need_locality;
>
>   	rc = tpm_validate_command(chip, space, buf, bufsiz);
>   	if (rc == -EINVAL)
> @@ -201,30 +200,9 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   		return -E2BIG;
>   	}
>
> -	if (!(flags & TPM_TRANSMIT_UNLOCKED) && !(flags & TPM_TRANSMIT_NESTED))
> -		mutex_lock(&chip->tpm_mutex);
> -
> -	if (chip->ops->clk_enable != NULL)
> -		chip->ops->clk_enable(chip, true);
> -
> -	/* Store the decision as chip->locality will be changed. */
> -	need_locality = chip->locality == -1;
> -
> -	if (need_locality) {
> -		rc = tpm_request_locality(chip, flags);
> -		if (rc < 0) {
> -			need_locality = false;
> -			goto out_locality;
> -		}
> -	}
> -
> -	rc = tpm_cmd_ready(chip, flags);
> -	if (rc)
> -		goto out_locality;
> -
>   	rc = tpm2_prepare_space(chip, space, ordinal, buf);


tpm2_prepare_space() may issue TPM commands itself. Following your tree 
the move should now put the clk_enable() and tpm_request_locality() in 
the path. So, looks good.


>   	if (rc)
> -		goto out_idle;
> +		return rc;
>
>   	rc = chip->ops->send(chip, buf, count);
>   	if (rc < 0) {
> @@ -265,40 +243,16 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   		rc = len;
>   		dev_err(&chip->dev,
>   			"tpm_transmit: tpm_recv: error %d\n", rc);
> -		goto out_idle;
> -	} else if (len < TPM_HEADER_SIZE) {
> -		rc = -EFAULT;
> -		goto out_idle;
> -	}
> -
> -	if (len != be32_to_cpu(header->length)) {
> +		tpm2_flush_space(chip);
> +	} else if (len < TPM_HEADER_SIZE || len != be32_to_cpu(header->length))
>   		rc = -EFAULT;
> -		goto out_idle;
> -	}
>
>   out_space:
> -	if (rc) {
> +	if (rc)
>   		tpm2_flush_space(chip);
> -	} else {
> +	else
>   		rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
> -		if (rc)
> -			dev_err(&chip->dev, "tpm2_commit_space: error %d\n",
> -				rc);


This should have been removed in the previous patch...


> -	}
> -
> -out_idle:
> -	/* may fail but do not override previous error value in rc */
> -	tpm_go_idle(chip, flags);
> -
> -out_locality:
> -	if (need_locality)
> -		tpm_relinquish_locality(chip, flags);
> -
> -	if (chip->ops->clk_enable != NULL)
> -		chip->ops->clk_enable(chip, false);
>
> -	if (!(flags & TPM_TRANSMIT_UNLOCKED) && !(flags & TPM_TRANSMIT_NESTED))
> -		mutex_unlock(&chip->tpm_mutex);
>   	return rc ? rc : len;
>   }
>
> @@ -328,6 +282,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>   	/* space for header and handles */
>   	u8 save[TPM_HEADER_SIZE + 3*sizeof(u32)];
>   	unsigned int delay_msec = TPM2_DURATION_SHORT;
> +	bool has_locality = false;
>   	u32 rc = 0;
>   	ssize_t ret;
>   	const size_t save_size = min(space ? sizeof(save) : TPM_HEADER_SIZE,
> @@ -343,7 +298,40 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
>   	memcpy(save, buf, save_size);
>
>   	for (;;) {
> +		if (!(flags & TPM_TRANSMIT_UNLOCKED) &&
> +		    !(flags & TPM_TRANSMIT_NESTED))
> +			mutex_lock(&chip->tpm_mutex);
> +
> +		if (chip->ops->clk_enable != NULL)
> +			chip->ops->clk_enable(chip, true);
> +
> +		if (chip->locality == -1) {
> +			ret = tpm_request_locality(chip, flags);
> +			if (ret)
> +				goto out_locality;
> +			has_locality = true;
> +		}
> +
> +		ret = tpm_cmd_ready(chip, flags);
> +		if (ret)
> +			goto out_locality;
> +
>   		ret = tpm_try_transmit(chip, space, buf, bufsiz, flags);
> +
> +		/* This may fail but do not override ret. */
> +		tpm_go_idle(chip, flags);
> +
> +out_locality:
> +		if (has_locality)
> +			tpm_relinquish_locality(chip, flags);

Safer to also put has_locality = false here ?


> +
> +		if (chip->ops->clk_enable != NULL)
> +			chip->ops->clk_enable(chip, false);
> +
> +		if (!(flags & TPM_TRANSMIT_UNLOCKED) &&
> +		    !(flags & TPM_TRANSMIT_NESTED))
> +			mutex_unlock(&chip->tpm_mutex);
> +
>   		if (ret < 0)
>   			break;
>   		rc = be32_to_cpu(header->return_code);



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

* Re: [PATCH v3 07/16] tpm: access command header through struct in tpm_try_transmit()
  2018-11-05  1:45 ` [PATCH v3 07/16] tpm: access command header through struct in tpm_try_transmit() Jarkko Sakkinen
@ 2018-11-05 22:26   ` Stefan Berger
  2018-11-06  6:08     ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2018-11-05 22:26 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Instead of accessing fields of the command header through offsets to
> the raw buffer, it is a better idea to use the header struct pointer
> that is already used elsewhere in the function.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm-interface.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 0f343407daf8..422e3bb0bd3d 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -190,8 +190,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   	if (bufsiz > TPM_BUFSIZE)
>   		bufsiz = TPM_BUFSIZE;
>
> -	count = be32_to_cpu(*((__be32 *) (buf + 2)));
> -	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> +	count = be32_to_cpu(header->length);
> +	ordinal = be32_to_cpu(header->return_code);

Hm. This should use the proper type of header and use in_header->ordinal.


>   	if (count == 0)
>   		return -ENODATA;
>   	if (count > bufsiz) {



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

* Re: [PATCH v3 08/16] tpm: move tpm_validate_commmand() to tpm2-space.c
  2018-11-05  1:45 ` [PATCH v3 08/16] tpm: move tpm_validate_commmand() to tpm2-space.c Jarkko Sakkinen
@ 2018-11-05 22:36   ` Stefan Berger
  2018-11-06  6:10     ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Berger @ 2018-11-05 22:36 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Move tpm_validate_command() to tpm2-space.c and make it part of the
> tpm2_prepare_space() flow. Make cc resolution as part of the TPM space
> functionality in order to detach it from rest of the tpm_transmit()
> flow.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>   drivers/char/tpm/tpm-interface.c | 72 +++++++-------------------------
>   drivers/char/tpm/tpm.h           |  9 ++--
>   drivers/char/tpm/tpm2-space.c    | 54 +++++++++++++++++++++---
>   3 files changed, 68 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 422e3bb0bd3d..3bf0c51b7b4f 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -62,47 +62,6 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
>   }
>   EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
>
> -static int tpm_validate_command(struct tpm_chip *chip,
> -				 struct tpm_space *space,
> -				 const u8 *cmd,
> -				 size_t len)
> -{
> -	const struct tpm_input_header *header = (const void *)cmd;
> -	int i;
> -	u32 cc;
> -	u32 attrs;
> -	unsigned int nr_handles;
> -
> -	if (len < TPM_HEADER_SIZE)
> -		return -EINVAL;
> -
> -	if (!space)
> -		return 0;
> -
> -	if (chip->flags & TPM_CHIP_FLAG_TPM2 && chip->nr_commands) {
> -		cc = be32_to_cpu(header->ordinal);
> -
> -		i = tpm2_find_cc(chip, cc);
> -		if (i < 0) {
> -			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
> -				cc);
> -			return -EOPNOTSUPP;
> -		}
> -
> -		attrs = chip->cc_attrs_tbl[i];
> -		nr_handles =
> -			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
> -		if (len < TPM_HEADER_SIZE + 4 * nr_handles)
> -			goto err_len;
> -	}
> -
> -	return 0;
> -err_len:
> -	dev_dbg(&chip->dev,
> -		"%s: insufficient command length %zu", __func__, len);
> -	return -EINVAL;
> -}
> -
>   static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
>   {
>   	int rc;
> @@ -172,20 +131,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   	u32 count, ordinal;
>   	unsigned long stop;
>
> -	rc = tpm_validate_command(chip, space, buf, bufsiz);
> -	if (rc == -EINVAL)
> -		return rc;
> -	/*
> -	 * If the command is not implemented by the TPM, synthesize a
> -	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
> -	 */
> -	if (rc == -EOPNOTSUPP) {
> -		header->length = cpu_to_be32(sizeof(*header));
> -		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
> -		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE |
> -						  TSS2_RESMGR_TPM_RC_LAYER);
> -		return sizeof(*header);
> -	}
> +	if (bufsiz < TPM_HEADER_SIZE)
> +		return -EINVAL;
>
>   	if (bufsiz > TPM_BUFSIZE)
>   		bufsiz = TPM_BUFSIZE;
> @@ -200,7 +147,18 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   		return -E2BIG;
>   	}
>
> -	rc = tpm2_prepare_space(chip, space, ordinal, buf);
> +	rc = tpm2_prepare_space(chip, space, buf, bufsiz);
> +	/*
> +	 * If the command is not implemented by the TPM, synthesize a
> +	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
> +	 */
> +	if (rc == -EOPNOTSUPP) {
> +		header->length = cpu_to_be32(sizeof(*header));
> +		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
> +		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE |
> +						  TSS2_RESMGR_TPM_RC_LAYER);
> +		return sizeof(*header);
> +	}
>   	if (rc)
>   		return rc;
>
> @@ -251,7 +209,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
>   	if (rc)
>   		tpm2_flush_space(chip);
>   	else
> -		rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
> +		rc = tpm2_commit_space(chip, space, buf, &len);
>
>   	return rc ? rc : len;
>   }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 229ac42b644e..8503dd261897 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -264,6 +264,7 @@ struct tpm_chip {
>   #endif /* CONFIG_ACPI */
>
>   	struct tpm_space work_space;
> +	u32 last_cc;
>   	u32 nr_commands;
>   	u32 *cc_attrs_tbl;
>
> @@ -580,10 +581,10 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
>   int tpm2_init_space(struct tpm_space *space);
>   void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
>   void tpm2_flush_space(struct tpm_chip *chip);
> -int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
> -		       u8 *cmd);
> -int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> -		      u32 cc, u8 *buf, size_t *bufsiz);
> +int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u8 *cmd,
> +		       size_t cmdsiz);
> +int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, u8 *buf,
> +		       size_t *bufsiz);
>
>   int tpm_bios_log_setup(struct tpm_chip *chip);
>   void tpm_bios_log_teardown(struct tpm_chip *chip);
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 3d5f9577e5de..20c295fadd50 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -264,14 +264,55 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd)
>   	return 0;
>   }
>
> -int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
> -		       u8 *cmd)
> +static int tpm_validate_command(struct tpm_chip *chip, struct tpm_space *space,
> +				const u8 *cmd, size_t len)


Nit: len -> cmdsiz (like below)


> +{
> +	const struct tpm_input_header *header = (const void *)cmd;
> +	int i;
> +	u32 cc;
> +	u32 attrs;
> +	unsigned int nr_handles;
> +
> +	if (len < TPM_HEADER_SIZE)
> +		return -EINVAL;
> +
> +	if (chip->nr_commands) {
> +		cc = be32_to_cpu(header->ordinal);
> +
> +		i = tpm2_find_cc(chip, cc);
> +		if (i < 0) {
> +			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
> +				cc);
> +			return -EOPNOTSUPP;
> +		}
> +
> +		attrs = chip->cc_attrs_tbl[i];
> +		nr_handles =
> +			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
> +		if (len < TPM_HEADER_SIZE + 4 * nr_handles)
> +			goto err_len;
> +	}
> +
> +	return cc;
> +err_len:
> +	dev_dbg(&chip->dev, "%s: insufficient command length %zu", __func__,
> +		len);
> +	return -EINVAL;
> +}
> +
> +int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u8 *cmd,
> +		       size_t cmdsiz)
>   {
>   	int rc;
> +	int cc;
>
>   	if (!space)
>   		return 0;
>
> +	cc = tpm_validate_command(chip, space, cmd, cmdsiz);
> +	if (cc < 0)
> +		return cc;
> +
>   	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
>   	       sizeof(space->context_tbl));
>   	memcpy(&chip->work_space.session_tbl, &space->session_tbl,
> @@ -291,6 +332,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
>   		return rc;
>   	}
>
> +	chip->last_cc = cc;
>   	return 0;
>   }
>
> @@ -489,8 +531,8 @@ static int tpm2_save_space(struct tpm_chip *chip)
>   	return 0;
>   }
>
> -int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> -		      u32 cc, u8 *buf, size_t *bufsiz)
> +int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, u8 *buf,
> +		      size_t *bufsiz)
>   {
>   	struct tpm_output_header *header = (void *)buf;
>   	int rc;
> @@ -498,13 +540,13 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
>   	if (!space)
>   		return 0;
>
> -	rc = tpm2_map_response_header(chip, cc, buf, *bufsiz);
> +	rc = tpm2_map_response_header(chip, chip->last_cc, buf, *bufsiz);
>   	if (rc) {
>   		tpm2_flush_space(chip);
>   		goto out;
>   	}
>
> -	rc = tpm2_map_response_body(chip, cc, buf, *bufsiz);
> +	rc = tpm2_map_response_body(chip, chip->last_cc, buf, *bufsiz);
>   	if (rc) {
>   		tpm2_flush_space(chip);
>   		goto out;

Rest looks good.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>


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

* Re: [PATCH v3 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter
  2018-11-05 21:48   ` Stefan Berger
@ 2018-11-06  5:52     ` Jarkko Sakkinen
  0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-06  5:52 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-integrity, linux-security-module, James Bottomley,
	Tomas Winkler, Tadeusz Struk, Stefan Berger, Nayna Jain,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

On Mon, Nov 05, 2018 at 04:48:13PM -0500, Stefan Berger wrote:
> >   int tpm_send(struct tpm_chip *chip, void *cmd, size_t buflen)
> >   {
> > +	struct tpm_buf buf;
> >   	int rc;
> > 
> >   	chip = tpm_find_get_ops(chip);
> >   	if (!chip)
> >   		return -ENODEV;
> > 
> > -	rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
> > +	rc = tpm_buf_init(&buf, 0, 0);
> > +	if (rc)
> > +		goto out;
> > +
> > +	memcpy(buf.data, cmd, buflen);
> 
> 
> Nit: buflen -> cmd_len

Agreed that it should but I try to keep the patch as minimal and
mechanical as I can.

Thank you.

/Jarkko

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

* Re: [PATCH v3 03/16] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails
  2018-11-05 21:54   ` Stefan Berger
@ 2018-11-06  5:54     ` Jarkko Sakkinen
  0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-06  5:54 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-integrity, linux-security-module, James Bottomley,
	Tomas Winkler, Tadeusz Struk, Stefan Berger, Nayna Jain,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

On Mon, Nov 05, 2018 at 04:54:52PM -0500, Stefan Berger wrote:
> On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> > Do not print partial list of PCRs when tpm1_pcr_read() fails but instead
> > return 0 from pcrs_show(). This is consistent behavior with other sysfs
> > functions.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >   drivers/char/tpm/tpm-sysfs.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> > index 7ed7eb6f906a..d3b05c7526c8 100644
> > --- a/drivers/char/tpm/tpm-sysfs.c
> > +++ b/drivers/char/tpm/tpm-sysfs.c
> > @@ -115,13 +115,15 @@ static ssize_t pcrs_show(struct device *dev, struct device_attribute *attr,
> >   	for (i = 0; i < num_pcrs; i++) {
> >   		rc = tpm1_pcr_read(chip, i, digest);
> >   		if (rc)
> > -			break;
> > +			goto out;
> 
> Previous patch said "Return zero when tpm_buf_init() fails as we do for
> other functions in tpm-sysfs.c.". Why not return 0 in this case as well?

Yes, good catch, thank you.

/Jarkko

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

* Re: [PATCH v3 04/16] tpm: call tpm2_flush_space() on error in tpm_try_transmit()
  2018-11-05 22:01   ` Stefan Berger
@ 2018-11-06  5:55     ` Jarkko Sakkinen
  0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-06  5:55 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-integrity, linux-security-module, James Bottomley,
	Tomas Winkler, Tadeusz Struk, Stefan Berger, Nayna Jain, stable,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

On Mon, Nov 05, 2018 at 05:01:46PM -0500, Stefan Berger wrote:
> On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> > Always call tpm2_flush_space() on failure in tpm_try_transmit() so that
> > the volatile memory of the TPM gets cleared. If /dev/tpm0 does not have
> > sufficient permissions (usually it has), this could leak to the leakage
> 
> leak ->lead
> 
> 
> > of TPM objects. Through /dev/tpmrm0 this issue does not raise new
> > security concerns.
> > 
> > Cc: stable@vger.kernel.org
> > Fixes: 745b361e989a ("tpm:tpm: infrastructure for TPM spaces")
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >   drivers/char/tpm/tpm-interface.c | 28 +++++++++++++++++-----------
> >   drivers/char/tpm/tpm.h           |  1 +
> >   drivers/char/tpm/tpm2-space.c    |  2 +-
> >   3 files changed, 19 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index 64510ed81b46..ecda6f96cde0 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -224,14 +224,14 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> > 
> >   	rc = tpm2_prepare_space(chip, space, ordinal, buf);
> >   	if (rc)
> > -		goto out;
> > +		goto out_idle;
> > 
> >   	rc = chip->ops->send(chip, buf, count);
> >   	if (rc < 0) {
> >   		if (rc != -EPIPE)
> >   			dev_err(&chip->dev,
> >   				"%s: tpm_send: error %d\n", __func__, rc);
> > -		goto out;
> > +		goto out_space;
> >   	}
> > 
> >   	if (chip->flags & TPM_CHIP_FLAG_IRQ)
> > @@ -247,7 +247,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> >   		if (chip->ops->req_canceled(chip, status)) {
> >   			dev_err(&chip->dev, "Operation Canceled\n");
> >   			rc = -ECANCELED;
> > -			goto out;
> > +			goto out_space;
> >   		}
> > 
> >   		tpm_msleep(TPM_TIMEOUT_POLL);
> > @@ -257,7 +257,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> >   	chip->ops->cancel(chip);
> >   	dev_err(&chip->dev, "Operation Timed out\n");
> >   	rc = -ETIME;
> > -	goto out;
> > +	goto out_space;
> > 
> >   out_recv:
> >   	len = chip->ops->recv(chip, buf, bufsiz);
> > @@ -265,22 +265,28 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> >   		rc = len;
> >   		dev_err(&chip->dev,
> >   			"tpm_transmit: tpm_recv: error %d\n", rc);
> > -		goto out;
> > +		goto out_idle;
> 
> Why not jump to out_space here and in 2 instances below?

Ugh, should (and meant) to be like that. Thanks!

/Jarkko


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

* Re: [PATCH v3 05/16] tpm: print tpm2_commit_space() error inside tpm2_commit_space()
  2018-11-05 22:04   ` Stefan Berger
@ 2018-11-06  5:58     ` Jarkko Sakkinen
  0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-06  5:58 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-integrity, linux-security-module, James Bottomley,
	Tomas Winkler, Tadeusz Struk, Stefan Berger, Nayna Jain,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

On Mon, Nov 05, 2018 at 05:04:46PM -0500, Stefan Berger wrote:
> On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> > The error logging for tpm2_commit_space() is in a wrong place. This
> > commit moves it inside that function.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >   drivers/char/tpm/tpm2-space.c | 9 ++++++---
> >   1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> > index d53c882268ff..3d5f9577e5de 100644
> > --- a/drivers/char/tpm/tpm2-space.c
> > +++ b/drivers/char/tpm/tpm2-space.c
> > @@ -501,19 +501,19 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> >   	rc = tpm2_map_response_header(chip, cc, buf, *bufsiz);
> >   	if (rc) {
> >   		tpm2_flush_space(chip);
> > -		return rc;
> > +		goto out;
> >   	}
> > 
> >   	rc = tpm2_map_response_body(chip, cc, buf, *bufsiz);
> >   	if (rc) {
> >   		tpm2_flush_space(chip);
> > -		return rc;
> > +		goto out;
> >   	}
> > 
> >   	rc = tpm2_save_space(chip);
> >   	if (rc) {
> >   		tpm2_flush_space(chip);
> > -		return rc;
> > +		goto out;
> >   	}
> > 
> >   	*bufsiz = be32_to_cpu(header->length);
> > @@ -526,4 +526,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> >   	memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE);
> > 
> >   	return 0;
> > +out:
> > +	dev_err(&chip->dev, "%s: error %d\n", __func__, rc);
> > +	return rc;
> >   }
> 
> 
> This is in tpm-interface.c:
> 
>     rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
>     if (rc)
>         dev_err(&chip->dev, "tpm2_commit_space: error %d\n", rc);
> 
> I suppose you will remove if(rc) ... ?

That removal must have gone at some point when rebasing fixups.
Thanks.

/Jarkko

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

* Re: [PATCH v3 06/16] tpm: clean up tpm_try_transmit() error handling flow
  2018-11-05 22:20   ` Stefan Berger
@ 2018-11-06  6:01     ` Jarkko Sakkinen
  0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-06  6:01 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-integrity, linux-security-module, James Bottomley,
	Tomas Winkler, Tadeusz Struk, Stefan Berger, Nayna Jain,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

On Mon, Nov 05, 2018 at 05:20:33PM -0500, Stefan Berger wrote:
> > +out_locality:
> > +		if (has_locality)
> > +			tpm_relinquish_locality(chip, flags);
> 
> Safer to also put has_locality = false here ?

Not really sure why?

/Jarkko

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

* Re: [PATCH v3 07/16] tpm: access command header through struct in tpm_try_transmit()
  2018-11-05 22:26   ` Stefan Berger
@ 2018-11-06  6:08     ` Jarkko Sakkinen
  0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-06  6:08 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-integrity, linux-security-module, James Bottomley,
	Tomas Winkler, Tadeusz Struk, Stefan Berger, Nayna Jain,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

On Mon, Nov 05, 2018 at 05:26:30PM -0500, Stefan Berger wrote:
> On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> > Instead of accessing fields of the command header through offsets to
> > the raw buffer, it is a better idea to use the header struct pointer
> > that is already used elsewhere in the function.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >   drivers/char/tpm/tpm-interface.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index 0f343407daf8..422e3bb0bd3d 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -190,8 +190,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> >   	if (bufsiz > TPM_BUFSIZE)
> >   		bufsiz = TPM_BUFSIZE;
> > 
> > -	count = be32_to_cpu(*((__be32 *) (buf + 2)));
> > -	ordinal = be32_to_cpu(*((__be32 *) (buf + 6)));
> > +	count = be32_to_cpu(header->length);
> > +	ordinal = be32_to_cpu(header->return_code);
> 
> Hm. This should use the proper type of header and use in_header->ordinal.

Well, the fuction has output header already declared. What I could do
as a prequel commit is to take these:

struct tpm_input_header {
	__be16	tag;
	__be32	length;
	__be32	ordinal;
} __packed;

struct tpm_output_header {
	__be16	tag;
	__be32	length;
	__be32	return_code;
} __packed;

and replace them with this:

struct tpm_header {
	__be16	tag;
	__be32	length;
	union {
		__be32 ordinal;
		__be32 return_code;
	};
} __packed;

/Jarkko

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

* Re: [PATCH v3 08/16] tpm: move tpm_validate_commmand() to tpm2-space.c
  2018-11-05 22:36   ` Stefan Berger
@ 2018-11-06  6:10     ` Jarkko Sakkinen
  0 siblings, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2018-11-06  6:10 UTC (permalink / raw)
  To: Stefan Berger
  Cc: linux-integrity, linux-security-module, James Bottomley,
	Tomas Winkler, Tadeusz Struk, Stefan Berger, Nayna Jain,
	Peter Huewe, Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman,
	open list

On Mon, Nov 05, 2018 at 05:36:03PM -0500, Stefan Berger wrote:
> On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> > Move tpm_validate_command() to tpm2-space.c and make it part of the
> > tpm2_prepare_space() flow. Make cc resolution as part of the TPM space
> > functionality in order to detach it from rest of the tpm_transmit()
> > flow.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > ---
> >   drivers/char/tpm/tpm-interface.c | 72 +++++++-------------------------
> >   drivers/char/tpm/tpm.h           |  9 ++--
> >   drivers/char/tpm/tpm2-space.c    | 54 +++++++++++++++++++++---
> >   3 files changed, 68 insertions(+), 67 deletions(-)
> > 
> > diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> > index 422e3bb0bd3d..3bf0c51b7b4f 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -62,47 +62,6 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
> >   }
> >   EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
> > 
> > -static int tpm_validate_command(struct tpm_chip *chip,
> > -				 struct tpm_space *space,
> > -				 const u8 *cmd,
> > -				 size_t len)
> > -{
> > -	const struct tpm_input_header *header = (const void *)cmd;
> > -	int i;
> > -	u32 cc;
> > -	u32 attrs;
> > -	unsigned int nr_handles;
> > -
> > -	if (len < TPM_HEADER_SIZE)
> > -		return -EINVAL;
> > -
> > -	if (!space)
> > -		return 0;
> > -
> > -	if (chip->flags & TPM_CHIP_FLAG_TPM2 && chip->nr_commands) {
> > -		cc = be32_to_cpu(header->ordinal);
> > -
> > -		i = tpm2_find_cc(chip, cc);
> > -		if (i < 0) {
> > -			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
> > -				cc);
> > -			return -EOPNOTSUPP;
> > -		}
> > -
> > -		attrs = chip->cc_attrs_tbl[i];
> > -		nr_handles =
> > -			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
> > -		if (len < TPM_HEADER_SIZE + 4 * nr_handles)
> > -			goto err_len;
> > -	}
> > -
> > -	return 0;
> > -err_len:
> > -	dev_dbg(&chip->dev,
> > -		"%s: insufficient command length %zu", __func__, len);
> > -	return -EINVAL;
> > -}
> > -
> >   static int tpm_request_locality(struct tpm_chip *chip, unsigned int flags)
> >   {
> >   	int rc;
> > @@ -172,20 +131,8 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> >   	u32 count, ordinal;
> >   	unsigned long stop;
> > 
> > -	rc = tpm_validate_command(chip, space, buf, bufsiz);
> > -	if (rc == -EINVAL)
> > -		return rc;
> > -	/*
> > -	 * If the command is not implemented by the TPM, synthesize a
> > -	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
> > -	 */
> > -	if (rc == -EOPNOTSUPP) {
> > -		header->length = cpu_to_be32(sizeof(*header));
> > -		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
> > -		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE |
> > -						  TSS2_RESMGR_TPM_RC_LAYER);
> > -		return sizeof(*header);
> > -	}
> > +	if (bufsiz < TPM_HEADER_SIZE)
> > +		return -EINVAL;
> > 
> >   	if (bufsiz > TPM_BUFSIZE)
> >   		bufsiz = TPM_BUFSIZE;
> > @@ -200,7 +147,18 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> >   		return -E2BIG;
> >   	}
> > 
> > -	rc = tpm2_prepare_space(chip, space, ordinal, buf);
> > +	rc = tpm2_prepare_space(chip, space, buf, bufsiz);
> > +	/*
> > +	 * If the command is not implemented by the TPM, synthesize a
> > +	 * response with a TPM2_RC_COMMAND_CODE return for user-space.
> > +	 */
> > +	if (rc == -EOPNOTSUPP) {
> > +		header->length = cpu_to_be32(sizeof(*header));
> > +		header->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS);
> > +		header->return_code = cpu_to_be32(TPM2_RC_COMMAND_CODE |
> > +						  TSS2_RESMGR_TPM_RC_LAYER);
> > +		return sizeof(*header);
> > +	}
> >   	if (rc)
> >   		return rc;
> > 
> > @@ -251,7 +209,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip,
> >   	if (rc)
> >   		tpm2_flush_space(chip);
> >   	else
> > -		rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
> > +		rc = tpm2_commit_space(chip, space, buf, &len);
> > 
> >   	return rc ? rc : len;
> >   }
> > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> > index 229ac42b644e..8503dd261897 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -264,6 +264,7 @@ struct tpm_chip {
> >   #endif /* CONFIG_ACPI */
> > 
> >   	struct tpm_space work_space;
> > +	u32 last_cc;
> >   	u32 nr_commands;
> >   	u32 *cc_attrs_tbl;
> > 
> > @@ -580,10 +581,10 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
> >   int tpm2_init_space(struct tpm_space *space);
> >   void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
> >   void tpm2_flush_space(struct tpm_chip *chip);
> > -int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
> > -		       u8 *cmd);
> > -int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> > -		      u32 cc, u8 *buf, size_t *bufsiz);
> > +int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u8 *cmd,
> > +		       size_t cmdsiz);
> > +int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, u8 *buf,
> > +		       size_t *bufsiz);
> > 
> >   int tpm_bios_log_setup(struct tpm_chip *chip);
> >   void tpm_bios_log_teardown(struct tpm_chip *chip);
> > diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> > index 3d5f9577e5de..20c295fadd50 100644
> > --- a/drivers/char/tpm/tpm2-space.c
> > +++ b/drivers/char/tpm/tpm2-space.c
> > @@ -264,14 +264,55 @@ static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd)
> >   	return 0;
> >   }
> > 
> > -int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
> > -		       u8 *cmd)
> > +static int tpm_validate_command(struct tpm_chip *chip, struct tpm_space *space,
> > +				const u8 *cmd, size_t len)
> 
> 
> Nit: len -> cmdsiz (like below)
> 
> 
> > +{
> > +	const struct tpm_input_header *header = (const void *)cmd;
> > +	int i;
> > +	u32 cc;
> > +	u32 attrs;
> > +	unsigned int nr_handles;
> > +
> > +	if (len < TPM_HEADER_SIZE)
> > +		return -EINVAL;
> > +
> > +	if (chip->nr_commands) {
> > +		cc = be32_to_cpu(header->ordinal);
> > +
> > +		i = tpm2_find_cc(chip, cc);
> > +		if (i < 0) {
> > +			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
> > +				cc);
> > +			return -EOPNOTSUPP;
> > +		}
> > +
> > +		attrs = chip->cc_attrs_tbl[i];
> > +		nr_handles =
> > +			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
> > +		if (len < TPM_HEADER_SIZE + 4 * nr_handles)
> > +			goto err_len;
> > +	}
> > +
> > +	return cc;
> > +err_len:
> > +	dev_dbg(&chip->dev, "%s: insufficient command length %zu", __func__,
> > +		len);
> > +	return -EINVAL;
> > +}
> > +
> > +int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u8 *cmd,
> > +		       size_t cmdsiz)
> >   {
> >   	int rc;
> > +	int cc;
> > 
> >   	if (!space)
> >   		return 0;
> > 
> > +	cc = tpm_validate_command(chip, space, cmd, cmdsiz);
> > +	if (cc < 0)
> > +		return cc;
> > +
> >   	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
> >   	       sizeof(space->context_tbl));
> >   	memcpy(&chip->work_space.session_tbl, &space->session_tbl,
> > @@ -291,6 +332,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
> >   		return rc;
> >   	}
> > 
> > +	chip->last_cc = cc;
> >   	return 0;
> >   }
> > 
> > @@ -489,8 +531,8 @@ static int tpm2_save_space(struct tpm_chip *chip)
> >   	return 0;
> >   }
> > 
> > -int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> > -		      u32 cc, u8 *buf, size_t *bufsiz)
> > +int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, u8 *buf,
> > +		      size_t *bufsiz)
> >   {
> >   	struct tpm_output_header *header = (void *)buf;
> >   	int rc;
> > @@ -498,13 +540,13 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> >   	if (!space)
> >   		return 0;
> > 
> > -	rc = tpm2_map_response_header(chip, cc, buf, *bufsiz);
> > +	rc = tpm2_map_response_header(chip, chip->last_cc, buf, *bufsiz);
> >   	if (rc) {
> >   		tpm2_flush_space(chip);
> >   		goto out;
> >   	}
> > 
> > -	rc = tpm2_map_response_body(chip, cc, buf, *bufsiz);
> > +	rc = tpm2_map_response_body(chip, chip->last_cc, buf, *bufsiz);
> >   	if (rc) {
> >   		tpm2_flush_space(chip);
> >   		goto out;
> 
> Rest looks good.
> 
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Great, awesome, thank you for all your reviews!

/Jarkko

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

* Re: [PATCH v3 09/16] tpm: encapsulate tpm_dev_transmit()
  2018-11-05  1:45 ` [PATCH v3 09/16] tpm: encapsulate tpm_dev_transmit() Jarkko Sakkinen
@ 2018-11-06 15:17   ` Stefan Berger
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Berger @ 2018-11-06 15:17 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Encapsulate tpm_transmit() call pattern to tpm_dev_transmit() because it
> is identically used from two places. Use unlocked version of
> tpm_transmit() so that we are able to move the calls to
> tpm2_prepare_space() and tpm2_commit_space() later on to this new
> function.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> ---
>   drivers/char/tpm/tpm-dev-common.c | 25 ++++++++++++++++++-------
>   1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> index 99b5133a9d05..cbb0ee30b511 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -27,7 +27,19 @@
>   static struct workqueue_struct *tpm_dev_wq;
>   static DEFINE_MUTEX(tpm_dev_wq_lock);
>
> -static void tpm_async_work(struct work_struct *work)
> +static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
> +				u8 *buf, size_t bufsiz)
> +{
> +	ssize_t ret;
> +
> +	mutex_lock(&chip->tpm_mutex);
> +	ret = tpm_transmit(chip, space, buf, bufsiz, TPM_TRANSMIT_UNLOCKED);
> +	mutex_unlock(&chip->tpm_mutex);
> +
> +	return ret;
> +}
> +
> +static void tpm_dev_async_work(struct work_struct *work)
>   {
>   	struct file_priv *priv =
>   			container_of(work, struct file_priv, async_work);
> @@ -35,9 +47,8 @@ static void tpm_async_work(struct work_struct *work)
>
>   	mutex_lock(&priv->buffer_mutex);
>   	priv->command_enqueued = false;
> -	ret = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
> -			   sizeof(priv->data_buffer), 0);
> -
> +	ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer,
> +			       sizeof(priv->data_buffer));
>   	tpm_put_ops(priv->chip);
>   	if (ret > 0) {
>   		priv->data_pending = ret;
> @@ -78,7 +89,7 @@ void tpm_common_open(struct file *file, struct tpm_chip *chip,
>   	mutex_init(&priv->buffer_mutex);
>   	timer_setup(&priv->user_read_timer, user_reader_timeout, 0);
>   	INIT_WORK(&priv->timeout_work, tpm_timeout_work);
> -	INIT_WORK(&priv->async_work, tpm_async_work);
> +	INIT_WORK(&priv->async_work, tpm_dev_async_work);
>   	init_waitqueue_head(&priv->async_wait);
>   	file->private_data = priv;
>   }
> @@ -163,8 +174,8 @@ ssize_t tpm_common_write(struct file *file, const char __user *buf,
>   		return size;
>   	}
>
> -	ret = tpm_transmit(priv->chip, priv->space, priv->data_buffer,
> -			   sizeof(priv->data_buffer), 0);
> +	ret = tpm_dev_transmit(priv->chip, priv->space, priv->data_buffer,
> +			       sizeof(priv->data_buffer));
>   	tpm_put_ops(priv->chip);
>
>   	if (ret > 0) {



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

* Re: [PATCH v3 11/16] tpm: remove @space from tpm_transmit()
       [not found] ` <20181105014552.20262-12-jarkko.sakkinen@linux.intel.com>
@ 2018-11-06 15:25   ` Stefan Berger
  0 siblings, 0 replies; 33+ messages in thread
From: Stefan Berger @ 2018-11-06 15:25 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: linux-security-module, James Bottomley, Tomas Winkler,
	Tadeusz Struk, Stefan Berger, Nayna Jain, Peter Huewe,
	Jason Gunthorpe, Arnd Bergmann, Greg Kroah-Hartman, open list

On 11/4/18 8:45 PM, Jarkko Sakkinen wrote:
> Remove @space from tpm_transmit() API` in order to completely remove the
> bound between low-level transmission functionality and TPM spaces. The
> only real dependency existing is the amount of data saved before trying
> to send a command to the TPM.
>
> It doesn't really matter if we save always a bit more than needed so
> this commit changes the amount saved always to be the size of the TPM
> header and three handles.
>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>


This patch only removes a dead parameter from API calls:

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>



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

end of thread, other threads:[~2018-11-06 15:26 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-05  1:45 [PATCH v3 00/16] Remove nested TPM operations Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 01/16] tpm: use tpm_buf in tpm_transmit_cmd() as the IO parameter Jarkko Sakkinen
2018-11-05 21:48   ` Stefan Berger
2018-11-06  5:52     ` Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 02/16] tpm: fix invalid return value in pubek_show() Jarkko Sakkinen
2018-11-05 21:51   ` Stefan Berger
2018-11-05  1:45 ` [PATCH v3 03/16] tpm: return 0 from pcrs_show() when tpm1_pcr_read() fails Jarkko Sakkinen
2018-11-05 21:54   ` Stefan Berger
2018-11-06  5:54     ` Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 04/16] tpm: call tpm2_flush_space() on error in tpm_try_transmit() Jarkko Sakkinen
2018-11-05 22:01   ` Stefan Berger
2018-11-06  5:55     ` Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 05/16] tpm: print tpm2_commit_space() error inside tpm2_commit_space() Jarkko Sakkinen
2018-11-05 22:04   ` Stefan Berger
2018-11-06  5:58     ` Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 06/16] tpm: clean up tpm_try_transmit() error handling flow Jarkko Sakkinen
2018-11-05 22:20   ` Stefan Berger
2018-11-06  6:01     ` Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 07/16] tpm: access command header through struct in tpm_try_transmit() Jarkko Sakkinen
2018-11-05 22:26   ` Stefan Berger
2018-11-06  6:08     ` Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 08/16] tpm: move tpm_validate_commmand() to tpm2-space.c Jarkko Sakkinen
2018-11-05 22:36   ` Stefan Berger
2018-11-06  6:10     ` Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 09/16] tpm: encapsulate tpm_dev_transmit() Jarkko Sakkinen
2018-11-06 15:17   ` Stefan Berger
2018-11-05  1:45 ` [PATCH v3 10/16] tpm: move TPM space code out of tpm_transmit() Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 12/16] tpm: use tpm_try_get_ops() in tpm-sysfs.c Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 13/16] tpm: remove TPM_TRANSMIT_UNLOCKED flag Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 14/16] tpm: introduce tpm_chip_start() and tpm_chip_stop() Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 15/16] tpm: take TPM chip power gating out of tpm_transmit() Jarkko Sakkinen
2018-11-05  1:45 ` [PATCH v3 16/16] tpm: remove @flags from tpm_transmit() Jarkko Sakkinen
     [not found] ` <20181105014552.20262-12-jarkko.sakkinen@linux.intel.com>
2018-11-06 15:25   ` [PATCH v3 11/16] tpm: remove @space " Stefan Berger

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).