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

Groups > linux.kernel > #1549178 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
    [PATCH RFC 2/4] tpm: validate TPM 2.0 commands Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
    [PATCH RFC 3/4] tpm: export tpm2_flush_context_cmd Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
    [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
      Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for  TPM 2.0 Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-02 22:20 +0100
        Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for  TPM 2.0 Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 05:10 +0100
    [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-02 14:30 +0100
      Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-02 22:10 +0100
        Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 05:10 +0100
    Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-02 17:40 +0100
      Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-02 22:50 +0100
        Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-03 06:30 +0100
          Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 14:50 +0100
            Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <jejb@linux.vnet.ibm.com> - 2017-01-03 17:20 +0100
        Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 15:00 +0100
          Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager James Bottomley <James.Bottomley@HansenPartnership.com> - 2017-01-03 17:40 +0100
      Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-03 05:10 +0100

#1549178 — [PATCH RFC 0/4] 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).

There's a test script for trying out TPM spaces in

  git://git.infradead.org/users/jjs/tpm2-scripts.git

A simple smoke test can be run by

  sudo python -m unittest -v tpm2_smoke.SpaceTest   

Jarkko Sakkinen (4):
  tpm: migrate struct tpm_buf to struct tpm_chip
  tpm: validate TPM 2.0 commands
  tpm: export tpm2_flush_context_cmd
  tpm: add the infrastructure for TPM space for TPM 2.0

 drivers/char/tpm/Makefile        |   2 +-
 drivers/char/tpm/tpm-chip.c      |  15 ++
 drivers/char/tpm/tpm-dev.c       |  80 ++++++++++-
 drivers/char/tpm/tpm-interface.c |  93 +++++++++----
 drivers/char/tpm/tpm-sysfs.c     |   2 +-
 drivers/char/tpm/tpm.h           | 106 ++++++++------
 drivers/char/tpm/tpm2-cmd.c      | 232 ++++++++++++++++---------------
 drivers/char/tpm/tpm2-space.c    | 288 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/tpm.h         |  23 ++++
 9 files changed, 662 insertions(+), 179 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-space.c
 create mode 100644 include/uapi/linux/tpm.h

-- 
2.9.3

[toc] | [next] | [standalone]

#1549179 — [PATCH RFC 2/4] 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           | 15 +++++++++++++-
 drivers/char/tpm/tpm2-cmd.c      | 43 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 769d8b0..0794a5d3 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 c74a663..ed21c2c 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
@@ -207,6 +217,8 @@ struct tpm_chip {
 #endif /* CONFIG_ACPI */
 
 	struct tpm_buf tr_buf;
+	u32 nr_commands;
+	u32 *cc_attrs_tbl;
 };
 
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
@@ -534,4 +546,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 f0e0807..fa928c7 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -943,7 +943,9 @@ EXPORT_SYMBOL_GPL(tpm2_probe);
  */
 int tpm2_auto_startup(struct tpm_chip *chip)
 {
+	u32 nr_commands;
 	int rc;
+	int i;
 
 	rc = tpm_get_timeouts(chip);
 	if (rc)
@@ -967,8 +969,49 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 		}
 	}
 
+	rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands, NULL);
+	if (rc)
+		return rc;
+
+	chip->cc_attrs_tbl = devm_kzalloc(&chip->dev, 4 * nr_commands,
+					  GFP_KERNEL);
+
+	tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS,
+		     TPM2_CC_GET_CAPABILITY);
+	tpm_buf_append_u32(&chip->tr_buf, TPM2_CAP_COMMANDS);
+	tpm_buf_append_u32(&chip->tr_buf, TPM2_CC_FIRST);
+	tpm_buf_append_u32(&chip->tr_buf, nr_commands);
+
+	rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, 0, NULL);
+	if (rc < 0)
+		goto out;
+
+	if (nr_commands !=
+	    be32_to_cpup((__be32 *)&chip->tr_buf.data[TPM_HEADER_SIZE + 5]))
+		return -EINVAL;
+
+	for (i = 0; i < nr_commands; i++)
+		chip->cc_attrs_tbl[i] = be32_to_cpup(
+			(u32 *)&chip->tr_buf.data[TPM_HEADER_SIZE + 9 + 4 * i]);
+
+	chip->nr_commands = nr_commands;
+
 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]

#1549182 — [PATCH RFC 3/4] 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 | 47 ++++++++++++++++++++++-----------------------
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index ed21c2c..fb02b57 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -533,6 +533,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 fa928c7..311dc8e 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -401,6 +401,29 @@ 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)
+{
+	int rc;
+
+	tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
+	tpm_buf_append_u32(&chip->tr_buf, handle);
+
+	rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, flags,
+			      "flushing context");
+	if (rc)
+		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
+			 rc);
+}
+
+/**
  * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
  *
  * @buf: an allocated tpm_buf instance
@@ -589,30 +612,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)
-{
-	int rc;
-
-	tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
-	tpm_buf_append_u32(&chip->tr_buf, handle);
-
-	rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, flags,
-			      "flushing context");
-	if (rc)
-		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
-			 rc);
-}
-
-/**
  * tpm2_unseal_cmd() - execute a TPM2_Unload command
  *
  * @chip: TPM chip to use
-- 
2.9.3

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

#1549183 — [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0

Added a ioctl for creating a TPM space. The space is isolated from the
other users of the TPM. Only a process holding the file with the handle
can access the objects and only objects that are created through that
file handle can be accessed.

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       |  80 ++++++++++-
 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      |  30 ++--
 drivers/char/tpm/tpm2-space.c    | 288 +++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/tpm.h         |  23 ++++
 9 files changed, 470 insertions(+), 45 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-space.c
 create mode 100644 include/uapi/linux/tpm.h

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 41e518e..468af50 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -129,6 +129,7 @@ static void tpm_dev_release(struct device *dev)
 
 	kfree(chip->log.bios_event_log);
 	kfree(chip->tr_buf.data);
+	kfree(chip->work_space.context_buf);
 	kfree(chip);
 }
 
@@ -197,6 +198,12 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	}
 	chip->tr_buf.size = TPM_BUFSIZE;
 
+	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..139638b 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -19,6 +19,7 @@
  */
 #include <linux/slab.h>
 #include <linux/uaccess.h>
+#include <uapi/linux/tpm.h>
 #include "tpm.h"
 
 struct file_priv {
@@ -32,6 +33,8 @@ struct file_priv {
 	struct work_struct work;
 
 	u8 data_buffer[TPM_BUFSIZE];
+	struct tpm_space space;
+	bool has_space;
 };
 
 static void user_reader_timeout(unsigned long ptr)
@@ -115,6 +118,7 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 			 size_t size, loff_t *off)
 {
 	struct file_priv *priv = file->private_data;
+	struct tpm_space *space = NULL;
 	size_t in_size = size;
 	ssize_t out_size;
 
@@ -130,6 +134,9 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 
 	mutex_lock(&priv->buffer_mutex);
 
+	if (priv->has_space)
+		space = &priv->space;
+
 	if (copy_from_user
 	    (priv->data_buffer, (void __user *) buf, in_size)) {
 		mutex_unlock(&priv->buffer_mutex);
@@ -144,7 +151,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, space, priv->data_buffer,
 				sizeof(priv->data_buffer), 0);
 
 	tpm_put_ops(priv->chip);
@@ -162,6 +169,65 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 	return in_size;
 }
 
+/**
+ * tpm_ioc_new_space - handler for %SGX_IOC_NEW_SPACE ioctl
+ *
+ * Creates a new TPM space that can hold a set of transient objects. The space
+ * is isolated with virtual handles that are mapped into physical handles by the
+ * driver.
+ */
+static long tpm_ioc_new_space(struct file *file, unsigned int ioctl,
+			      unsigned long arg)
+{
+	struct file_priv *priv = file->private_data;
+	struct tpm_chip *chip = priv->chip;
+	int rc = 0;
+
+	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
+		return -EOPNOTSUPP;
+
+	mutex_lock(&priv->buffer_mutex);
+
+	if (priv->has_space) {
+		rc = -EBUSY;
+		goto out;
+	}
+
+	priv->space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!priv->space.context_buf) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	/* The TPM device can be opened again as this file has been moved to a
+	 * TPM handle space.
+	 */
+	priv->has_space = true;
+	clear_bit(0, &chip->is_open);
+out:
+	mutex_unlock(&priv->buffer_mutex);
+	return rc;
+}
+
+static long tpm_ioctl(struct file *file, unsigned int ioctl,
+		      unsigned long arg)
+{
+	switch (ioctl) {
+	case TPM_IOC_NEW_SPACE:
+		return tpm_ioc_new_space(file, ioctl, arg);
+	default:
+		return -ENOIOCTLCMD;
+	}
+}
+
+#ifdef CONFIG_COMPAT
+static long tpm_compat_ioctl(struct file *file, unsigned int ioctl,
+			     unsigned long arg)
+{
+	return tpm_ioctl(file, ioctl, arg);
+}
+#endif
+
 /*
  * Called on file close
  */
@@ -169,6 +235,14 @@ static int tpm_release(struct inode *inode, struct file *file)
 {
 	struct file_priv *priv = file->private_data;
 
+	if (tpm_try_get_ops(priv->chip)) {
+		mutex_unlock(&priv->buffer_mutex);
+		return -EPIPE;
+	}
+	if (priv->has_space)
+		kfree(priv->space.context_buf);
+	tpm_put_ops(priv->chip);
+
 	del_singleshot_timer_sync(&priv->user_read_timer);
 	flush_work(&priv->work);
 	file->private_data = NULL;
@@ -184,6 +258,10 @@ const struct file_operations tpm_fops = {
 	.open = tpm_open,
 	.read = tpm_read,
 	.write = tpm_write,
+	.unlocked_ioctl = tpm_ioctl,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl = tpm_compat_ioctl,
+#endif
 	.release = tpm_release,
 };
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 0794a5d3..a1ae57e 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)
@@ -684,8 +697,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;
 }
 
@@ -704,7 +717,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)
@@ -802,7 +815,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);
@@ -906,7 +919,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;
@@ -1008,15 +1021,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
@@ -1099,7 +1112,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 fb02b57..c6171e5 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),
@@ -217,6 +225,7 @@ struct tpm_chip {
 #endif /* CONFIG_ACPI */
 
 	struct tpm_buf tr_buf;
+	struct tpm_space work_space;
 	u32 nr_commands;
 	u32 *cc_attrs_tbl;
 };
@@ -492,10 +501,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 *);
@@ -549,4 +559,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 311dc8e..abaa355 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;
@@ -416,7 +416,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 	tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
 	tpm_buf_append_u32(&chip->tr_buf, handle);
 
-	rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, flags,
+	rc = tpm_transmit_cmd(chip, NULL, chip->tr_buf.data, TPM_BUFSIZE, flags,
 			      "flushing context");
 	if (rc)
 		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
@@ -529,7 +529,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	if (chip->tr_buf.flags & TPM_BUF_OVERFLOW)
 		return -E2BIG;
 
-	rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &chip->tr_buf, TPM_BUFSIZE, 0,
 			      "sealing data");
 	if (rc)
 		return rc;
@@ -599,7 +599,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	if (chip->tr_buf.flags & TPM_BUF_OVERFLOW)
 		return -E2BIG;
 
-	rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, flags,
+	rc = tpm_transmit_cmd(chip, NULL, &chip->tr_buf, TPM_BUFSIZE, flags,
 			      "loading blob");
 	if (!rc)
 		*blob_handle = be32_to_cpup(
@@ -644,7 +644,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, flags,
+	rc = tpm_transmit_cmd(chip, NULL, chip->tr_buf.data, TPM_BUFSIZE, flags,
 			      "unsealing");
 	if (rc > 0)
 		rc = -EPERM;
@@ -712,7 +712,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);
 
@@ -746,7 +746,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");
 }
 
@@ -775,7 +775,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.
@@ -838,7 +839,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
@@ -889,7 +890,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;
 
@@ -922,7 +923,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;
 
@@ -981,7 +982,8 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 	tpm_buf_append_u32(&chip->tr_buf, TPM2_CC_FIRST);
 	tpm_buf_append_u32(&chip->tr_buf, nr_commands);
 
-	rc = tpm_transmit_cmd(chip, chip->tr_buf.data, TPM_BUFSIZE, 0, NULL);
+	rc = tpm_transmit_cmd(chip, NULL, chip->tr_buf.data, TPM_BUFSIZE, 0,
+			      NULL);
 	if (rc < 0)
 		goto out;
 
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
new file mode 100644
index 0000000..12a84e6
--- /dev/null
+++ b/drivers/char/tpm/tpm2-space.c
@@ -0,0 +1,288 @@
+/*
+ * 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;
+	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;
+		}
+
+		tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS,
+			     TPM2_CC_CONTEXT_LOAD);
+
+		ctx = (struct tpm2_context *)&space->context_buf[j];
+		s = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
+		tpm_buf_append(&chip->tr_buf, &space->context_buf[j], s);
+
+		rc = tpm_transmit_cmd(chip, NULL, chip->tr_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 *)&chip->tr_buf.data[TPM_HEADER_SIZE]);
+
+		j += s;
+	}
+
+	return 0;
+
+out_err:
+	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;
+	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;
+
+		tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS,
+			     TPM2_CC_CONTEXT_SAVE);
+		tpm_buf_append_u32(&chip->tr_buf, space->context_tbl[i]);
+
+		rc = tpm_transmit_cmd(chip, NULL, chip->tr_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(&chip->tr_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],
+		       &chip->tr_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;
+	}
+
+	return 0;
+
+out_err:
+	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;
+}
diff --git a/include/uapi/linux/tpm.h b/include/uapi/linux/tpm.h
new file mode 100644
index 0000000..1df5b61
--- /dev/null
+++ b/include/uapi/linux/tpm.h
@@ -0,0 +1,23 @@
+/*
+ * API and definitions for the TPM device driver
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
+ *
+ * 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.
+ */
+
+#ifndef _UAPI_TPM_H
+#define _UAPI_TPM_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+#define TPM_IOC_MAGIC 0xa2
+#define TPM_IOC_NEW_SPACE _IO(TPM_IOC_MAGIC, 0x00)
+
+#endif /* _UAPI_TPM_H */
-- 
2.9.3

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

#1549400 — Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0

On Mon, Jan 02, 2017 at 03:22:10PM +0200, Jarkko Sakkinen wrote:
> Added a ioctl for creating a TPM space. The space is isolated from the
> other users of the TPM. Only a process holding the file with the handle
> can access the objects and only objects that are created through that
> file handle can be accessed.

I don't understand this comment. /dev/tpmX is forced to be
single-process-open, so how can there ever be more than 1 FD for it?

Since the space is tied to that single fd these patches just create a
way for the single user-space process to auto-cleanup if it crashes?

Is that the entire intent of this design? I guess it is OK as a
stepping point..

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

Please split this patch so that 'struct tpm_space' introduction is in
its own patch and the actual UAPI change is in a much smaller
patch. It is very hard to see the uapi stuff in all of this churn.

Jason

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

#1549525 — Re: [PATCH RFC 4/4] tpm: add the infrastructure for TPM space for TPM 2.0

On Mon, Jan 02, 2017 at 02:09:53PM -0700, Jason Gunthorpe wrote:
> On Mon, Jan 02, 2017 at 03:22:10PM +0200, Jarkko Sakkinen wrote:
> > Added a ioctl for creating a TPM space. The space is isolated from the
> > other users of the TPM. Only a process holding the file with the handle
> > can access the objects and only objects that are created through that
> > file handle can be accessed.
> 
> I don't understand this comment. /dev/tpmX is forced to be
> single-process-open, so how can there ever be more than 1 FD for it?
> 
> Since the space is tied to that single fd these patches just create a
> way for the single user-space process to auto-cleanup if it crashes?
> 
> Is that the entire intent of this design? I guess it is OK as a
> stepping point..

is_open is cleared in tpm_ioc_new_space.

/Jarkko

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

#1549184 — [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip

Since there is only one thread using TPM chip at a time to transmit data
we can migrate struct tpm_buf to struct tpm_chip. This makes the use of
it more fail safe as the buffer is allocated from heap when the device
is created and not for every transaction.

This is needed characteristic for the resource manager so that we can
minimize the probability of failure for loading, saving and flushings
of TPM contexts.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm-chip.c |   8 +++
 drivers/char/tpm/tpm.h      |  67 ++++++++++-------------
 drivers/char/tpm/tpm2-cmd.c | 128 ++++++++++++++++++--------------------------
 3 files changed, 87 insertions(+), 116 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index eefdc80..41e518e 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->tr_buf.data);
 	kfree(chip);
 }
 
@@ -189,6 +190,13 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	chip->cdev.owner = THIS_MODULE;
 	chip->cdev.kobj.parent = &chip->dev.kobj;
 
+	chip->tr_buf.data = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
+	if (!chip->tr_buf.data) {
+		rc = -ENOMEM;
+		goto out;
+	}
+	chip->tr_buf.size = TPM_BUFSIZE;
+
 	return chip;
 
 out:
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 1ae9768..c74a663 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -153,6 +153,20 @@ struct tpm_chip_seqops {
 	const struct seq_operations *seqops;
 };
 
+/* A string buffer pointer type for constructing TPM commands. Does not own
+ * the data.
+ */
+
+enum tpm_buf_flags {
+	TPM_BUF_OVERFLOW        = BIT(0),
+};
+
+struct tpm_buf {
+	unsigned int flags;
+	u8 *data;
+	unsigned int size;
+};
+
 struct tpm_chip {
 	struct device dev;
 	struct cdev cdev;
@@ -191,6 +205,8 @@ struct tpm_chip {
 	acpi_handle acpi_dev_handle;
 	char ppi_version[TPM_PPI_VERSION_LEN + 1];
 #endif /* CONFIG_ACPI */
+
+	struct tpm_buf tr_buf;
 };
 
 #define to_tpm_chip(d) container_of(d, struct tpm_chip, dev)
@@ -387,57 +403,28 @@ struct tpm_cmd_t {
 	tpm_cmd_params	params;
 } __packed;
 
-/* A string buffer type for constructing TPM commands. This is based on the
- * ideas of string buffer code in security/keys/trusted.h but is heap based
- * in order to keep the stack usage minimal.
- */
-
-enum tpm_buf_flags {
-	TPM_BUF_OVERFLOW	= BIT(0),
-};
+/* A string buffer for constructing TPM commands. */
 
-struct tpm_buf {
-	struct page *data_page;
-	unsigned int flags;
-	u8 *data;
-};
-
-static inline int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
+static inline void tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal)
 {
-	struct tpm_input_header *head;
-
-	buf->data_page = alloc_page(GFP_HIGHUSER);
-	if (!buf->data_page)
-		return -ENOMEM;
-
-	buf->flags = 0;
-	buf->data = kmap(buf->data_page);
-
-	head = (struct tpm_input_header *) buf->data;
+	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
 
+	buf->flags &= ~TPM_BUF_OVERFLOW;
 	head->tag = cpu_to_be16(tag);
 	head->length = cpu_to_be32(sizeof(*head));
 	head->ordinal = cpu_to_be32(ordinal);
-
-	return 0;
-}
-
-static inline void tpm_buf_destroy(struct tpm_buf *buf)
-{
-	kunmap(buf->data_page);
-	__free_page(buf->data_page);
 }
 
 static inline u32 tpm_buf_length(struct tpm_buf *buf)
 {
-	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
 
 	return be32_to_cpu(head->length);
 }
 
 static inline u16 tpm_buf_tag(struct tpm_buf *buf)
 {
-	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
 
 	return be16_to_cpu(head->tag);
 }
@@ -446,14 +433,16 @@ static inline void tpm_buf_append(struct tpm_buf *buf,
 				  const unsigned char *new_data,
 				  unsigned int new_len)
 {
-	struct tpm_input_header *head = (struct tpm_input_header *) buf->data;
+
+	struct tpm_input_header *head = (struct tpm_input_header *)buf->data;
+
 	u32 len = tpm_buf_length(buf);
 
 	/* Return silently if overflow has already happened. */
 	if (buf->flags & TPM_BUF_OVERFLOW)
 		return;
 
-	if ((len + new_len) > PAGE_SIZE) {
+	if ((len + new_len) > buf->size) {
 		WARN(1, "tpm_buf: overflow\n");
 		buf->flags |= TPM_BUF_OVERFLOW;
 		return;
@@ -472,14 +461,14 @@ static inline void tpm_buf_append_u16(struct tpm_buf *buf, const u16 value)
 {
 	__be16 value2 = cpu_to_be16(value);
 
-	tpm_buf_append(buf, (u8 *) &value2, 2);
+	tpm_buf_append(buf, (u8 *)&value2, 2);
 }
 
 static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 {
 	__be32 value2 = cpu_to_be32(value);
 
-	tpm_buf_append(buf, (u8 *) &value2, 4);
+	tpm_buf_append(buf, (u8 *)&value2, 4);
 }
 
 extern struct class *tpm_class;
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 6eda239..f0e0807 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -444,7 +444,6 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		      struct trusted_key_options *options)
 {
 	unsigned int blob_len;
-	struct tpm_buf buf;
 	u32 hash;
 	int i;
 	int rc;
@@ -459,73 +458,67 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 	if (i == ARRAY_SIZE(tpm2_hash_map))
 		return -EINVAL;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
-	if (rc)
-		return rc;
+	tpm_buf_init(&chip->tr_buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE);
 
-	tpm_buf_append_u32(&buf, options->keyhandle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+	tpm_buf_append_u32(&chip->tr_buf, options->keyhandle);
+	tpm2_buf_append_auth(&chip->tr_buf, TPM2_RS_PW,
 			     NULL /* nonce */, 0,
 			     0 /* session_attributes */,
 			     options->keyauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
 	/* sensitive */
-	tpm_buf_append_u16(&buf, 4 + TPM_DIGEST_SIZE + payload->key_len + 1);
+	tpm_buf_append_u16(&chip->tr_buf,
+			   4 + TPM_DIGEST_SIZE + payload->key_len + 1);
 
-	tpm_buf_append_u16(&buf, TPM_DIGEST_SIZE);
-	tpm_buf_append(&buf, options->blobauth, TPM_DIGEST_SIZE);
-	tpm_buf_append_u16(&buf, payload->key_len + 1);
-	tpm_buf_append(&buf, payload->key, payload->key_len);
-	tpm_buf_append_u8(&buf, payload->migratable);
+	tpm_buf_append_u16(&chip->tr_buf, TPM_DIGEST_SIZE);
+	tpm_buf_append(&chip->tr_buf, options->blobauth, TPM_DIGEST_SIZE);
+	tpm_buf_append_u16(&chip->tr_buf, payload->key_len + 1);
+	tpm_buf_append(&chip->tr_buf, payload->key, payload->key_len);
+	tpm_buf_append_u8(&chip->tr_buf, payload->migratable);
 
 	/* public */
-	tpm_buf_append_u16(&buf, 14 + options->policydigest_len);
-	tpm_buf_append_u16(&buf, TPM2_ALG_KEYEDHASH);
-	tpm_buf_append_u16(&buf, hash);
+	tpm_buf_append_u16(&chip->tr_buf, 14 + options->policydigest_len);
+	tpm_buf_append_u16(&chip->tr_buf, TPM2_ALG_KEYEDHASH);
+	tpm_buf_append_u16(&chip->tr_buf, hash);
 
 	/* policy */
 	if (options->policydigest_len) {
-		tpm_buf_append_u32(&buf, 0);
-		tpm_buf_append_u16(&buf, options->policydigest_len);
-		tpm_buf_append(&buf, options->policydigest,
+		tpm_buf_append_u32(&chip->tr_buf, 0);
+		tpm_buf_append_u16(&chip->tr_buf, options->policydigest_len);
+		tpm_buf_append(&chip->tr_buf, options->policydigest,
 			       options->policydigest_len);
 	} else {
-		tpm_buf_append_u32(&buf, TPM2_OA_USER_WITH_AUTH);
-		tpm_buf_append_u16(&buf, 0);
+		tpm_buf_append_u32(&chip->tr_buf, TPM2_OA_USER_WITH_AUTH);
+		tpm_buf_append_u16(&chip->tr_buf, 0);
 	}
 
 	/* public parameters */
-	tpm_buf_append_u16(&buf, TPM2_ALG_NULL);
-	tpm_buf_append_u16(&buf, 0);
+	tpm_buf_append_u16(&chip->tr_buf, TPM2_ALG_NULL);
+	tpm_buf_append_u16(&chip->tr_buf, 0);
 
 	/* outside info */
-	tpm_buf_append_u16(&buf, 0);
+	tpm_buf_append_u16(&chip->tr_buf, 0);
 
 	/* creation PCR */
-	tpm_buf_append_u32(&buf, 0);
+	tpm_buf_append_u32(&chip->tr_buf, 0);
 
-	if (buf.flags & TPM_BUF_OVERFLOW) {
-		rc = -E2BIG;
-		goto out;
-	}
+	if (chip->tr_buf.flags & TPM_BUF_OVERFLOW)
+		return -E2BIG;
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, "sealing data");
+	rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, 0,
+			      "sealing data");
 	if (rc)
-		goto out;
+		return rc;
 
-	blob_len = be32_to_cpup((__be32 *) &buf.data[TPM_HEADER_SIZE]);
-	if (blob_len > MAX_BLOB_SIZE) {
-		rc = -E2BIG;
-		goto out;
-	}
+	blob_len = be32_to_cpup((__be32 *)&chip->tr_buf.data[TPM_HEADER_SIZE]);
+	if (blob_len > MAX_BLOB_SIZE)
+		return -E2BIG;
 
-	memcpy(payload->blob, &buf.data[TPM_HEADER_SIZE + 4], blob_len);
+	memcpy(payload->blob, &chip->tr_buf.data[TPM_HEADER_SIZE + 4],
+	       blob_len);
 	payload->blob_len = blob_len;
 
-out:
-	tpm_buf_destroy(&buf);
-
 	if (rc > 0) {
 		if ((rc & TPM2_RC_HASH) == TPM2_RC_HASH)
 			rc = -EINVAL;
@@ -555,7 +548,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 			 struct trusted_key_options *options,
 			 u32 *blob_handle, unsigned int flags)
 {
-	struct tpm_buf buf;
 	unsigned int private_len;
 	unsigned int public_len;
 	unsigned int blob_len;
@@ -570,31 +562,25 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 	if (blob_len > payload->blob_len)
 		return -E2BIG;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
-	if (rc)
-		return rc;
+	tpm_buf_init(&chip->tr_buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
 
-	tpm_buf_append_u32(&buf, options->keyhandle);
-	tpm2_buf_append_auth(&buf, TPM2_RS_PW,
+	tpm_buf_append_u32(&chip->tr_buf, options->keyhandle);
+	tpm2_buf_append_auth(&chip->tr_buf, TPM2_RS_PW,
 			     NULL /* nonce */, 0,
 			     0 /* session_attributes */,
 			     options->keyauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	tpm_buf_append(&buf, payload->blob, blob_len);
+	tpm_buf_append(&chip->tr_buf, payload->blob, blob_len);
 
-	if (buf.flags & TPM_BUF_OVERFLOW) {
-		rc = -E2BIG;
-		goto out;
-	}
+	if (chip->tr_buf.flags & TPM_BUF_OVERFLOW)
+		return -E2BIG;
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags, "loading blob");
+	rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, flags,
+			      "loading blob");
 	if (!rc)
 		*blob_handle = be32_to_cpup(
-			(__be32 *) &buf.data[TPM_HEADER_SIZE]);
-
-out:
-	tpm_buf_destroy(&buf);
+			(__be32 *)&chip->tr_buf.data[TPM_HEADER_SIZE]);
 
 	if (rc > 0)
 		rc = -EPERM;
@@ -614,25 +600,16 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 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);
+	tpm_buf_init(&chip->tr_buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT);
+	tpm_buf_append_u32(&chip->tr_buf, handle);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, flags,
+	rc = tpm_transmit_cmd(chip, &chip->tr_buf, TPM_BUFSIZE, flags,
 			      "flushing context");
 	if (rc)
 		dev_warn(&chip->dev, "0x%08x was not flushed, rc=%d\n", handle,
 			 rc);
-
-	tpm_buf_destroy(&buf);
 }
 
 /**
@@ -653,17 +630,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			   struct trusted_key_options *options,
 			   u32 blob_handle, unsigned int flags)
 {
-	struct tpm_buf buf;
 	u16 data_len;
 	u8 *data;
 	int rc;
 
-	rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
-	if (rc)
-		return rc;
+	tpm_buf_init(&chip->tr_buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL);
 
-	tpm_buf_append_u32(&buf, blob_handle);
-	tpm2_buf_append_auth(&buf,
+	tpm_buf_append_u32(&chip->tr_buf, blob_handle);
+	tpm2_buf_append_auth(&chip->tr_buf,
 			     options->policyhandle ?
 			     options->policyhandle : TPM2_RS_PW,
 			     NULL /* nonce */, 0,
@@ -671,21 +645,21 @@ 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, chip->tr_buf.data, TPM_BUFSIZE, flags,
+			      "unsealing");
 	if (rc > 0)
 		rc = -EPERM;
 
 	if (!rc) {
 		data_len = be16_to_cpup(
-			(__be16 *) &buf.data[TPM_HEADER_SIZE + 4]);
-		data = &buf.data[TPM_HEADER_SIZE + 6];
+			(__be16 *)&chip->tr_buf.data[TPM_HEADER_SIZE + 4]);
+		data = &chip->tr_buf.data[TPM_HEADER_SIZE + 6];
 
 		memcpy(payload->key, data, data_len - 1);
 		payload->key_len = data_len - 1;
 		payload->migratable = data[data_len - 1];
 	}
 
-	tpm_buf_destroy(&buf);
 	return rc;
 }
 
-- 
2.9.3

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

#1549395 — Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip

On Mon, Jan 02, 2017 at 03:22:07PM +0200, Jarkko Sakkinen wrote:
> Since there is only one thread using TPM chip at a time to transmit data
> we can migrate struct tpm_buf to struct tpm_chip. This makes the use of
> it more fail safe as the buffer is allocated from heap when the device
> is created and not for every transaction.

Eh? What? I don't think that is the case..

We don't serialize until we hit tramsit_cmd at which point the buffer
is already being used and cannot be shared between threads.

Only /dev/tpmX has any sort of locking, but even that is
designed to be optional (eg I patch it out of my kernels), and only
covers userspace, not contention with in-kernel threads.

Why would the resource manager need a single global tpm buffer? That
seems like a big regression from where we have been going. I don't
think this is a good idea to go down this road.

> -	tpm_buf_append(buf, (u8 *) &value2, 4);
> +	tpm_buf_append(buf, (u8 *)&value2, 4);

Please try and avoid this sort of churn in patches that change things..

Jason

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

#1549524 — Re: [PATCH RFC 1/4] tpm: migrate struct tpm_buf to struct tpm_chip

On Mon, Jan 02, 2017 at 02:01:01PM -0700, Jason Gunthorpe wrote:
> On Mon, Jan 02, 2017 at 03:22:07PM +0200, Jarkko Sakkinen wrote:
> > Since there is only one thread using TPM chip at a time to transmit data
> > we can migrate struct tpm_buf to struct tpm_chip. This makes the use of
> > it more fail safe as the buffer is allocated from heap when the device
> > is created and not for every transaction.
> 
> Eh? What? I don't think that is the case..
> 
> We don't serialize until we hit tramsit_cmd at which point the buffer
> is already being used and cannot be shared between threads.

There is a regression in the patch. All functions that use 'tr_buf'
should take tpm_mutex first and use TPM_TRANSMIT_UNLOCKED. There's
also a similar regression in TPM space patch that I have to correct.

> Why would the resource manager need a single global tpm buffer? That
> seems like a big regression from where we have been going. I don't
> think this is a good idea to go down this road.

What? 'tr_buf' is not specifically for resource manager. This commit
makes creating TPM commands more fail-safe because there is no need
to allocate page for every transmit.

For RM decorations this is really important because I rather would have
them fail as rarely as possible. If this would become a scalability
issue then the granularity could be reconsidered.

> > -	tpm_buf_append(buf, (u8 *) &value2, 4);
> > +	tpm_buf_append(buf, (u8 *)&value2, 4);
> 
> Please try and avoid this sort of churn in patches that change things..

It wasn't there on purpose. I do not know how these slipped. I can
clean these up.

> Jason

/Jarkko

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

#1549286 — Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote:
> 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).

The approach looks fine to me.  The only basic query I have is about
the default: shouldn't it be with resource manager on rather than off? 
 I can't really think of a use case that wants the RM off (even if
you're running your own, having another doesn't hurt anything, and it's
still required to share with in-kernel uses).

> There's a test script for trying out TPM spaces in
> 
>   git://git.infradead.org/users/jjs/tpm2-scripts.git
> 
> A simple smoke test can be run by
> 
>   sudo python -m unittest -v tpm2_smoke.SpaceTest   

I've also added an enabling patch to the tss

https://build.opensuse.org/package/view_file/home:jejb1:Tumbleweed/tss2/0002-tssProperties-add-TPM_USE_RESOURCE_MANAGER.patch?expand=1

And with that, I've TPM 2 enabled both gnome-keyring and openssl:

https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/gnome-keyring
https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/openssl_tpm_engine

I'm running them in production on my day to day laptop and so far
everything's working nicely (better than 1.2, in fact, since tcsd
periodically crashes necessitating a restart of everything).

So you can definitely add my Tested-By.

James

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

#1549407 — Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

On Mon, 2017-01-02 at 21:33 +0200, Jarkko Sakkinen wrote:
> On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley wrote:
> > On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote:
> > > 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).
> > 
> > The approach looks fine to me.  The only basic query I have is 
> > about the default: shouldn't it be with resource manager on rather 
> > than off?  I can't really think of a use case that wants the RM off 
> > (even if you're running your own, having another doesn't hurt 
> > anything, and it's still required to share with in-kernel uses).
> 
> This is a valid question and here's a longish explanation.
> 
> In TPM2_GetCapability and maybe couple of other commands you can get
> handles in the response body. I do not want to have special cases in 
> the kernel for response bodies because there is no a generic way to 
> do the substitution. What's worse, new commands in the standard 
> future revisions could have such commands requiring special cases. In
> addition, vendor specific commans could have handles in the response
> bodies.

OK, in general I buy this ... what you're effectively saying is that we
need a non-RM interface for certain management type commands.

However, let me expand a bit on why I'm fretting about the non-RM use
case.  Right at the moment, we have a single TPM device which you use
for access to the kernel TPM.  The current tss2 just makes direct use
of this, meaning it has to have 0666 permissions.  This means that any
local user can simply DoS the TPM by running us out of transient
resources if they don't activate the RM.  If they get a connection
always via the RM, this isn't a worry.  Perhaps the best way of fixing
this is to expose two separate device nodes: one raw to the TPM which
we could keep at 0600 and one with an always RM connection which we can
set to 0666.  That would mean that access to the non-RM connection is
either root only or governed by a system set ACL.

James

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

#1549537 — Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

On Mon, 2017-01-02 at 13:40 -0800, James Bottomley wrote:
> On Mon, 2017-01-02 at 21:33 +0200, Jarkko Sakkinen wrote:
> > On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley wrote:
> > > On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote:
> > > > 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).
> > > 
> > > The approach looks fine to me.  The only basic query I have is 
> > > about the default: shouldn't it be with resource manager on 
> > > rather than off?  I can't really think of a use case that wants 
> > > the RM off (even if you're running your own, having another 
> > > doesn't hurt anything, and it's still required to share with in
> > > -kernel uses).
> > 
> > This is a valid question and here's a longish explanation.
> > 
> > In TPM2_GetCapability and maybe couple of other commands you can 
> > get handles in the response body. I do not want to have special 
> > cases in the kernel for response bodies because there is no a 
> > generic way to do the substitution. What's worse, new commands in 
> > the standard future revisions could have such commands requiring 
> > special cases. In addition, vendor specific commans could have 
> > handles in the response bodies.
> 
> OK, in general I buy this ... what you're effectively saying is that 
> we need a non-RM interface for certain management type commands.
> 
> However, let me expand a bit on why I'm fretting about the non-RM use
> case.  Right at the moment, we have a single TPM device which you use
> for access to the kernel TPM.  The current tss2 just makes direct use
> of this, meaning it has to have 0666 permissions.  This means that 
> any local user can simply DoS the TPM by running us out of transient
> resources if they don't activate the RM.  If they get a connection
> always via the RM, this isn't a worry.  Perhaps the best way of 
> fixing this is to expose two separate device nodes: one raw to the 
> TPM which we could keep at 0600 and one with an always RM connection 
> which we can set to 0666.  That would mean that access to the non-RM 
> connection is either root only or governed by a system set ACL.

OK, so I put a patch together that does this (see below). It all works
nicely (with a udev script that sets the resource manager device to
0666):

jejb@jarvis:~> ls -l /dev/tpm*
crw------- 1 root root  10,   224 Jan  2 20:54 /dev/tpm0
crw-rw-rw- 1 root root 246, 65536 Jan  2 20:54 /dev/tpm0rm

I've modified the tss to connect to /dev/tpm0rm by default and it all
seems to work.

The patch applies on top of your tabrm branch, by the way.

James

---

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index ac4c05f..25b8d30 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;
 
 /**
@@ -169,27 +170,39 @@ 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, "tpm%drm", 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->tr_buf.data = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
 	if (!chip->tr_buf.data) {
@@ -208,6 +221,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);
@@ -252,7 +266,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);
@@ -262,16 +276,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)
@@ -279,6 +321,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-dev.c b/drivers/char/tpm/tpm-dev.c
index 139638b..bed29f9 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -54,23 +54,28 @@ static void timeout_work(struct work_struct *work)
 	mutex_unlock(&priv->buffer_mutex);
 }
 
-static int tpm_open(struct inode *inode, struct file *file)
+static int tpm_open_internal(struct inode *inode, struct file *file, bool is_rm)
 {
-	struct tpm_chip *chip =
-		container_of(inode->i_cdev, struct tpm_chip, cdev);
+	struct tpm_chip *chip;
 	struct file_priv *priv;
 
+	if (is_rm)
+		chip = container_of(inode->i_cdev, struct tpm_chip, cdevrm);
+	else
+		chip = container_of(inode->i_cdev, struct tpm_chip, cdev);
+
 	/* It's assured that the chip will be opened just once,
 	 * by the check of is_open variable, which is protected
 	 * by driver_lock. */
-	if (test_and_set_bit(0, &chip->is_open)) {
+	if (!is_rm && test_and_set_bit(0, &chip->is_open)) {
 		dev_dbg(&chip->dev, "Another process owns this TPM\n");
 		return -EBUSY;
 	}
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (priv == NULL) {
-		clear_bit(0, &chip->is_open);
+		if (!is_rm)
+			clear_bit(0, &chip->is_open);
 		return -ENOMEM;
 	}
 
@@ -82,9 +87,27 @@ static int tpm_open(struct inode *inode, struct file *file)
 	INIT_WORK(&priv->work, timeout_work);
 
 	file->private_data = priv;
+
+	if (is_rm) {
+		priv->space.context_buf =  kzalloc(PAGE_SIZE, GFP_KERNEL);
+		if (!priv->space.context_buf)
+			return -ENOMEM;
+		priv->has_space = true;
+	}
+
 	return 0;
 }
 
+static int tpm_open(struct inode *inode, struct file *file)
+{
+	return tpm_open_internal(inode, file, false);
+}
+
+static int tpm_rm_open(struct inode *inode, struct file *file)
+{
+	return tpm_open_internal(inode, file, true);
+}
+
 static ssize_t tpm_read(struct file *file, char __user *buf,
 			size_t size, loff_t *off)
 {
@@ -169,65 +192,6 @@ static ssize_t tpm_write(struct file *file, const char __user *buf,
 	return in_size;
 }
 
-/**
- * tpm_ioc_new_space - handler for %SGX_IOC_NEW_SPACE ioctl
- *
- * Creates a new TPM space that can hold a set of transient objects. The space
- * is isolated with virtual handles that are mapped into physical handles by the
- * driver.
- */
-static long tpm_ioc_new_space(struct file *file, unsigned int ioctl,
-			      unsigned long arg)
-{
-	struct file_priv *priv = file->private_data;
-	struct tpm_chip *chip = priv->chip;
-	int rc = 0;
-
-	if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
-		return -EOPNOTSUPP;
-
-	mutex_lock(&priv->buffer_mutex);
-
-	if (priv->has_space) {
-		rc = -EBUSY;
-		goto out;
-	}
-
-	priv->space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!priv->space.context_buf) {
-		rc = -ENOMEM;
-		goto out;
-	}
-
-	/* The TPM device can be opened again as this file has been moved to a
-	 * TPM handle space.
-	 */
-	priv->has_space = true;
-	clear_bit(0, &chip->is_open);
-out:
-	mutex_unlock(&priv->buffer_mutex);
-	return rc;
-}
-
-static long tpm_ioctl(struct file *file, unsigned int ioctl,
-		      unsigned long arg)
-{
-	switch (ioctl) {
-	case TPM_IOC_NEW_SPACE:
-		return tpm_ioc_new_space(file, ioctl, arg);
-	default:
-		return -ENOIOCTLCMD;
-	}
-}
-
-#ifdef CONFIG_COMPAT
-static long tpm_compat_ioctl(struct file *file, unsigned int ioctl,
-			     unsigned long arg)
-{
-	return tpm_ioctl(file, ioctl, arg);
-}
-#endif
-
 /*
  * Called on file close
  */
@@ -247,7 +211,8 @@ static int tpm_release(struct inode *inode, struct file *file)
 	flush_work(&priv->work);
 	file->private_data = NULL;
 	atomic_set(&priv->data_pending, 0);
-	clear_bit(0, &priv->chip->is_open);
+	if (!priv->has_space)
+		clear_bit(0, &priv->chip->is_open);
 	kfree(priv);
 	return 0;
 }
@@ -258,10 +223,15 @@ const struct file_operations tpm_fops = {
 	.open = tpm_open,
 	.read = tpm_read,
 	.write = tpm_write,
-	.unlocked_ioctl = tpm_ioctl,
-#ifdef CONFIG_COMPAT
-	.compat_ioctl = tpm_compat_ioctl,
-#endif
+	.release = tpm_release,
+};
+
+const struct file_operations tpm_rm_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.open = tpm_rm_open,
+	.read = tpm_read,
+	.write = tpm_write,
 	.release = tpm_release,
 };
 
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index a1ae57e..c1829de 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1194,9 +1194,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, "tpmrm");
+	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;
 	}
@@ -1208,7 +1216,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 c6171e5..890fb6b 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -186,8 +186,8 @@ struct tpm_buf {
 };
 
 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
@@ -493,8 +493,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 {

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

#1549782 — Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

On Mon, Jan 02, 2017 at 09:26:58PM -0800, James Bottomley wrote:
> On Mon, 2017-01-02 at 13:40 -0800, James Bottomley wrote:
> > On Mon, 2017-01-02 at 21:33 +0200, Jarkko Sakkinen wrote:
> > > On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley wrote:
> > > > On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote:
> > > > > 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).
> > > > 
> > > > The approach looks fine to me.  The only basic query I have is 
> > > > about the default: shouldn't it be with resource manager on 
> > > > rather than off?  I can't really think of a use case that wants 
> > > > the RM off (even if you're running your own, having another 
> > > > doesn't hurt anything, and it's still required to share with in
> > > > -kernel uses).
> > > 
> > > This is a valid question and here's a longish explanation.
> > > 
> > > In TPM2_GetCapability and maybe couple of other commands you can 
> > > get handles in the response body. I do not want to have special 
> > > cases in the kernel for response bodies because there is no a 
> > > generic way to do the substitution. What's worse, new commands in 
> > > the standard future revisions could have such commands requiring 
> > > special cases. In addition, vendor specific commans could have 
> > > handles in the response bodies.
> > 
> > OK, in general I buy this ... what you're effectively saying is that 
> > we need a non-RM interface for certain management type commands.
> > 
> > However, let me expand a bit on why I'm fretting about the non-RM use
> > case.  Right at the moment, we have a single TPM device which you use
> > for access to the kernel TPM.  The current tss2 just makes direct use
> > of this, meaning it has to have 0666 permissions.  This means that 
> > any local user can simply DoS the TPM by running us out of transient
> > resources if they don't activate the RM.  If they get a connection
> > always via the RM, this isn't a worry.  Perhaps the best way of 
> > fixing this is to expose two separate device nodes: one raw to the 
> > TPM which we could keep at 0600 and one with an always RM connection 
> > which we can set to 0666.  That would mean that access to the non-RM 
> > connection is either root only or governed by a system set ACL.
> 
> OK, so I put a patch together that does this (see below). It all works
> nicely (with a udev script that sets the resource manager device to
> 0666):

This is not yet a comment about this suggestion but I guess one thing
is clear: the stuff in tpm2-space.c and tpm-interface.c changes are the
thing that we can mostly agree on and the area of argumentation is the
user space interface to it?

Just thinking how to split up the non-RFC patch set. This was also what
Jason suggested if I understood his remark correctly.

/Jarkko

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

#1549920 — Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

On Tue, 2017-01-03 at 15:41 +0200, Jarkko Sakkinen wrote:
> On Mon, Jan 02, 2017 at 09:26:58PM -0800, James Bottomley wrote:
> > On Mon, 2017-01-02 at 13:40 -0800, James Bottomley wrote:
> > > On Mon, 2017-01-02 at 21:33 +0200, Jarkko Sakkinen wrote:
> > > > On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley
> > > > wrote:
> > > > > On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote:
> > > > > > 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).
> > > > > 
> > > > > The approach looks fine to me.  The only basic query I have 
> > > > > is about the default: shouldn't it be with resource manager 
> > > > > on rather than off?  I can't really think of a use case that
> > > > > wants the RM off (even if you're running your own, having 
> > > > > another doesn't hurt anything, and it's still required to 
> > > > > share with in-kernel uses).
> > > > 
> > > > This is a valid question and here's a longish explanation.
> > > > 
> > > > In TPM2_GetCapability and maybe couple of other commands you 
> > > > can get handles in the response body. I do not want to have 
> > > > special cases in the kernel for response bodies because there 
> > > > is no a generic way to do the substitution. What's worse, new 
> > > > commands in the standard future revisions could have such 
> > > > commands requiring special cases. In addition, vendor specific 
> > > > commans could have handles in the response bodies.
> > > 
> > > OK, in general I buy this ... what you're effectively saying is 
> > > that we need a non-RM interface for certain management type
> > > commands.
> > > 
> > > However, let me expand a bit on why I'm fretting about the non-RM 
> > > use case.  Right at the moment, we have a single TPM device which 
> > > you use for access to the kernel TPM.  The current tss2 just 
> > > makes direct use of this, meaning it has to have 0666 
> > > permissions.  This means that any local user can simply DoS the 
> > > TPM by running us out of transient resources if they don't 
> > > activate the RM.  If they get a connection always via the RM, 
> > > this isn't a worry.  Perhaps the best way of fixing this is to 
> > > expose two separate device nodes: one raw to the TPM which we 
> > > could keep at 0600 and one with an always RM connection
> > > which we can set to 0666.  That would mean that access to the non
> > > -RM connection is either root only or governed by a system set
> > > ACL.
> > 
> > OK, so I put a patch together that does this (see below). It all 
> > works nicely (with a udev script that sets the resource manager 
> > device to 0666):
> 
> This is not yet a comment about this suggestion but I guess one thing
> is clear: the stuff in tpm2-space.c and tpm-interface.c changes are 
> the thing that we can mostly agree on and the area of argumentation 
> is the user space interface to it?

Agreed.  As I've already said, the space and interface code is working
well for me in production on my laptop.

> Just thinking how to split up the non-RFC patch set. This was also 
> what Jason suggested if I understood his remark correctly.

SUre ... let's get agreement on how we move forward first.  How the
patch is activated by the user has to be sorted out as well before it
can go in, but it doesn't have to be the first thing we do.  I'm happy
to continue playing with the interfaces to see what works and what
doesn't.  My main current feedback is that I think separate devices
works way better than an ioctl becuase the separate devices approach
allows differing system policies for who accesses the RM backed TPM vs
who accesses the raw one.

James

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

#1549790 — Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

On Mon, Jan 02, 2017 at 01:40:48PM -0800, James Bottomley wrote:
> On Mon, 2017-01-02 at 21:33 +0200, Jarkko Sakkinen wrote:
> > On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley wrote:
> > > On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote:
> > > > 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).
> > > 
> > > The approach looks fine to me.  The only basic query I have is 
> > > about the default: shouldn't it be with resource manager on rather 
> > > than off?  I can't really think of a use case that wants the RM off 
> > > (even if you're running your own, having another doesn't hurt 
> > > anything, and it's still required to share with in-kernel uses).
> > 
> > This is a valid question and here's a longish explanation.
> > 
> > In TPM2_GetCapability and maybe couple of other commands you can get
> > handles in the response body. I do not want to have special cases in 
> > the kernel for response bodies because there is no a generic way to 
> > do the substitution. What's worse, new commands in the standard 
> > future revisions could have such commands requiring special cases. In
> > addition, vendor specific commans could have handles in the response
> > bodies.
> 
> OK, in general I buy this ... what you're effectively saying is that we
> need a non-RM interface for certain management type commands.

Not only that.

Doing virtualization for commands like GetCapability is just a better
fit for doing in the user space. You could have a thin translation layer
in your TSS library for example to handle these specific messages.

> However, let me expand a bit on why I'm fretting about the non-RM use
> case.  Right at the moment, we have a single TPM device which you use
> for access to the kernel TPM.  The current tss2 just makes direct use
> of this, meaning it has to have 0666 permissions.  This means that any
> local user can simply DoS the TPM by running us out of transient
> resources if they don't activate the RM.  If they get a connection
> always via the RM, this isn't a worry.  Perhaps the best way of fixing
> this is to expose two separate device nodes: one raw to the TPM which
> we could keep at 0600 and one with an always RM connection which we can
> set to 0666.  That would mean that access to the non-RM connection is
> either root only or governed by a system set ACL.

I'm not sure about this. Why you couldn't have a very thin daemon that
prepares the file descriptor and sends it through UDS socket to a
client.  The non-RFC version will also have whitelisting ioctl for
further restricting the file descriptor to only specific TPM commands.

This is also architecture I preseted in my LSS presentation and I think
it makes sense especially when I add the whitelisting to the pack.

> James

I'm more dilated to keep things way they are now. I'll stick to that at
least with the first non-RFC version and hopefully get the tpm2-space.c
part reviewed as I split that stuff to a separate commit.

/Jarkko

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

#1549936 — Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

On Tue, 2017-01-03 at 15:51 +0200, Jarkko Sakkinen wrote:
> On Mon, Jan 02, 2017 at 01:40:48PM -0800, James Bottomley wrote:
> > On Mon, 2017-01-02 at 21:33 +0200, Jarkko Sakkinen wrote:
> > > On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley wrote:
> > > > On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote:
> > > > > 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).
> > > > 
> > > > The approach looks fine to me.  The only basic query I have is 
> > > > about the default: shouldn't it be with resource manager on 
> > > > rather than off?  I can't really think of a use case that wants 
> > > > the RM off (even if you're running your own, having another 
> > > > doesn't hurt anything, and it's still required to share with in
> > > > -kernel uses).
> > > 
> > > This is a valid question and here's a longish explanation.
> > > 
> > > In TPM2_GetCapability and maybe couple of other commands you can 
> > > get handles in the response body. I do not want to have special 
> > > cases in the kernel for response bodies because there is no a 
> > > generic way to do the substitution. What's worse, new commands in 
> > > the standard future revisions could have such commands requiring 
> > > special cases. In addition, vendor specific commans could have 
> > > handles in the response bodies.
> > 
> > OK, in general I buy this ... what you're effectively saying is 
> > that we need a non-RM interface for certain management type
> > commands.
> 
> Not only that.
> 
> Doing virtualization for commands like GetCapability is just a better
> fit for doing in the user space. You could have a thin translation 
> layer in your TSS library for example to handle these specific
> messages.

Yes, we could do it that way too.  To be honest I can't see much use
for getting the transient handles and all the other handles you'd be
interested in aren't virtualized.

> > However, let me expand a bit on why I'm fretting about the non-RM 
> > use case.  Right at the moment, we have a single TPM device which 
> > you use for access to the kernel TPM.  The current tss2 just makes 
> > direct use of this, meaning it has to have 0666 permissions.  This 
> > means that any local user can simply DoS the TPM by running us out 
> > of transient resources if they don't activate the RM.  If they get 
> > a connection always via the RM, this isn't a worry.  Perhaps the 
> > best way of fixing this is to expose two separate device nodes: one 
> > raw to the TPM which we could keep at 0600 and one with an always 
> > RM connection which we can set to 0666.  That would mean that 
> > access to the non-RM connection is either root only or governed by
> > a system set ACL.
> 
> I'm not sure about this. Why you couldn't have a very thin daemon 
> that prepares the file descriptor and sends it through UDS socket to 
> a client.

So I'm a bit soured on daemons from the trousers experience: tcsd
crashed regularly and when it did it took all the TPM connections down
irrecoverably.  I'm not saying we can't write a stateless daemon to fix
most of the trousers issues, but I think it's valuable first to ask the
question, "can we manage without a daemon at all?"  I actually think
the answer is "yes", so I'm interested in seeing how far that line of
research gets us.

>   The non-RFC version will also have whitelisting ioctl for
> further restricting the file descriptor to only specific TPM
> commands.
> 
> This is also architecture I preseted in my LSS presentation and I 
> think it makes sense especially when I add the whitelisting to the
> pack.

Do you have a link to the presentation?  The Plumbers etherpad doesn't
contain it.  I've been trying to work out whether a properly set up TPM
actually does need any protections at all.  As far as I can tell, once
you've set all the hierarchy authorities and the lockout one, you're
pretty well protected.

> > James
> 
> I'm more dilated to keep things way they are now. I'll stick to that 
> at least with the first non-RFC version and hopefully get the tpm2
> -space.c part reviewed as I split that stuff to a separate commit.

Sure, we need the patch in an acceptable form first.  I'll keep
worrying about the systems implications, but I can layer playing with
those on top of what you do.

James

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

#1549522 — Re: [tpmdd-devel] [PATCH RFC 0/4] RFC: in-kernel resource manager

On Mon, Jan 02, 2017 at 08:36:20AM -0800, James Bottomley wrote:
> On Mon, 2017-01-02 at 15:22 +0200, Jarkko Sakkinen wrote:
> > 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).
> 
> The approach looks fine to me.  The only basic query I have is about
> the default: shouldn't it be with resource manager on rather than off? 
>  I can't really think of a use case that wants the RM off (even if
> you're running your own, having another doesn't hurt anything, and it's
> still required to share with in-kernel uses).

This is a valid question and here's a longish explanation.

In TPM2_GetCapability and maybe couple of other commands you can get
handles in the response body. I do not want to have special cases in the
kernel for response bodies because there is no a generic way to do the
substitution. What's worse, new commands in the standard future
revisions could have such commands requiring special cases. In addition,
vendor specific commans could have handles in the response bodies.

It's better to leverage that to the user space. I would do only simple
and fail-safe stuff in the kernel.

Turning RM on by default would raise a backwards compatibility issue.

> 
> > There's a test script for trying out TPM spaces in
> > 
> >   git://git.infradead.org/users/jjs/tpm2-scripts.git
> > 
> > A simple smoke test can be run by
> > 
> >   sudo python -m unittest -v tpm2_smoke.SpaceTest   
> 
> I've also added an enabling patch to the tss
> 
> https://build.opensuse.org/package/view_file/home:jejb1:Tumbleweed/tss2/0002-tssProperties-add-TPM_USE_RESOURCE_MANAGER.patch?expand=1
> 
> And with that, I've TPM 2 enabled both gnome-keyring and openssl:
> 
> https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/gnome-keyring
> https://build.opensuse.org/package/show/home:jejb1:Tumbleweed/openssl_tpm_engine
> 
> I'm running them in production on my day to day laptop and so far
> everything's working nicely (better than 1.2, in fact, since tcsd
> periodically crashes necessitating a restart of everything).

Great, thanks for doing this!

> So you can definitely add my Tested-By.

Thank you.

> James

/Jarkko

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web