Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1557653 > unrolled thread

[PATCH RFC v2 0/5] RFC: in-kernel resource manager

Back to article view | Back to linux.kernel

Contents

  [PATCH RFC v2 0/5] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
    [PATCH RFC v2 1/5] tpm: validate TPM 2.0 commands Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
      Re: [PATCH RFC v2 1/5] tpm: validate TPM 2.0 commands Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 21:40 +0100
    [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-12 19:50 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 21:40 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-12 21:40 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-13 17:30 +0100
          Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 11:00 +0100
      Re: [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 22:00 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-13 02:20 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-13 17:40 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 10:10 +0100
          Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-16 15:30 +0100
            Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 15:50 +0100
              Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-16 16:00 +0100
                Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM  spaces Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 18:00 +0100
    [PATCH RFC v2 2/5] tpm: export tpm2_flush_context_cmd Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
    [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
      Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device  link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-12 19:50 +0100
        Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 20:30 +0100
          Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-13 20:50 +0100
            Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 21:10 +0100
              Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-13 22:30 +0100
                Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-14 02:20 +0100
                  Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-16 18:00 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via    a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-12 21:00 +0100
      Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device  link /dev/tpms<n> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 22:00 +0100
        Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device  link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-13 18:30 +0100
          Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 18:50 +0100
            Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-13 19:10 +0100
              Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 19:20 +0100
            Re: [tpmdd-devel] [PATCH RFC v2 5/5] tpm2: expose resource manager  via a device link /dev/tpms<n> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 10:50 +0100
    [PATCH RFC v2 4/5] tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-12 18:50 +0100
      Re: [tpmdd-devel] [PATCH RFC v2 4/5] tpm: split out tpm-dev.c into  tpm-dev.c and tpm-common-dev.c James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-13 20:20 +0100

Page 1 of 2  [1] 2  Next page →

#1557653 — [PATCH RFC v2 0/5] RFC: in-kernel resource manager

This patch set adds support for TPM spaces that provide a context
for isolating and swapping transient objects. This patch set does
not yet include support for isolating policy and HMAC sessions but
it is trivial to add once the basic approach is settled (and that's
why I created an RFC patch set).

v2:
Changed to James' proposal of API. I did not make any other changes
except split core TPM space code its own patch because I want to find
consensus on the API before polishing the corners. Thus, this version
also carries the RFC tag. I have not yet locked in my standpoint whether
ioctl or a device file is a better deal.

James Bottomley (2):
  tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c
  tpm2: expose resource manager via a device link /dev/tpms<n>

Jarkko Sakkinen (3):
  tpm: validate TPM 2.0 commands
  tpm: export tpm2_flush_context_cmd
  tpm: infrastructure for TPM spaces

 drivers/char/tpm/Makefile         |   2 +-
 drivers/char/tpm/tpm-chip.c       |  61 +++++++-
 drivers/char/tpm/tpm-dev-common.c | 145 +++++++++++++++++++
 drivers/char/tpm/tpm-dev.c        | 141 ++----------------
 drivers/char/tpm/tpm-dev.h        |  27 ++++
 drivers/char/tpm/tpm-interface.c  | 106 ++++++++++----
 drivers/char/tpm/tpm-sysfs.c      |   2 +-
 drivers/char/tpm/tpm.h            |  57 ++++++--
 drivers/char/tpm/tpm2-cmd.c       | 144 ++++++++++++------
 drivers/char/tpm/tpm2-space.c     | 298 ++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpms-dev.c       |  57 ++++++++
 11 files changed, 826 insertions(+), 214 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-dev-common.c
 create mode 100644 drivers/char/tpm/tpm-dev.h
 create mode 100644 drivers/char/tpm/tpm2-space.c
 create mode 100644 drivers/char/tpm/tpms-dev.c

-- 
2.9.3

[toc] | [next] | [standalone]

#1557654 — [PATCH RFC v2 1/5] tpm: validate TPM 2.0 commands

Check for every TPM 2.0 command that the command code is supported and
the command buffer has at least the length that can contain the header
and the handle area.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-interface.c | 32 ++++++++++++++++++++++++-
 drivers/char/tpm/tpm.h           | 27 +++++++++++++++++----
 drivers/char/tpm/tpm2-cmd.c      | 51 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fecdd3f..0c5aba1 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -328,6 +328,36 @@ unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip,
 }
 EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 
+static bool tpm_validate_command(struct tpm_chip *chip, const u8 *cmd,
+				 size_t len)
+{
+	const struct tpm_input_header *header = (const void *)cmd;
+	u32 cc;
+	size_t len_min = TPM_HEADER_SIZE;
+	u32 attrs;
+
+	if ((len >= len_min) && (chip->flags & TPM_CHIP_FLAG_TPM2) &&
+	    chip->nr_commands) {
+		cc = be32_to_cpu(header->ordinal);
+		if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
+			dev_dbg(&chip->dev, "0x%04x is an invalid command\n",
+				cc);
+			return false;
+		}
+		len_min +=
+			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
+	}
+
+	if (len < len_min) {
+		dev_dbg(&chip->dev,
+			"%s: insufficient command length %zu < %zu\n",
+			__func__, len, len_min);
+		return false;
+	}
+
+	return true;
+}
+
 /**
  * tmp_transmit - Internal kernel interface to transmit TPM commands.
  *
@@ -347,7 +377,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
 	u32 count, ordinal;
 	unsigned long stop;
 
-	if (bufsiz < TPM_HEADER_SIZE)
+	if (!tpm_validate_command(chip, buf, bufsiz))
 		return -EINVAL;
 
 	if (bufsiz > TPM_BUFSIZE)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..80fa606 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -127,7 +127,12 @@ enum tpm2_permanent_handles {
 };
 
 enum tpm2_capabilities {
-	TPM2_CAP_TPM_PROPERTIES = 6,
+	TPM2_CAP_COMMANDS	= 2,
+	TPM2_CAP_TPM_PROPERTIES	= 6,
+};
+
+enum tpm2_properties {
+	TPM_PT_TOTAL_COMMANDS	= 0x0129,
 };
 
 enum tpm2_startup_types {
@@ -135,6 +140,11 @@ enum tpm2_startup_types {
 	TPM2_SU_STATE	= 0x0001,
 };
 
+enum tpm2_cc_attrs {
+	TPM2_CC_ATTR_CHANDLES	= 25,
+	TPM2_CC_ATTR_RHANDLE	= 28,
+};
+
 #define TPM_VID_INTEL    0x8086
 #define TPM_VID_WINBOND  0x1050
 #define TPM_VID_STM      0x104A
@@ -191,6 +201,9 @@ struct tpm_chip {
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
 #endif /* CONFIG_ACPI */
+
+	u32 nr_commands;
+	u32 *cc_attrs_tbl;
 };
 
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
@@ -393,7 +406,8 @@ struct tpm_cmd_t {
  */
 
 enum tpm_buf_flags {
-	TPM_BUF_OVERFLOW	= BIT(0),
+	TPM_BUF_INITIALIZED	= BIT(0),
+	TPM_BUF_OVERFLOW	= BIT(1),
 };
 
 struct tpm_buf {
@@ -419,13 +433,17 @@ static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
 	head->length = cpu_to_be32(sizeof(*head));
 	head->ordinal = cpu_to_be32(ordinal);
 
+	buf->flags = TPM_BUF_INITIALIZED;
+
 	return 0;
 }
 
 static inline void tpm_buf_destroy(struct tpm_buf *buf)
 {
-	kunmap(buf->data_page);
-	__free_page(buf->data_page);
+	if (buf->flags & TPM_BUF_INITIALIZED) {
+		kunmap(buf->data_page);
+		__free_page(buf->data_page);
+	}
 }
 
 static inline u32 tpm_buf_length(struct tpm_buf *buf)
@@ -545,4 +563,5 @@ int tpm2_auto_startup(struct tpm_chip *chip);
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm2_probe(struct tpm_chip *chip);
+bool tpm2_find_cc_attrs(struct tpm_chip *chip, u32 cc, u32 *attrs);
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eda239..c4d25b2 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -969,7 +969,10 @@ EXPORT_SYMBOL_GPL(tpm2_probe);
  */
 int tpm2_auto_startup(struct tpm_chip *chip)
 {
+	struct tpm_buf buf;
+	u32 nr_commands;
 	int rc;
+	int i;
 
 	rc = tpm_get_timeouts(chip);
 	if (rc)
@@ -993,8 +996,56 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		}
 	}
 
+	rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands, NULL);
+	if (rc)
+		goto out;
+
+	chip->cc_attrs_tbl = devm_kzalloc(&chip->dev, 4 * nr_commands,
+					  GFP_KERNEL);
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY);
+	if (rc)
+		goto out;
+
+	tpm_buf_append_u32(&buf, TPM2_CAP_COMMANDS);
+	tpm_buf_append_u32(&buf, TPM2_CC_FIRST);
+	tpm_buf_append_u32(&buf, nr_commands);
+
+	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, NULL);
+	if (rc < 0) {
+		tpm_buf_destroy(&buf);
+		goto out;
+	}
+
+	if (nr_commands !=
+	    be32_to_cpup((__be32 *)&buf.data[TPM_HEADER_SIZE + 5])) {
+		tpm_buf_destroy(&buf);
+		goto out;
+	}
+
+	for (i = 0; i < nr_commands; i++)
+		chip->cc_attrs_tbl[i] = be32_to_cpup(
+			(u32 *)&buf.data[TPM_HEADER_SIZE + 9 + 4 * i]);
+
+	chip->nr_commands = nr_commands;
+	tpm_buf_destroy(&buf);
+
 out:
 	if (rc > 0)
 		rc = -ENODEV;
 	return rc;
 }
+
+bool tpm2_find_cc_attrs(struct tpm_chip *chip, u32 cc, u32 *attrs)
+{
+	int i;
+
+	for (i = 0; i < chip->nr_commands; i++) {
+		if (cc == (chip->cc_attrs_tbl[i] & GENMASK(15, 0))) {
+			*attrs = chip->cc_attrs_tbl[i];
+			return true;
+		}
+	}
+
+	return false;
+}
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1557784 — Re: [PATCH RFC v2 1/5] tpm: validate TPM 2.0 commands

On Thu, Jan 12, 2017 at 07:46:04PM +0200, Jarkko Sakkinen wrote:
> Check for every TPM 2.0 command that the command code is supported and
> the command buffer has at least the length that can contain the header
> and the handle area.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

I think this could be bundled with Stefans patch. It is fairly
independent of the rest of the patch set.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1557658 — [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

Added ability to tpm_transmit() to supply a TPM space that contains
mapping from virtual handles to physical handles and backing storage for
swapping transient objects. TPM space is isolated from other users of
the TPM.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/Makefile        |   2 +-
 drivers/char/tpm/tpm-chip.c      |   7 +
 drivers/char/tpm/tpm-dev.c       |   2 +-
 drivers/char/tpm/tpm-interface.c |  61 ++++----
 drivers/char/tpm/tpm-sysfs.c     |   2 +-
 drivers/char/tpm/tpm.h           |  22 ++-
 drivers/char/tpm/tpm2-cmd.c      |  34 +++--
 drivers/char/tpm/tpm2-space.c    | 298 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 382 insertions(+), 46 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-space.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a05b1eb..251d0ed 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-		tpm_eventlog.o
+	 tpm_eventlog.o tpm2-space.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
 tpm-$(CONFIG_OF) += tpm_of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index c406343..993b9ae 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -128,6 +128,7 @@ static void tpm_dev_release(struct device *dev)
 	mutex_unlock(&idr_lock);
 
 	kfree(chip->log.bios_event_log);
+	kfree(chip->work_space.context_buf);
 	kfree(chip);
 }
 
@@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	chip->cdev.owner = THIS_MODULE;
 	chip->cdev.kobj.parent = &chip->dev.kobj;
 
+	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!chip->work_space.context_buf) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
 	return chip;
 
 out:
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 912ad30..249eeb0 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -144,7 +144,7 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 		mutex_unlock(&priv->buffer_mutex);
 		return -EPIPE;
 	}
-	out_size = tpm_transmit(priv->chip, priv->data_buffer,
+	out_size = tpm_transmit(priv->chip, NULL, priv->data_buffer,
 				sizeof(priv->data_buffer), 0);
 
 	tpm_put_ops(priv->chip);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 0c5aba1..65fcd04c 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -370,10 +370,11 @@ static bool tpm_validate_command(struct tpm_chip *chip, const u8 *cmd,
  *     0 when the operation is successful.
  *     A negative number for system errors (errno).
  */
-ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
-		     unsigned int flags)
+ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
+		     u8 *buf, size_t bufsiz, unsigned int flags)
 {
-	ssize_t rc;
+	int rc;
+	ssize_t len = 0;
 	u32 count, ordinal;
 	unsigned long stop;
 
@@ -399,10 +400,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
 	if (chip->dev.parent)
 		pm_runtime_get_sync(chip->dev.parent);
 
+	rc = tpm2_prepare_space(chip, space, ordinal, buf, bufsiz);
+	if (rc)
+		goto out;
+
 	rc = chip->ops->send(chip, (u8 *) buf, count);
 	if (rc < 0) {
 		dev_err(&chip->dev,
-			"tpm_transmit: tpm_send: error %zd\n", rc);
+			"tpm_transmit: tpm_send: error %d\n", rc);
 		goto out;
 	}
 
@@ -435,17 +440,23 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
 	goto out;
 
 out_recv:
-	rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
-	if (rc < 0)
+	len = chip->ops->recv(chip, (u8 *) buf, bufsiz);
+	if (len < 0) {
 		dev_err(&chip->dev,
-			"tpm_transmit: tpm_recv: error %zd\n", rc);
+			"tpm_transmit: tpm_recv: error %d\n", rc);
+		rc = len;
+		goto out;
+	}
+
+	rc = tpm2_commit_space(chip, space, ordinal, buf, bufsiz);
+
 out:
 	if (chip->dev.parent)
 		pm_runtime_put_sync(chip->dev.parent);
 
 	if (!(flags & TPM_TRANSMIT_UNLOCKED))
 		mutex_unlock(&chip->tpm_mutex);
-	return rc;
+	return rc ? rc : len;
 }
 
 /**
@@ -463,13 +474,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
  *     A negative number for system errors (errno).
  *     A positive number for a TPM error.
  */
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
-			 int len, unsigned int flags, const char *desc)
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
+			 void *cmd, int len, unsigned int flags,
+			 const char *desc)
 {
 	const struct tpm_output_header *header;
 	int err;
 
-	len = tpm_transmit(chip, (const u8 *)cmd, len, flags);
+	len = tpm_transmit(chip, space, cmd, len, flags);
 	if (len <  0)
 		return len;
 	else if (len < TPM_HEADER_SIZE)
@@ -521,7 +533,7 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
 		tpm_cmd.params.getcap_in.subcap = cpu_to_be32(subcap_id);
 	}
-	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
 			      desc);
 	if (!rc)
 		*cap = tpm_cmd.params.getcap_out.cap;
@@ -545,8 +557,9 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
 	start_cmd.header.in = tpm_startup_header;
 
 	start_cmd.params.startup_in.startup_type = startup_type;
-	return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
-				"attempting to start the TPM");
+	return tpm_transmit_cmd(chip, NULL, &start_cmd,
+				TPM_INTERNAL_RESULT_SIZE,
+				0, "attempting to start the TPM");
 }
 
 int tpm_get_timeouts(struct tpm_chip *chip)
@@ -687,8 +700,8 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
 	struct tpm_cmd_t cmd;
 
 	cmd.header.in = continue_selftest_header;
-	rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE, 0,
-			      "continue selftest");
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
+			      0, "continue selftest");
 	return rc;
 }
 
@@ -707,7 +720,7 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 
 	cmd.header.in = pcrread_header;
 	cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
-	rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, READ_PCR_RESULT_SIZE, 0,
 			      "attempting to read a pcr value");
 
 	if (rc == 0)
@@ -805,7 +818,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
 	cmd.header.in = pcrextend_header;
 	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
 	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
-	rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
 			      "attempting extend a PCR value");
 
 	tpm_put_ops(chip);
@@ -909,7 +922,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
 	if (chip == NULL)
 		return -ENODEV;
 
-	rc = tpm_transmit_cmd(chip, cmd, buflen, 0, "attempting tpm_cmd");
+	rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, "attempting tpm_cmd");
 
 	tpm_put_ops(chip);
 	return rc;
@@ -1011,15 +1024,15 @@ int tpm_pm_suspend(struct device *dev)
 		cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
 		memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
 		       TPM_DIGEST_SIZE);
-		rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
-				      "extending dummy pcr before suspend");
+		rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE,
+				      0, "extending dummy pcr before suspend");
 	}
 
 	/* now do the actual savestate */
 	for (try = 0; try < TPM_RETRY; try++) {
 		cmd.header.in = savestate_header;
-		rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, 0,
-				      NULL);
+		rc = tpm_transmit_cmd(chip, NULL, &cmd, SAVESTATE_RESULT_SIZE,
+				      0, NULL);
 
 		/*
 		 * If the TPM indicates that it is too busy to respond to
@@ -1102,7 +1115,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
 		tpm_cmd.header.in = tpm_getrandom_header;
 		tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
 
-		err = tpm_transmit_cmd(chip, &tpm_cmd,
+		err = tpm_transmit_cmd(chip, NULL, &tpm_cmd,
 				       TPM_GETRANDOM_RESULT_SIZE + num_bytes,
 				       0, "attempting get random");
 		if (err)
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 848ad65..dd31a00 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -39,7 +39,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
 	struct tpm_chip *chip = to_tpm_chip(dev);
 
 	tpm_cmd.header.in = tpm_readpubek_header;
-	err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE, 0,
+	err = tpm_transmit_cmd(chip, NULL, &tpm_cmd, READ_PUBEK_RESULT_SIZE, 0,
 			       "attempting to read the PUBEK");
 	if (err)
 		goto out;
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index c87c221..0e93b93 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -91,6 +91,7 @@ enum tpm2_structures {
 
 enum tpm2_return_codes {
 	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
+	TPM2_RC_HANDLE		= 0x008B,
 	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
 	TPM2_RC_DISABLED	= 0x0120,
 	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
@@ -114,6 +115,8 @@ enum tpm2_command_codes {
 	TPM2_CC_CREATE		= 0x0153,
 	TPM2_CC_LOAD		= 0x0157,
 	TPM2_CC_UNSEAL		= 0x015E,
+	TPM2_CC_CONTEXT_LOAD	= 0x0161,
+	TPM2_CC_CONTEXT_SAVE	= 0x0162,
 	TPM2_CC_FLUSH_CONTEXT	= 0x0165,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_GET_RANDOM	= 0x017B,
@@ -151,6 +154,11 @@ enum tpm2_cc_attrs {
 
 #define TPM_PPI_VERSION_LEN		3
 
+struct tpm_space {
+	u32 context_tbl[14];
+	u8 *context_buf;
+};
+
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_TPM2		= BIT(1),
 	TPM_CHIP_FLAG_IRQ		= BIT(2),
@@ -202,6 +210,7 @@ struct tpm_chip {
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
 #endif /* CONFIG_ACPI */
 
+	struct tpm_space work_space;
 	u32 nr_commands;
 	u32 *cc_attrs_tbl;
 };
@@ -509,10 +518,11 @@ enum tpm_transmit_flags {
 	TPM_TRANSMIT_UNLOCKED	= BIT(0),
 };
 
-ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
-		     unsigned int flags);
-ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd, int len,
-			 unsigned int flags, const char *desc);
+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 *cmd, int len, unsigned int flags,
+			 const char *desc);
 ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		   const char *desc);
 int tpm_get_timeouts(struct tpm_chip *);
@@ -566,4 +576,8 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm2_probe(struct tpm_chip *chip);
 bool tpm2_find_cc_attrs(struct tpm_chip *chip, u32 cc, u32 *attrs);
+int tpm2_prepare_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,
+		      u32 cc, u8 *buf, size_t bufsiz);
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index e2b4c75..6842555 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -280,7 +280,7 @@ int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
 	       sizeof(cmd.params.pcrread_in.pcr_select));
 	cmd.params.pcrread_in.pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7);
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0,
 			      "attempting to read a pcr value");
 	if (rc == 0) {
 		buf = cmd.params.pcrread_out.digest;
@@ -327,7 +327,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash)
 	cmd.params.pcrextend_in.hash_alg = cpu_to_be16(TPM2_ALG_SHA1);
 	memcpy(cmd.params.pcrextend_in.digest, hash, TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0,
 			      "attempting extend a PCR value");
 
 	return rc;
@@ -373,7 +373,7 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
 		cmd.header.in = tpm2_getrandom_header;
 		cmd.params.getrandom_in.size = cpu_to_be16(num_bytes);
 
-		err = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
+		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0,
 				       "attempting get random");
 		if (err)
 			break;
@@ -423,8 +423,8 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 
 	tpm_buf_append_u32(&buf, handle);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags,
-			     "flushing context");
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, TPM_BUFSIZE, flags,
+			      "flushing context");
 	if (rc)
 		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
 			 rc);
@@ -542,7 +542,8 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, "sealing data");
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, TPM_BUFSIZE, 0,
+			      "sealing data");
 	if (rc)
 		goto out;
 
@@ -620,7 +621,8 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags, "loading blob");
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, TPM_BUFSIZE, flags,
+			      "loading blob");
 	if (!rc)
 		*blob_handle = be32_to_cpup(
 			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
@@ -670,7 +672,8 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags, "unsealing");
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, TPM_BUFSIZE, flags,
+			      "unsealing");
 	if (rc > 0)
 		rc = -EPERM;
 
@@ -738,7 +741,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,  u32 *value,
 	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(property_id);
 	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, desc);
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, desc);
 	if (!rc)
 		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
 
@@ -772,7 +775,7 @@ static int tpm2_startup(struct tpm_chip *chip, u16 startup_type)
 	cmd.header.in = tpm2_startup_header;
 
 	cmd.params.startup_in.startup_type = cpu_to_be16(startup_type);
-	return tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0,
+	return tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0,
 				"attempting to start the TPM");
 }
 
@@ -801,7 +804,8 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type)
 	cmd.header.in = tpm2_shutdown_header;
 	cmd.params.startup_in.startup_type = cpu_to_be16(shutdown_type);
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, "stopping the TPM");
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0,
+			      "stopping the TPM");
 
 	/* In places where shutdown command is sent there's no much we can do
 	 * except print the error code on a system failure.
@@ -864,7 +868,7 @@ static int tpm2_start_selftest(struct tpm_chip *chip, bool full)
 	cmd.header.in = tpm2_selftest_header;
 	cmd.params.selftest_in.full_test = full;
 
-	rc = tpm_transmit_cmd(chip, &cmd, TPM2_SELF_TEST_IN_SIZE, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE, 0,
 			      "continue selftest");
 
 	/* At least some prototype chips seem to give RC_TESTING error
@@ -915,7 +919,7 @@ static int tpm2_do_selftest(struct tpm_chip *chip)
 		cmd.params.pcrread_in.pcr_select[1] = 0x00;
 		cmd.params.pcrread_in.pcr_select[2] = 0x00;
 
-		rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd), 0, NULL);
+		rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, NULL);
 		if (rc < 0)
 			break;
 
@@ -948,7 +952,7 @@ int tpm2_probe(struct tpm_chip *chip)
 	cmd.params.get_tpm_pt_in.property_id = cpu_to_be32(0x100);
 	cmd.params.get_tpm_pt_in.property_cnt = cpu_to_be32(1);
 
-	rc = tpm_transmit_cmd(chip, &cmd, sizeof(cmd),  0, NULL);
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),  0, NULL);
 	if (rc <  0)
 		return rc;
 
@@ -1010,7 +1014,7 @@ int tpm2_auto_startup(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.data, PAGE_SIZE, 0, NULL);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, NULL);
 	if (rc < 0) {
 		tpm_buf_destroy(&buf);
 		goto out;
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
new file mode 100644
index 0000000..ca55feb
--- /dev/null
+++ b/drivers/char/tpm/tpm2-space.c
@@ -0,0 +1,298 @@
+/*
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * This file contains TPM2 protocol implementations of the commands
+ * used by the kernel internally.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; version 2
+ * of the License.
+ */
+
+#include <linux/gfp.h>
+#include "tpm.h"
+
+enum tpm2_handle_types {
+	TPM2_HT_HMAC_SESSION	= 0x02000000,
+	TPM2_HT_POLICY_SESSION	= 0x03000000,
+	TPM2_HT_TRANSIENT	= 0x80000000,
+};
+
+static void tpm2_flush_space(struct tpm_chip *chip)
+{
+	struct tpm_space *space = &chip->work_space;
+	int i;
+
+	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);
+}
+
+struct tpm2_context {
+	__be64 sequence;
+	__be32 saved_handle;
+	__be32 hierarchy;
+	__be16 blob_size;
+} __packed;
+
+static int tpm2_load_space(struct tpm_chip *chip)
+{
+	struct tpm_space *space = &chip->work_space;
+	struct tpm2_context *ctx;
+	struct tpm_buf buf;
+	int i;
+	int j;
+	int rc;
+	u32 s;
+
+	for (i = 0, j = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
+		if (!space->context_tbl[i])
+			continue;
+
+		/* sanity check, should never happen */
+		if (~space->context_tbl[i]) {
+			dev_err(&chip->dev, "context table is inconsistent");
+			return -EFAULT;
+		}
+
+		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
+				 TPM2_CC_CONTEXT_LOAD);
+		if (rc)
+			return rc;
+
+		ctx = (struct tpm2_context *)&space->context_buf[j];
+		s = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
+		tpm_buf_append(&buf, &space->context_buf[j], s);
+
+		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
+				      TPM_TRANSMIT_UNLOCKED, NULL);
+		if (rc) {
+			dev_warn(&chip->dev, "%s: loading failed with %d\n",
+				 __func__, rc);
+			rc = -EFAULT;
+			goto out_err;
+		}
+
+		space->context_tbl[i] =
+			be32_to_cpup((__be32 *)&buf.data[TPM_HEADER_SIZE]);
+
+		j += s;
+
+		tpm_buf_destroy(&buf);
+	}
+
+	return 0;
+
+out_err:
+	tpm_buf_destroy(&buf);
+	tpm2_flush_space(chip);
+	return rc;
+}
+
+static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len)
+{
+	struct tpm_space *space = &chip->work_space;
+	unsigned int nr_handles;
+	u32 vhandle;
+	u32 phandle;
+	u32 attrs;
+	int i;
+	int j;
+	int rc;
+
+	if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
+		rc = -EINVAL;
+		goto out_err;
+	}
+
+	nr_handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+
+	for (i = 0; i < nr_handles; i++) {
+		vhandle = be32_to_cpup((__be32 *)&cmd[TPM_HEADER_SIZE + 4 * i]);
+		if ((vhandle & 0xFF000000) != TPM2_HT_TRANSIENT)
+			continue;
+
+		j = 0xFFFFFF - (vhandle & 0xFFFFFF);
+		if (j > ARRAY_SIZE(space->context_tbl) ||
+		    !space->context_tbl[j]) {
+			rc = -EINVAL;
+			goto out_err;
+		}
+
+		phandle = space->context_tbl[j];
+		*((__be32 *)&cmd[TPM_HEADER_SIZE + 4 * i]) =
+			cpu_to_be32(phandle);
+	}
+
+	return 0;
+
+out_err:
+	tpm2_flush_space(chip);
+	return rc;
+}
+
+int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
+		       u32 cc, u8 *buf, size_t bufsiz)
+{
+	int rc;
+
+	if (!space)
+		return 0;
+
+	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
+	       sizeof(space->context_tbl));
+	memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
+
+	rc = tpm2_load_space(chip);
+	if (rc)
+		return rc;
+
+	rc = tpm2_map_command(chip, cc, buf, bufsiz);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
+{
+	struct tpm_space *space = &chip->work_space;
+	u32 phandle;
+	u32 vhandle;
+	u32 attrs;
+	int i;
+	int rc;
+
+	if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
+		/* should never happen */
+		dev_err(&chip->dev, "TPM returned a different CC: 0x%04x\n",
+			cc);
+		rc = -EFAULT;
+		goto out_err;
+	}
+
+	if (!((attrs >> TPM2_CC_ATTR_RHANDLE) & 1))
+		return 0;
+
+	phandle = be32_to_cpup((__be32 *)&rsp[TPM_HEADER_SIZE]);
+	if ((phandle & 0xFF000000) != TPM2_HT_TRANSIENT)
+		return 0;
+
+	/* Garbage collect a dead context. */
+	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
+		if (space->context_tbl[i] == phandle) {
+			space->context_tbl[i] = 0;
+			break;
+		}
+	}
+
+	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
+		if (!space->context_tbl[i])
+			break;
+
+	if (i == ARRAY_SIZE(space->context_tbl)) {
+		dev_warn(&chip->dev, "%s: out of context slots\n", __func__);
+		tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_UNLOCKED);
+		rc = -ENOMEM;
+		goto out_err;
+	}
+
+	space->context_tbl[i] = phandle;
+	vhandle = TPM2_HT_TRANSIENT | (0xFFFFFF - i);
+	*(__be32 *)&rsp[TPM_HEADER_SIZE] = cpu_to_be32(vhandle);
+
+	return 0;
+
+out_err:
+	tpm2_flush_space(chip);
+	return rc;
+}
+
+static int tpm2_save_space(struct tpm_chip *chip)
+{
+	struct tpm_space *space = &chip->work_space;
+	struct tpm_buf buf;
+	int i;
+	int j;
+	int rc;
+	u32 s;
+
+	for (i = 0, j = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
+		if (!(space->context_tbl[i] && ~space->context_tbl[i]))
+			continue;
+
+		rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS,
+				  TPM2_CC_CONTEXT_SAVE);
+		if (rc)
+			return rc;
+
+		tpm_buf_append_u32(&buf, space->context_tbl[i]);
+
+		rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
+				      TPM_TRANSMIT_UNLOCKED, NULL);
+		if ((rc & TPM2_RC_HANDLE) == TPM2_RC_HANDLE) {
+			space->context_tbl[i] = 0;
+			continue;
+		} else if (rc) {
+			dev_warn(&chip->dev, "%s: saving failed with %d\n",
+				 __func__, rc);
+			rc = -EFAULT;
+			goto out_err;
+		}
+
+		s = tpm_buf_length(&buf) - TPM_HEADER_SIZE;
+		if ((j + s) > PAGE_SIZE) {
+			dev_warn(&chip->dev, "%s: out of backing storage\n",
+				 __func__);
+			rc = -ENOMEM;
+			goto out_err;
+		}
+
+		memcpy(&space->context_buf[j], &buf.data[TPM_HEADER_SIZE], s);
+
+		tpm2_flush_context_cmd(chip, space->context_tbl[i],
+				       TPM_TRANSMIT_UNLOCKED);
+
+		space->context_tbl[i] = ~0;
+
+		j += s;
+
+		tpm_buf_destroy(&buf);
+	}
+
+	return 0;
+out_err:
+	tpm_buf_destroy(&buf);
+	tpm2_flush_space(chip);
+	return rc;
+}
+
+int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
+		      u32 cc, u8 *buf, size_t bufsiz)
+{
+	int rc;
+
+	if (!space)
+		return 0;
+
+	rc = tpm2_map_response(chip, cc, buf, bufsiz);
+	if (rc)
+		return rc;
+
+	rc = tpm2_save_space(chip);
+	if (rc)
+		return rc;
+
+	memcpy(&space->context_tbl, &chip->work_space.context_tbl,
+	       sizeof(space->context_tbl));
+	memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
+
+	return 0;
+}
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1557725 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> @@ -435,17 +440,23 @@ ssize_t tpm_transmit(struct tpm_chip *chip,
> const u8 *buf, size_t bufsiz,
>         goto out;
> 
>  out_recv:
> -       rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
> -       if (rc < 0)
> +       len = chip->ops->recv(chip, (u8 *) buf, bufsiz);
> +       if (len < 0) {
>                 dev_err(&chip->dev,
> -                       "tpm_transmit: tpm_recv: error %zd\n", rc);
> +                       "tpm_transmit: tpm_recv: error %d\n", rc);
> +               rc = len;
> +               goto out;
> +       }
> +
> +       rc = tpm2_commit_space(chip, space, ordinal, buf, bufsiz);

I think this should be

rc = tpm2_commit_space(chip, space, ordinal, buf, len)

because tpm2_commit_space wants to know the length of the response, not
the length of the original command.

James

[toc] | [prev] | [next] | [standalone]

#1557786 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Thu, Jan 12, 2017 at 10:38:30AM -0800, James Bottomley wrote:
> On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > @@ -435,17 +440,23 @@ ssize_t tpm_transmit(struct tpm_chip *chip,
> > const u8 *buf, size_t bufsiz,
> >         goto out;
> > 
> >  out_recv:
> > -       rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
> > -       if (rc < 0)
> > +       len = chip->ops->recv(chip, (u8 *) buf, bufsiz);
> > +       if (len < 0) {
> >                 dev_err(&chip->dev,
> > -                       "tpm_transmit: tpm_recv: error %zd\n", rc);
> > +                       "tpm_transmit: tpm_recv: error %d\n", rc);
> > +               rc = len;
> > +               goto out;
> > +       }
> > +
> > +       rc = tpm2_commit_space(chip, space, ordinal, buf, bufsiz);
> 
> I think this should be
> 
> rc = tpm2_commit_space(chip, space, ordinal, buf, len)
> 
> because tpm2_commit_space wants to know the length of the response, not
> the length of the original command.

Yeah, would make sense.

> James

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1557783 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> +static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp,
> size_t len)
> +{
> +       struct tpm_space *space = &chip->work_space;
> +       u32 phandle;
> +       u32 vhandle;
> +       u32 attrs;
> +       int i;
> +       int rc;
> +
> +       if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
> +               /* should never happen */
> +               dev_err(&chip->dev, "TPM returned a different CC:
> 0x%04x\n",
> +                       cc);
> +               rc = -EFAULT;
> +               goto out_err;
> +       }
> +
> +       if (!((attrs >> TPM2_CC_ATTR_RHANDLE) & 1))
> +               return 0;
> +
> +       phandle = be32_to_cpup((__be32 *)&rsp[TPM_HEADER_SIZE]);

I think we have to check the command return code here.  We can't
blindly fish handles out of the response if the TPM returned an error
because they won't exist and we'll pull rubbish from the buffer. 
 Incremental patch below.

Note I think we should use get_unaligned_be32 because we're pulling a
32 bit word from something that's on byte 6 (so misaligned): some
architectures will trigger an unaligned trap for this (it's not a
problem: they trap handle it, it just slows down processing a lot).

James

---

commit d17ad905ff7b114f7efd23f930e9a541ccdf7621
Author: James Bottomley <James.Bottomley@HansenPartnership.com>
Date:   Wed Jan 11 22:01:29 2017 -0800

    check return code

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 61422e6..8009ed4 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -90,6 +90,7 @@ enum tpm2_structures {
 };
 
 enum tpm2_return_codes {
+	TPM2_RC_SUCCESS		= 0x0000,
 	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
 	TPM2_RC_HANDLE		= 0x008B,
 	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index ca55feb..44e5501 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/gfp.h>
+#include <asm/unaligned.h>
 #include "tpm.h"
 
 enum tpm2_handle_types {
@@ -167,9 +168,13 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
 	u32 phandle;
 	u32 vhandle;
 	u32 attrs;
+	u32 return_code = get_unaligned_be32((__be32 *)&rsp[6]);
 	int i;
 	int rc;
 
+	if (return_code != TPM2_RC_SUCCESS)
+		return 0;
+
 	if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
 		/* should never happen */
 		dev_err(&chip->dev, "TPM returned a different CC: 0x%04x\n",

[toc] | [prev] | [next] | [standalone]

#1558553 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Thu, Jan 12, 2017 at 12:38:52PM -0800, James Bottomley wrote:
> On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > +static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp,
> > size_t len)
> > +{
> > +       struct tpm_space *space = &chip->work_space;
> > +       u32 phandle;
> > +       u32 vhandle;
> > +       u32 attrs;
> > +       int i;
> > +       int rc;
> > +
> > +       if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
> > +               /* should never happen */
> > +               dev_err(&chip->dev, "TPM returned a different CC:
> > 0x%04x\n",
> > +                       cc);
> > +               rc = -EFAULT;
> > +               goto out_err;
> > +       }
> > +
> > +       if (!((attrs >> TPM2_CC_ATTR_RHANDLE) & 1))
> > +               return 0;
> > +
> > +       phandle = be32_to_cpup((__be32 *)&rsp[TPM_HEADER_SIZE]);
> 
> I think we have to check the command return code here.  We can't
> blindly fish handles out of the response if the TPM returned an error
> because they won't exist and we'll pull rubbish from the buffer. 
>  Incremental patch below.
> 
> Note I think we should use get_unaligned_be32 because we're pulling a
> 32 bit word from something that's on byte 6 (so misaligned): some
> architectures will trigger an unaligned trap for this (it's not a
> problem: they trap handle it, it just slows down processing a lot).
> 
> James
> 
> ---
> 
> commit d17ad905ff7b114f7efd23f930e9a541ccdf7621
> Author: James Bottomley <James.Bottomley@HansenPartnership.com>
> Date:   Wed Jan 11 22:01:29 2017 -0800
> 
>     check return code
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 61422e6..8009ed4 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -90,6 +90,7 @@ enum tpm2_structures {
>  };
>  
>  enum tpm2_return_codes {
> +	TPM2_RC_SUCCESS		= 0x0000,
>  	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
>  	TPM2_RC_HANDLE		= 0x008B,
>  	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index ca55feb..44e5501 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -16,6 +16,7 @@
>   */
>  
>  #include <linux/gfp.h>
> +#include <asm/unaligned.h>
>  #include "tpm.h"
>  
>  enum tpm2_handle_types {
> @@ -167,9 +168,13 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
>  	u32 phandle;
>  	u32 vhandle;
>  	u32 attrs;
> +	u32 return_code = get_unaligned_be32((__be32 *)&rsp[6]);
>  	int i;
>  	int rc;
>  
> +	if (return_code != TPM2_RC_SUCCESS)
> +		return 0;
> +
>  	if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
>  		/* should never happen */
>  		dev_err(&chip->dev, "TPM returned a different CC: 0x%04x\n",
> 

[x]

I'll squash this to the next patch set version.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1559573 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Sat, Jan 14, 2017 at 12:53:15PM -0500, Ken Goldman wrote:
> On 1/13/2017 11:28 AM, Jarkko Sakkinen wrote:
> 
> > > > +
> > > > +       if (!tpm2_find_cc_attrs(chip, cc, &attrs)) {
> > > > +               /* should never happen */
> > > > +               dev_err(&chip->dev, "TPM returned a different CC:
> > > > 0x%04x\n",
> > > > +                       cc);
> > > > +               rc = -EFAULT;
> > > > +               goto out_err;
> > > > +       }
> 
> Something is strange here.  Is "CC" command code?
> 
> The TPM does not return a command code.  The mapping should use the
> command code from the command.
> 
> It could be the code is correct - the command code mapped OK in the command
> but then failed in the response.  But the error message is strange.

Thanks. I'll update that message.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1557805 — Re: [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Thu, Jan 12, 2017 at 07:46:06PM +0200, Jarkko Sakkinen wrote:
> Added ability to tpm_transmit() to supply a TPM space that contains
> mapping from virtual handles to physical handles and backing storage for
> swapping transient objects. TPM space is isolated from other users of
> the TPM.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  drivers/char/tpm/Makefile        |   2 +-
>  drivers/char/tpm/tpm-chip.c      |   7 +
>  drivers/char/tpm/tpm-dev.c       |   2 +-
>  drivers/char/tpm/tpm-interface.c |  61 ++++----
>  drivers/char/tpm/tpm-sysfs.c     |   2 +-
>  drivers/char/tpm/tpm.h           |  22 ++-
>  drivers/char/tpm/tpm2-cmd.c      |  34 +++--
>  drivers/char/tpm/tpm2-space.c    | 298 +++++++++++++++++++++++++++++++++++++++
>  8 files changed, 382 insertions(+), 46 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm2-space.c
> 
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index a05b1eb..251d0ed 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -3,7 +3,7 @@
>  #
>  obj-$(CONFIG_TCG_TPM) += tpm.o
>  tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
> -		tpm_eventlog.o
> +	 tpm_eventlog.o tpm2-space.o
>  tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
>  tpm-$(CONFIG_OF) += tpm_of.o
>  obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index c406343..993b9ae 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -128,6 +128,7 @@ static void tpm_dev_release(struct device *dev)
>  	mutex_unlock(&idr_lock);
>  
>  	kfree(chip->log.bios_event_log);
> +	kfree(chip->work_space.context_buf);
>  	kfree(chip);
>  }
>  
> @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  	chip->cdev.owner = THIS_MODULE;
>  	chip->cdev.kobj.parent = &chip->dev.kobj;
>  
> +	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (!chip->work_space.context_buf) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
>  	return chip;
>  
>  out:
> diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
> index 912ad30..249eeb0 100644
> --- a/drivers/char/tpm/tpm-dev.c
> +++ b/drivers/char/tpm/tpm-dev.c
> @@ -144,7 +144,7 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
>  		mutex_unlock(&priv->buffer_mutex);
>  		return -EPIPE;
>  	}
> -	out_size = tpm_transmit(priv->chip, priv->data_buffer,
> +	out_size = tpm_transmit(priv->chip, NULL, priv->data_buffer,
>  				sizeof(priv->data_buffer), 0);
>  
>  	tpm_put_ops(priv->chip);
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index 0c5aba1..65fcd04c 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -370,10 +370,11 @@ static bool tpm_validate_command(struct tpm_chip *chip, const u8 *cmd,
>   *     0 when the operation is successful.
>   *     A negative number for system errors (errno).
>   */
> -ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
> -		     unsigned int flags)
> +ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> +		     u8 *buf, size_t bufsiz, unsigned int flags)
>  {
> -	ssize_t rc;
> +	int rc;
> +	ssize_t len = 0;
>  	u32 count, ordinal;
>  	unsigned long stop;
>  
> @@ -399,10 +400,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>  	if (chip->dev.parent)
>  		pm_runtime_get_sync(chip->dev.parent);
>  
> +	rc = tpm2_prepare_space(chip, space, ordinal, buf, bufsiz);
> +	if (rc)
> +		goto out;
> +
>  	rc = chip->ops->send(chip, (u8 *) buf, count);
>  	if (rc < 0) {
>  		dev_err(&chip->dev,
> -			"tpm_transmit: tpm_send: error %zd\n", rc);
> +			"tpm_transmit: tpm_send: error %d\n", rc);
>  		goto out;
>  	}
>  
> @@ -435,17 +440,23 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>  	goto out;
>  
>  out_recv:
> -	rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
> -	if (rc < 0)
> +	len = chip->ops->recv(chip, (u8 *) buf, bufsiz);
> +	if (len < 0) {
>  		dev_err(&chip->dev,
> -			"tpm_transmit: tpm_recv: error %zd\n", rc);
> +			"tpm_transmit: tpm_recv: error %d\n", rc);
> +		rc = len;
> +		goto out;
> +	}
> +
> +	rc = tpm2_commit_space(chip, space, ordinal, buf, bufsiz);
> +
>  out:
>  	if (chip->dev.parent)
>  		pm_runtime_put_sync(chip->dev.parent);
>  
>  	if (!(flags & TPM_TRANSMIT_UNLOCKED))
>  		mutex_unlock(&chip->tpm_mutex);
> -	return rc;
> +	return rc ? rc : len;
>  }
>  
>  /**
> @@ -463,13 +474,14 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>   *     A negative number for system errors (errno).
>   *     A positive number for a TPM error.
>   */
> -ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *cmd,
> -			 int len, unsigned int flags, const char *desc)
> +ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> +			 void *cmd, int len, unsigned int flags,
> +			 const char *desc)
>  {
>  	const struct tpm_output_header *header;
>  	int err;
>  
> -	len = tpm_transmit(chip, (const u8 *)cmd, len, flags);
> +	len = tpm_transmit(chip, space, cmd, len, flags);
>  	if (len <  0)
>  		return len;
>  	else if (len < TPM_HEADER_SIZE)
> @@ -521,7 +533,7 @@ ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>  		tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
>  		tpm_cmd.params.getcap_in.subcap = cpu_to_be32(subcap_id);
>  	}
> -	rc = tpm_transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
>  			      desc);
>  	if (!rc)
>  		*cap = tpm_cmd.params.getcap_out.cap;
> @@ -545,8 +557,9 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
>  	start_cmd.header.in = tpm_startup_header;
>  
>  	start_cmd.params.startup_in.startup_type = startup_type;
> -	return tpm_transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE, 0,
> -				"attempting to start the TPM");
> +	return tpm_transmit_cmd(chip, NULL, &start_cmd,
> +				TPM_INTERNAL_RESULT_SIZE,
> +				0, "attempting to start the TPM");
>  }
>  
>  int tpm_get_timeouts(struct tpm_chip *chip)
> @@ -687,8 +700,8 @@ static int tpm_continue_selftest(struct tpm_chip *chip)
>  	struct tpm_cmd_t cmd;
>  
>  	cmd.header.in = continue_selftest_header;
> -	rc = tpm_transmit_cmd(chip, &cmd, CONTINUE_SELFTEST_RESULT_SIZE, 0,
> -			      "continue selftest");
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
> +			      0, "continue selftest");
>  	return rc;
>  }
>  
> @@ -707,7 +720,7 @@ int tpm_pcr_read_dev(struct tpm_chip *chip, int pcr_idx, u8 *res_buf)
>  
>  	cmd.header.in = pcrread_header;
>  	cmd.params.pcrread_in.pcr_idx = cpu_to_be32(pcr_idx);
> -	rc = tpm_transmit_cmd(chip, &cmd, READ_PCR_RESULT_SIZE, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, READ_PCR_RESULT_SIZE, 0,
>  			      "attempting to read a pcr value");
>  
>  	if (rc == 0)
> @@ -805,7 +818,7 @@ int tpm_pcr_extend(u32 chip_num, int pcr_idx, const u8 *hash)
>  	cmd.header.in = pcrextend_header;
>  	cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(pcr_idx);
>  	memcpy(cmd.params.pcrextend_in.hash, hash, TPM_DIGEST_SIZE);
> -	rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
>  			      "attempting extend a PCR value");
>  
>  	tpm_put_ops(chip);
> @@ -909,7 +922,7 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
>  	if (chip == NULL)
>  		return -ENODEV;
>  
> -	rc = tpm_transmit_cmd(chip, cmd, buflen, 0, "attempting tpm_cmd");
> +	rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, "attempting tpm_cmd");
>  
>  	tpm_put_ops(chip);
>  	return rc;
> @@ -1011,15 +1024,15 @@ int tpm_pm_suspend(struct device *dev)
>  		cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
>  		memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
>  		       TPM_DIGEST_SIZE);
> -		rc = tpm_transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE, 0,
> -				      "extending dummy pcr before suspend");
> +		rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE,
> +				      0, "extending dummy pcr before suspend");
>  	}
>  
>  	/* now do the actual savestate */
>  	for (try = 0; try < TPM_RETRY; try++) {
>  		cmd.header.in = savestate_header;
> -		rc = tpm_transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE, 0,
> -				      NULL);
> +		rc = tpm_transmit_cmd(chip, NULL, &cmd, SAVESTATE_RESULT_SIZE,
> +				      0, NULL);
>  
>  		/*
>  		 * If the TPM indicates that it is too busy to respond to
> @@ -1102,7 +1115,7 @@ int tpm_get_random(u32 chip_num, u8 *out, size_t max)
>  		tpm_cmd.header.in = tpm_getrandom_header;
>  		tpm_cmd.params.getrandom_in.num_bytes = cpu_to_be32(num_bytes);
>  
> -		err = tpm_transmit_cmd(chip, &tpm_cmd,
> +		err = tpm_transmit_cmd(chip, NULL, &tpm_cmd,
>  				       TPM_GETRANDOM_RESULT_SIZE + num_bytes,
>  				       0, "attempting get random");
>  		if (err)
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 848ad65..dd31a00 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -39,7 +39,7 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr,
>  	struct tpm_chip *chip = to_tpm_chip(dev);
>  
>  	tpm_cmd.header.in = tpm_readpubek_header;
> -	err = tpm_transmit_cmd(chip, &tpm_cmd, READ_PUBEK_RESULT_SIZE, 0,
> +	err = tpm_transmit_cmd(chip, NULL, &tpm_cmd, READ_PUBEK_RESULT_SIZE, 0,
>  			       "attempting to read the PUBEK");
>  	if (err)
>  		goto out;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index c87c221..0e93b93 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -91,6 +91,7 @@ enum tpm2_structures {
>  
>  enum tpm2_return_codes {
>  	TPM2_RC_HASH		= 0x0083, /* RC_FMT1 */
> +	TPM2_RC_HANDLE		= 0x008B,
>  	TPM2_RC_INITIALIZE	= 0x0100, /* RC_VER1 */
>  	TPM2_RC_DISABLED	= 0x0120,
>  	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
> @@ -114,6 +115,8 @@ enum tpm2_command_codes {
>  	TPM2_CC_CREATE		= 0x0153,
>  	TPM2_CC_LOAD		= 0x0157,
>  	TPM2_CC_UNSEAL		= 0x015E,
> +	TPM2_CC_CONTEXT_LOAD	= 0x0161,
> +	TPM2_CC_CONTEXT_SAVE	= 0x0162,
>  	TPM2_CC_FLUSH_CONTEXT	= 0x0165,
>  	TPM2_CC_GET_CAPABILITY	= 0x017A,
>  	TPM2_CC_GET_RANDOM	= 0x017B,
> @@ -151,6 +154,11 @@ enum tpm2_cc_attrs {
>  
>  #define TPM_PPI_VERSION_LEN		3
>  
> +struct tpm_space {
> +	u32 context_tbl[14];
> +	u8 *context_buf;
> +};

I'll add a cc whitelist here in the next version. I also
decrease the size of the context_tbl to three.

It could be something like

DECLARE_BITMAP(cc_map, TPM2_CC_LAST - TPM2_CC_FIRST + 1);

Or

u32 *cc_tbl;
unsigned int cc_tbl_cnt;

The first alternative takes less space and is quicker to look up.
The second alternative is more dynamic (in terms of range).

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1557943 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct device
> *pdev,
>  	chip->cdev.owner = THIS_MODULE;
>  	chip->cdev.kobj.parent = &chip->dev.kobj;
> 
> +	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> GFP_KERNEL);
> +	if (!chip->work_space.context_buf) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +

I think the work_buf handling can be greatly simplified by making it a
pointer to the space: it's only usable between tpm2_prepare_space() and
tpm2_commit_space() which are protected by the chip mutex, so there's
no need for it to exist outside of these calls (i.e. it can be NULL).

Doing it this way also saves the allocation and copying overhead of
work_space.

The patch below can be folded to effect this.

James

---

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 13cac09..770a8c0 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -131,7 +131,6 @@ static void tpm_dev_release(struct device *dev)
 	mutex_unlock(&idr_lock);
 
 	kfree(chip->log.bios_event_log);
-	kfree(chip->work_space.context_buf);
 	kfree(chip);
 }
 
@@ -206,12 +205,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	chip->cdev.kobj.parent = &chip->dev.kobj;
 	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
 
-	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!chip->work_space.context_buf) {
-		rc = -ENOMEM;
-		goto out;
-	}
-
 	return chip;
 
 out:
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8009ed4..adf7810 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -211,7 +211,7 @@ struct tpm_chip {
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
 #endif /* CONFIG_ACPI */
 
-	struct tpm_space work_space;
+	struct tpm_space *work_space;
 	u32 nr_commands;
 	u32 *cc_attrs_tbl;
 };
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index 44e5501..285361e 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -27,7 +27,7 @@ enum tpm2_handle_types {
 
 static void tpm2_flush_space(struct tpm_chip *chip)
 {
-	struct tpm_space *space = &chip->work_space;
+	struct tpm_space *space = chip->work_space;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
@@ -45,7 +45,7 @@ struct tpm2_context {
 
 static int tpm2_load_space(struct tpm_chip *chip)
 {
-	struct tpm_space *space = &chip->work_space;
+	struct tpm_space *space = chip->work_space;
 	struct tpm2_context *ctx;
 	struct tpm_buf buf;
 	int i;
@@ -99,7 +99,7 @@ static int tpm2_load_space(struct tpm_chip *chip)
 
 static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len)
 {
-	struct tpm_space *space = &chip->work_space;
+	struct tpm_space *space = chip->work_space;
 	unsigned int nr_handles;
 	u32 vhandle;
 	u32 phandle;
@@ -147,9 +147,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
 	if (!space)
 		return 0;
 
-	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
-	       sizeof(space->context_tbl));
-	memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
+	chip->work_space = space;
 
 	rc = tpm2_load_space(chip);
 	if (rc)
@@ -164,7 +162,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
 
 static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
 {
-	struct tpm_space *space = &chip->work_space;
+	struct tpm_space *space = chip->work_space;
 	u32 phandle;
 	u32 vhandle;
 	u32 attrs;
@@ -222,7 +220,7 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
 
 static int tpm2_save_space(struct tpm_chip *chip)
 {
-	struct tpm_space *space = &chip->work_space;
+	struct tpm_space *space = chip->work_space;
 	struct tpm_buf buf;
 	int i;
 	int j;
@@ -295,9 +293,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 	if (rc)
 		return rc;
 
-	memcpy(&space->context_tbl, &chip->work_space.context_tbl,
-	       sizeof(space->context_tbl));
-	memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
+	chip->work_space = NULL;
 
 	return 0;
 }

[toc] | [prev] | [next] | [standalone]

#1558559 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Thu, Jan 12, 2017 at 05:17:23PM -0800, James Bottomley wrote:
> On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct device
> > *pdev,
> >  	chip->cdev.owner = THIS_MODULE;
> >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > 
> > +	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> > GFP_KERNEL);
> > +	if (!chip->work_space.context_buf) {
> > +		rc = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> 
> I think the work_buf handling can be greatly simplified by making it a
> pointer to the space: it's only usable between tpm2_prepare_space() and
> tpm2_commit_space() which are protected by the chip mutex, so there's
> no need for it to exist outside of these calls (i.e. it can be NULL).
> 
> Doing it this way also saves the allocation and copying overhead of
> work_space.
> 
> The patch below can be folded to effect this.
> 
> James
> 
> ---
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 13cac09..770a8c0 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -131,7 +131,6 @@ static void tpm_dev_release(struct device *dev)
>  	mutex_unlock(&idr_lock);
>  
>  	kfree(chip->log.bios_event_log);
> -	kfree(chip->work_space.context_buf);
>  	kfree(chip);
>  }
>  
> @@ -206,12 +205,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  	chip->cdev.kobj.parent = &chip->dev.kobj;
>  	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
>  
> -	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> -	if (!chip->work_space.context_buf) {
> -		rc = -ENOMEM;
> -		goto out;
> -	}
> -
>  	return chip;
>  
>  out:
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8009ed4..adf7810 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -211,7 +211,7 @@ struct tpm_chip {
>  	char ppi_version[TPM_PPI_VERSION_LEN + 1];
>  #endif /* CONFIG_ACPI */
>  
> -	struct tpm_space work_space;
> +	struct tpm_space *work_space;
>  	u32 nr_commands;
>  	u32 *cc_attrs_tbl;
>  };
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index 44e5501..285361e 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -27,7 +27,7 @@ enum tpm2_handle_types {
>  
>  static void tpm2_flush_space(struct tpm_chip *chip)
>  {
> -	struct tpm_space *space = &chip->work_space;
> +	struct tpm_space *space = chip->work_space;
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++)
> @@ -45,7 +45,7 @@ struct tpm2_context {
>  
>  static int tpm2_load_space(struct tpm_chip *chip)
>  {
> -	struct tpm_space *space = &chip->work_space;
> +	struct tpm_space *space = chip->work_space;
>  	struct tpm2_context *ctx;
>  	struct tpm_buf buf;
>  	int i;
> @@ -99,7 +99,7 @@ static int tpm2_load_space(struct tpm_chip *chip)
>  
>  static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd, size_t len)
>  {
> -	struct tpm_space *space = &chip->work_space;
> +	struct tpm_space *space = chip->work_space;
>  	unsigned int nr_handles;
>  	u32 vhandle;
>  	u32 phandle;
> @@ -147,9 +147,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
>  	if (!space)
>  		return 0;
>  
> -	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
> -	       sizeof(space->context_tbl));
> -	memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
> +	chip->work_space = space;
>  
>  	rc = tpm2_load_space(chip);
>  	if (rc)
> @@ -164,7 +162,7 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space,
>  
>  static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
>  {
> -	struct tpm_space *space = &chip->work_space;
> +	struct tpm_space *space = chip->work_space;
>  	u32 phandle;
>  	u32 vhandle;
>  	u32 attrs;
> @@ -222,7 +220,7 @@ static int tpm2_map_response(struct tpm_chip *chip, u32 cc, u8 *rsp, size_t len)
>  
>  static int tpm2_save_space(struct tpm_chip *chip)
>  {
> -	struct tpm_space *space = &chip->work_space;
> +	struct tpm_space *space = chip->work_space;
>  	struct tpm_buf buf;
>  	int i;
>  	int j;
> @@ -295,9 +293,7 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
>  	if (rc)
>  		return rc;
>  
> -	memcpy(&space->context_tbl, &chip->work_space.context_tbl,
> -	       sizeof(space->context_tbl));
> -	memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
> +	chip->work_space = NULL;
>  
>  	return 0;
>  }

[x]

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1559534 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Thu, Jan 12, 2017 at 05:17:23PM -0800, James Bottomley wrote:
> On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct device
> > *pdev,
> >  	chip->cdev.owner = THIS_MODULE;
> >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > 
> > +	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> > GFP_KERNEL);
> > +	if (!chip->work_space.context_buf) {
> > +		rc = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> 
> I think the work_buf handling can be greatly simplified by making it a
> pointer to the space: it's only usable between tpm2_prepare_space() and
> tpm2_commit_space() which are protected by the chip mutex, so there's
> no need for it to exist outside of these calls (i.e. it can be NULL).
> 
> Doing it this way also saves the allocation and copying overhead of
> work_space.
> 
> The patch below can be folded to effect this.

Hey, I have to take my words back. There's a separate buffer for space
for a reason. If the transaction fails for example when RM is doing its
job, we can revert to the previous set of transient objects.

Your change would completely thrawt this. I tried varius ways to heal
when RM decorations fail and this is the most fail safe to do it so lets
stick with it.

> James

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1559787 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Mon, 2017-01-16 at 11:09 +0200, Jarkko Sakkinen wrote:
> On Thu, Jan 12, 2017 at 05:17:23PM -0800, James Bottomley wrote:
> > On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > > @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct
> > > device
> > > *pdev,
> > >  	chip->cdev.owner = THIS_MODULE;
> > >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > > 
> > > +	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> > > GFP_KERNEL);
> > > +	if (!chip->work_space.context_buf) {
> > > +		rc = -ENOMEM;
> > > +		goto out;
> > > +	}
> > > +
> > 
> > I think the work_buf handling can be greatly simplified by making 
> > it a pointer to the space: it's only usable between 
> > tpm2_prepare_space() and tpm2_commit_space() which are protected by 
> > the chip mutex, so there's no need for it to exist outside of these 
> > calls (i.e. it can be NULL).
> > 
> > Doing it this way also saves the allocation and copying overhead of
> > work_space.
> > 
> > The patch below can be folded to effect this.
> 
> Hey, I have to take my words back. There's a separate buffer for 
> space for a reason. If the transaction fails for example when RM is 
> doing its job, we can revert to the previous set of transient
> objects.
> 
> Your change would completely thrawt this. I tried varius ways to heal
> when RM decorations fail and this is the most fail safe to do it so 
> lets stick with it.

That's why I added the return code check in the other patch: if the
command fails in the TPM, the space state isn't updated at all, the net
result being that nothing changes in the space, thus you don't need the
copy, because there's nothing to revert on a failure.

If you're thinking transaction being a sequence of TPM commands, then
we might need an ioctl to transfer the space state to/from userspace,
so it can do rollback for several commands, but that too wouldn't need
us to have a single prior command saved copy.

James

[toc] | [prev] | [next] | [standalone]

#1559803 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Mon, Jan 16, 2017 at 06:24:48AM -0800, James Bottomley wrote:
> On Mon, 2017-01-16 at 11:09 +0200, Jarkko Sakkinen wrote:
> > On Thu, Jan 12, 2017 at 05:17:23PM -0800, James Bottomley wrote:
> > > On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > > > @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct
> > > > device
> > > > *pdev,
> > > >  	chip->cdev.owner = THIS_MODULE;
> > > >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > > > 
> > > > +	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> > > > GFP_KERNEL);
> > > > +	if (!chip->work_space.context_buf) {
> > > > +		rc = -ENOMEM;
> > > > +		goto out;
> > > > +	}
> > > > +
> > > 
> > > I think the work_buf handling can be greatly simplified by making 
> > > it a pointer to the space: it's only usable between 
> > > tpm2_prepare_space() and tpm2_commit_space() which are protected by 
> > > the chip mutex, so there's no need for it to exist outside of these 
> > > calls (i.e. it can be NULL).
> > > 
> > > Doing it this way also saves the allocation and copying overhead of
> > > work_space.
> > > 
> > > The patch below can be folded to effect this.
> > 
> > Hey, I have to take my words back. There's a separate buffer for 
> > space for a reason. If the transaction fails for example when RM is 
> > doing its job, we can revert to the previous set of transient
> > objects.
> > 
> > Your change would completely thrawt this. I tried varius ways to heal
> > when RM decorations fail and this is the most fail safe to do it so 
> > lets stick with it.
> 
> That's why I added the return code check in the other patch: if the
> command fails in the TPM, the space state isn't updated at all, the net
> result being that nothing changes in the space, thus you don't need the
> copy, because there's nothing to revert on a failure.

You are right in what you say but what if you save lets say 5 transient
contexts and ContextSave fails on 2nd. It's not for the command itself
but for falling back to a sane state when tpm2_commit_space fails (to
the previous set of transient objects).

I've never meant it as a fallback for the command itself...

> If you're thinking transaction being a sequence of TPM commands, then
> we might need an ioctl to transfer the space state to/from userspace,
> so it can do rollback for several commands, but that too wouldn't need
> us to have a single prior command saved copy.
> 
> James

Here I refer to transaction as a single tpm_transmit.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1559815 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Mon, 2017-01-16 at 16:48 +0200, Jarkko Sakkinen wrote:
> On Mon, Jan 16, 2017 at 06:24:48AM -0800, James Bottomley wrote:
> > On Mon, 2017-01-16 at 11:09 +0200, Jarkko Sakkinen wrote:
> > > On Thu, Jan 12, 2017 at 05:17:23PM -0800, James Bottomley wrote:
> > > > On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > > > > @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct
> > > > > device
> > > > > *pdev,
> > > > >  	chip->cdev.owner = THIS_MODULE;
> > > > >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > > > > 
> > > > > +	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> > > > > GFP_KERNEL);
> > > > > +	if (!chip->work_space.context_buf) {
> > > > > +		rc = -ENOMEM;
> > > > > +		goto out;
> > > > > +	}
> > > > > +
> > > > 
> > > > I think the work_buf handling can be greatly simplified by 
> > > > making it a pointer to the space: it's only usable between
> > > > tpm2_prepare_space() and tpm2_commit_space() which are 
> > > > protected by the chip mutex, so there's no need for it to exist 
> > > > outside of these calls (i.e. it can be NULL).
> > > > 
> > > > Doing it this way also saves the allocation and copying 
> > > > overhead of work_space.
> > > > 
> > > > The patch below can be folded to effect this.
> > > 
> > > Hey, I have to take my words back. There's a separate buffer for 
> > > space for a reason. If the transaction fails for example when RM 
> > > is doing its job, we can revert to the previous set of transient
> > > objects.
> > > 
> > > Your change would completely thrawt this. I tried varius ways to 
> > > heal when RM decorations fail and this is the most fail safe to 
> > > do it so lets stick with it.
> > 
> > That's why I added the return code check in the other patch: if the
> > command fails in the TPM, the space state isn't updated at all, the 
> > net result being that nothing changes in the space, thus you don't 
> > need the copy, because there's nothing to revert on a failure.
> 
> You are right in what you say but what if you save lets say 5 
> transient contexts and ContextSave fails on 2nd. It's not for the 
> command itself but for falling back to a sane state when 
> tpm2_commit_space fails (to the previous set of transient objects).
> 
> I've never meant it as a fallback for the command itself...

Current error handling is to flush the entire space and abort ... and I
think that's correct.  There can't be any recovery because the TPM just
signalled it's in an insane state (it failed a command that should
succeed, either because our state is wrong and the TPM doesn't have the
handle we think it does or the TPM has had some internal error).  If
I'm the user, I'm likely going to have to reset the TPM and restart and
I likely wouldn't trust any state it gave me before this.

James


> > If you're thinking transaction being a sequence of TPM commands, 
> > then we might need an ioctl to transfer the space state to/from
> > userspace, so it can do rollback for several commands, but that too 
> > wouldn't need us to have a single prior command saved copy.
> > 
> > James
> 
> Here I refer to transaction as a single tpm_transmit.
> 
> /Jarkko

[toc] | [prev] | [next] | [standalone]

#1559904 — Re: [tpmdd-devel] [PATCH RFC v2 3/5] tpm: infrastructure for TPM spaces

On Mon, Jan 16, 2017 at 06:58:44AM -0800, James Bottomley wrote:
> On Mon, 2017-01-16 at 16:48 +0200, Jarkko Sakkinen wrote:
> > On Mon, Jan 16, 2017 at 06:24:48AM -0800, James Bottomley wrote:
> > > On Mon, 2017-01-16 at 11:09 +0200, Jarkko Sakkinen wrote:
> > > > On Thu, Jan 12, 2017 at 05:17:23PM -0800, James Bottomley wrote:
> > > > > On Thu, 2017-01-12 at 19:46 +0200, Jarkko Sakkinen wrote:
> > > > > > @@ -189,6 +190,12 @@ struct tpm_chip *tpm_chip_alloc(struct
> > > > > > device
> > > > > > *pdev,
> > > > > >  	chip->cdev.owner = THIS_MODULE;
> > > > > >  	chip->cdev.kobj.parent = &chip->dev.kobj;
> > > > > > 
> > > > > > +	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> > > > > > GFP_KERNEL);
> > > > > > +	if (!chip->work_space.context_buf) {
> > > > > > +		rc = -ENOMEM;
> > > > > > +		goto out;
> > > > > > +	}
> > > > > > +
> > > > > 
> > > > > I think the work_buf handling can be greatly simplified by 
> > > > > making it a pointer to the space: it's only usable between
> > > > > tpm2_prepare_space() and tpm2_commit_space() which are 
> > > > > protected by the chip mutex, so there's no need for it to exist 
> > > > > outside of these calls (i.e. it can be NULL).
> > > > > 
> > > > > Doing it this way also saves the allocation and copying 
> > > > > overhead of work_space.
> > > > > 
> > > > > The patch below can be folded to effect this.
> > > > 
> > > > Hey, I have to take my words back. There's a separate buffer for 
> > > > space for a reason. If the transaction fails for example when RM 
> > > > is doing its job, we can revert to the previous set of transient
> > > > objects.
> > > > 
> > > > Your change would completely thrawt this. I tried varius ways to 
> > > > heal when RM decorations fail and this is the most fail safe to 
> > > > do it so lets stick with it.
> > > 
> > > That's why I added the return code check in the other patch: if the
> > > command fails in the TPM, the space state isn't updated at all, the 
> > > net result being that nothing changes in the space, thus you don't 
> > > need the copy, because there's nothing to revert on a failure.
> > 
> > You are right in what you say but what if you save lets say 5 
> > transient contexts and ContextSave fails on 2nd. It's not for the 
> > command itself but for falling back to a sane state when 
> > tpm2_commit_space fails (to the previous set of transient objects).
> > 
> > I've never meant it as a fallback for the command itself...
> 
> Current error handling is to flush the entire space and abort ... and I
> think that's correct.  There can't be any recovery because the TPM just
> signalled it's in an insane state (it failed a command that should
> succeed, either because our state is wrong and the TPM doesn't have the
> handle we think it does or the TPM has had some internal error).  If
> I'm the user, I'm likely going to have to reset the TPM and restart and
> I likely wouldn't trust any state it gave me before this.
> 
> James

The *current* fallback in the patch set is what I said. It keeps
previously saved set of transient objects and will try to load them next
time. Only when contexts are succesfully saved the code overwrite the
buffers with new stuff.

tpm2_flush_context() is only used to clean up remaining contexts after
ContextSave fails. It's not *the* fallback. Reverting to the previous
set of transient contexts is.

That's why I didn't check for TPM error in tpm2_commit_response (except
now I have the check in v3 because this wasn't bright in my mind at the
time).

The reasons for this fallback have nothing to do with mixed up TPM
state. There are legit reasons for tpm2_commit_response to fail like
running out of virtual handle slots or out of backing storage.

/Jarkko

[toc] | [prev] | [next] | [standalone]

#1557662 — [PATCH RFC v2 2/5] tpm: export tpm2_flush_context_cmd

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm.h      |  2 ++
 drivers/char/tpm/tpm2-cmd.c | 65 ++++++++++++++++++++++-----------------------
 2 files changed, 34 insertions(+), 33 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 80fa606..c87c221 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -550,6 +550,8 @@ static inline void tpm_add_ppi(struct tpm_chip *chip)
 int tpm2_pcr_read(struct tpm_chip *chip, int pcr_idx, u8 *res_buf);
 int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, const u8 *hash);
 int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max);
+void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
+			    unsigned int flags);
 int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_payload *payload,
 		      struct trusted_key_options *options);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index c4d25b2..e2b4c75 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -401,6 +401,38 @@ static const struct tpm_input_header tpm2_get_tpm_pt_header = {
 };
 
 /**
+ * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
+ * @chip: TPM chip to use
+ * @payload: the key data in clear and encrypted form
+ * @options: authentication values and other options
+ *
+ * Return: same as with tpm_transmit_cmd
+ */
+void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
+			    unsigned int flags)
+{
+	struct tpm_buf buf;
+	int rc;
+
+	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
+	if (rc) {
+		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
+			 handle);
+		return;
+	}
+
+	tpm_buf_append_u32(&buf, handle);
+
+	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags,
+			     "flushing context");
+	if (rc)
+		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
+			 rc);
+
+	tpm_buf_destroy(&buf);
+}
+
+/**
  * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
  *
  * @buf: an allocated tpm_buf instance
@@ -603,39 +635,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 }
 
 /**
- * tpm2_flush_context_cmd() - execute a TPM2_FlushContext command
- *
- * @chip: TPM chip to use
- * @handle: the key data in clear and encrypted form
- * @flags: tpm transmit flags
- *
- * Return: Same as with tpm_transmit_cmd.
- */
-static void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
-				   unsigned int flags)
-{
-	struct tpm_buf buf;
-	int rc;
-
-	rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
-	if (rc) {
-		dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
-			 handle);
-		return;
-	}
-
-	tpm_buf_append_u32(&buf, handle);
-
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags,
-			      "flushing context");
-	if (rc)
-		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
-			 rc);
-
-	tpm_buf_destroy(&buf);
-}
-
-/**
  * tpm2_unseal_cmd() - execute a TPM2_Unload command
  *
  * @chip: TPM chip to use
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1557664 — [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

From: James Bottomley <James.Bottomley@HansenPartnership.com>

Currently the Resource Manager (RM) is not exposed to userspace.  Make
this exposure via a separate device, which can now be opened multiple
times because each read/write transaction goes separately via the RM.

Concurrency is protected by the chip->tpm_mutex for each read/write
transaction separately.  The TPM is cleared of all transient objects
by the time the mutex is dropped, so there should be no interference
between the kernel and userspace.

Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/char/tpm/Makefile        |  2 +-
 drivers/char/tpm/tpm-chip.c      | 54 ++++++++++++++++++++++++++++++++++---
 drivers/char/tpm/tpm-interface.c | 13 +++++++--
 drivers/char/tpm/tpm.h           |  6 +++--
 drivers/char/tpm/tpms-dev.c      | 57 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 124 insertions(+), 8 deletions(-)
 create mode 100644 drivers/char/tpm/tpms-dev.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 13ff5da..e50d768 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-	 tpm_eventlog.o tpm2-space.o tpm-dev-common.o
+	 tpm_eventlog.o tpm2-space.o tpm-dev-common.o tpms-dev.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
 tpm-$(CONFIG_OF) += tpm_of.o
 obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 993b9ae..0d2be04 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -33,6 +33,7 @@ DEFINE_IDR(dev_nums_idr);
 static DEFINE_MUTEX(idr_lock);
 
 struct class *tpm_class;
+struct class *tpm_rm_class;
 dev_t tpm_devt;
 
 /**
@@ -168,27 +169,40 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	chip->dev_num = rc;
 
 	device_initialize(&chip->dev);
+	device_initialize(&chip->devrm);
 
 	chip->dev.class = tpm_class;
 	chip->dev.release = tpm_dev_release;
 	chip->dev.parent = pdev;
 	chip->dev.groups = chip->groups;
 
+	chip->devrm.parent = pdev;
+	chip->devrm.class = tpm_rm_class;
+
 	if (chip->dev_num == 0)
 		chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR);
 	else
 		chip->dev.devt = MKDEV(MAJOR(tpm_devt), chip->dev_num);
 
+	chip->devrm.devt =
+		MKDEV(MAJOR(tpm_devt), chip->dev_num + TPM_NUM_DEVICES);
+
 	rc = dev_set_name(&chip->dev, "tpm%d", chip->dev_num);
 	if (rc)
 		goto out;
+	rc = dev_set_name(&chip->devrm, "tpms%d", chip->dev_num);
+	if (rc)
+		goto out;
 
 	if (!pdev)
 		chip->flags |= TPM_CHIP_FLAG_VIRTUAL;
 
 	cdev_init(&chip->cdev, &tpm_fops);
+	cdev_init(&chip->cdevrm, &tpm_rm_fops);
 	chip->cdev.owner = THIS_MODULE;
+	chip->cdevrm.owner = THIS_MODULE;
 	chip->cdev.kobj.parent = &chip->dev.kobj;
+	chip->cdevrm.kobj.parent = &chip->devrm.kobj;
 
 	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!chip->work_space.context_buf) {
@@ -200,6 +214,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 
 out:
 	put_device(&chip->dev);
+	put_device(&chip->devrm);
 	return ERR_PTR(rc);
 }
 EXPORT_SYMBOL_GPL(tpm_chip_alloc);
@@ -244,7 +259,7 @@ static int tpm_add_char_device(struct tpm_chip *chip)
 			dev_name(&chip->dev), MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
-		return rc;
+		goto err_1;
 	}
 
 	rc = device_add(&chip->dev);
@@ -254,16 +269,44 @@ static int tpm_add_char_device(struct tpm_chip *chip)
 			dev_name(&chip->dev), MAJOR(chip->dev.devt),
 			MINOR(chip->dev.devt), rc);
 
-		cdev_del(&chip->cdev);
-		return rc;
+		goto err_2;
+	}
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = cdev_add(&chip->cdevrm, chip->devrm.devt, 1);
+	if (rc) {
+		dev_err(&chip->dev,
+			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devrm), MAJOR(chip->devrm.devt),
+			MINOR(chip->devrm.devt), rc);
+
+		goto err_3;
 	}
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = device_add(&chip->devrm);
+	if (rc) {
+		dev_err(&chip->dev,
+			"unable to device_register() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devrm), MAJOR(chip->devrm.devt),
+			MINOR(chip->devrm.devt), rc);
+
+		goto err_4;
+	}
 	/* Make the chip available. */
 	mutex_lock(&idr_lock);
 	idr_replace(&dev_nums_idr, chip, chip->dev_num);
 	mutex_unlock(&idr_lock);
 
 	return rc;
+ err_4:
+	cdev_del(&chip->cdevrm);
+ err_3:
+	device_del(&chip->dev);
+ err_2:
+	cdev_del(&chip->cdev);
+ err_1:
+	return rc;
 }
 
 static void tpm_del_char_device(struct tpm_chip *chip)
@@ -271,6 +314,11 @@ static void tpm_del_char_device(struct tpm_chip *chip)
 	cdev_del(&chip->cdev);
 	device_del(&chip->dev);
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
+		cdev_del(&chip->cdevrm);
+		device_del(&chip->devrm);
+	}
+
 	/* Make the chip unavailable. */
 	mutex_lock(&idr_lock);
 	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 65fcd04c..f5c9355 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1197,9 +1197,17 @@ static int __init tpm_init(void)
 		return PTR_ERR(tpm_class);
 	}
 
-	rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES, "tpm");
+	tpm_rm_class = class_create(THIS_MODULE, "tpms");
+	if (IS_ERR(tpm_rm_class)) {
+		pr_err("couldn't create tpmrm class\n");
+		class_destroy(tpm_class);
+		return PTR_ERR(tpm_rm_class);
+	}
+
+	rc = alloc_chrdev_region(&tpm_devt, 0, 2*TPM_NUM_DEVICES, "tpm");
 	if (rc < 0) {
 		pr_err("tpm: failed to allocate char dev region\n");
+		class_destroy(tpm_rm_class);
 		class_destroy(tpm_class);
 		return rc;
 	}
@@ -1211,7 +1219,8 @@ static void __exit tpm_exit(void)
 {
 	idr_destroy(&dev_nums_idr);
 	class_destroy(tpm_class);
-	unregister_chrdev_region(tpm_devt, TPM_NUM_DEVICES);
+	class_destroy(tpm_rm_class);
+	unregister_chrdev_region(tpm_devt, 2*TPM_NUM_DEVICES);
 }
 
 subsys_initcall(tpm_init);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 0e93b93..61422e6 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -172,8 +172,8 @@ struct tpm_chip_seqops {
 };
 
 struct tpm_chip {
-	struct device dev;
-	struct cdev cdev;
+	struct device dev, devrm;
+	struct cdev cdev, cdevrm;
 
 	/* A driver callback under ops cannot be run unless ops_sem is held
 	 * (sometimes implicitly, eg for the sysfs code). ops becomes null
@@ -510,8 +510,10 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 }
 
 extern struct class *tpm_class;
+extern struct class *tpm_rm_class;
 extern dev_t tpm_devt;
 extern const struct file_operations tpm_fops;
+extern const struct file_operations tpm_rm_fops;
 extern struct idr dev_nums_idr;
 
 enum tpm_transmit_flags {
diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms-dev.c
new file mode 100644
index 0000000..d822600
--- /dev/null
+++ b/drivers/char/tpm/tpms-dev.c
@@ -0,0 +1,57 @@
+/*
+ * Copyright (C) 2017 James.Bottomley@HansenPartnership.com
+ *
+ * GPLv2
+ */
+#include <linux/slab.h>
+#include "tpm-dev.h"
+
+struct tpms_priv {
+	struct file_priv priv;
+	struct tpm_space space;
+};
+
+static int tpms_open(struct inode *inode, struct file *file)
+{
+	struct tpm_chip *chip;
+	struct tpms_priv *priv;
+
+	chip = container_of(inode->i_cdev, struct tpm_chip, cdevrm);
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (priv == NULL)
+		return -ENOMEM;
+
+	tpm_common_open(file, chip, &priv->priv);
+
+	return 0;
+}
+
+static int tpms_release(struct inode *inode, struct file *file)
+{
+	struct file_priv *fpriv = file->private_data;
+	struct tpms_priv *priv = container_of(fpriv, struct tpms_priv, priv);
+
+	tpm_common_release(file, fpriv);
+	kfree(priv);
+
+	return 0;
+}
+
+ssize_t tpms_write(struct file *file, const char __user *buf,
+		   size_t size, loff_t *off)
+{
+	struct file_priv *fpriv = file->private_data;
+	struct tpms_priv *priv = container_of(fpriv, struct tpms_priv, priv);
+
+	return tpm_common_write(file, buf, size, off, &priv->space);
+}
+
+const struct file_operations tpm_rm_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.open = tpms_open,
+	.read = tpm_common_read,
+	.write = tpms_write,
+	.release = tpms_release,
+};
+
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1557726 — Re: [PATCH RFC v2 5/5] tpm2: expose resource manager via a device link /dev/tpms<n>

On Thu, Jan 12, 2017 at 07:46:08PM +0200, Jarkko Sakkinen wrote:

>  struct tpm_chip {
> -	struct device dev;
> -	struct cdev cdev;
> +	struct device dev, devrm;

Hum.. devrm adds a new kref but doesn't do anything with the release
function, so that is going to use after free, ie here:

> 	put_device(&chip->dev);
>+	put_device(&chip->devrm);
> 	return ERR_PTR(rc);

And other places.

One solution is to get_device(chip->dev) after
device_initialize(dev->rm) and add a devrm->dev.release function to
do put_device(chip->dev)

Jason

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web