[PATCH v2 0/7] in-kernel resource manager

[PATCH v2 0/7] in-kernel resource manager

[Jarkko Sakkinen]

This patch set adds support for TPM spaces that provide an isolated
execution context for transient objects and HMAC and policy sessions. A
space is swapped into TPM volatile memory only when it is used and
swapped out after the use.

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

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

A simple smoke test suite can be run by

  sudo python -m unittest -v tpm2_smoke.SpaceTest   

v2:
* Substitute virtual handle in ContextSave.
* Substitute virtual handles in GetCapability.
* Validate that the real response length and the one reported in the
  header match in tpm_transmit().

James Bottomley (3):
  tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c
  tpm: expose spaces via a device link /dev/tpms<n>
  tpm2: add session handle context saving and restoring to the space
    code

Jarkko Sakkinen (4):
  tpm: move length validation to tpm_transmit()
  tpm: validate TPM 2.0 commands
  tpm: export tpm2_flush_context_cmd
  tpm: infrastructure for TPM spaces

 drivers/char/tpm/Makefile         |   3 +-
 drivers/char/tpm/tpm-chip.c       |  86 +++++-
 drivers/char/tpm/tpm-dev-common.c | 148 +++++++++++
 drivers/char/tpm/tpm-dev.c        | 143 +---------
 drivers/char/tpm/tpm-dev.h        |  27 ++
 drivers/char/tpm/tpm-interface.c  | 131 +++++++---
 drivers/char/tpm/tpm-sysfs.c      |   2 +-
 drivers/char/tpm/tpm.h            |  49 +++-
 drivers/char/tpm/tpm2-cmd.c       | 168 ++++++++----
 drivers/char/tpm/tpm2-space.c     | 532 ++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpms-dev.c       |  65 +++++
 11 files changed, 1135 insertions(+), 219 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

[PATCH v2 1/7] tpm: move length validation to tpm_transmit()

[Jarkko Sakkinen]

Check that the length matches the length reported by the response
header already in tpm_transmit() to improve validation.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm-interface.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index bd2128e..708d356 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -343,6 +343,7 @@ EXPORT_SYMBOL_GPL(tpm_calc_ordinal_duration);
 ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
 		     unsigned int flags)
 {
+	const struct tpm_output_header *header = (void *)buf;
 	ssize_t rc;
 	u32 count, ordinal;
 	unsigned long stop;
@@ -406,9 +407,18 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
 
 out_recv:
 	rc = chip->ops->recv(chip, (u8 *) buf, bufsiz);
-	if (rc < 0)
+	if (rc < 0) {
 		dev_err(&chip->dev,
 			"tpm_transmit: tpm_recv: error %zd\n", rc);
+		goto out;
+	} else if (rc < TPM_HEADER_SIZE) {
+		rc = -EFAULT;
+		goto out;
+	}
+
+	if (rc != be32_to_cpu(header->length))
+		goto out;
+
 out:
 	if (chip->dev.parent)
 		pm_runtime_put_sync(chip->dev.parent);
@@ -438,19 +448,13 @@ ssize_t tpm_transmit_cmd(struct tpm_chip *chip, const void *buf,
 			 size_t bufsiz, size_t min_rsp_body_length,
 			 unsigned int flags, const char *desc)
 {
-	const struct tpm_output_header *header;
+	const struct tpm_output_header *header = buf;
 	int err;
 	ssize_t len;
 
 	len = tpm_transmit(chip, (const u8 *)buf, bufsiz, flags);
 	if (len <  0)
 		return len;
-	else if (len < TPM_HEADER_SIZE)
-		return -EFAULT;
-
-	header = buf;
-	if (len != be32_to_cpu(header->length))
-		return -EFAULT;
 
 	err = be32_to_cpu(header->return_code);
 	if (err != 0 && desc)
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

[PATCH v2 2/7] tpm: validate TPM 2.0 commands

[Jarkko Sakkinen]

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.

For ContextSave and FlushContext we mark the body to be part of the
handle area. This gives validation for these commands at zero
cost, including the body of the command.

The more important reason for this is that we can virtualize these
commands in the same way as you would virtualize the handle area of a
command.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm-interface.c | 38 ++++++++++++++++++-
 drivers/char/tpm/tpm.h           | 15 ++++++++
 drivers/char/tpm/tpm2-cmd.c      | 79 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 708d356..20b1fe3 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -328,6 +328,42 @@ 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;
+	int i;
+	u32 cc;
+	u32 attrs;
+	unsigned int nr_handles;
+
+	if (len < TPM_HEADER_SIZE)
+		return false;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2 && chip->nr_commands) {
+		cc = be32_to_cpu(header->ordinal);
+
+		i = tpm2_find_cc(chip, cc);
+		if (i < 0) {
+			dev_dbg(&chip->dev, "0x%04X is an invalid command\n",
+				cc);
+			return false;
+		}
+
+		attrs = chip->cc_attrs_tbl[i];
+		nr_handles =
+			4 * ((attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0));
+		if (len < TPM_HEADER_SIZE + 4 * nr_handles)
+			goto err_len;
+	}
+
+	return true;
+err_len:
+	dev_dbg(&chip->dev,
+		"%s: insufficient command length %zu", __func__, len);
+	return false;
+}
+
 /**
  * tmp_transmit - Internal kernel interface to transmit TPM commands.
  *
@@ -348,7 +384,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 6b4e7aa..50aebe9 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -114,6 +114,7 @@ enum tpm2_command_codes {
 	TPM2_CC_CREATE		= 0x0153,
 	TPM2_CC_LOAD		= 0x0157,
 	TPM2_CC_UNSEAL		= 0x015E,
+	TPM2_CC_CONTEXT_SAVE	= 0x0162,
 	TPM2_CC_FLUSH_CONTEXT	= 0x0165,
 	TPM2_CC_GET_CAPABILITY	= 0x017A,
 	TPM2_CC_GET_RANDOM	= 0x017B,
@@ -127,15 +128,25 @@ enum tpm2_permanent_handles {
 };
 
 enum tpm2_capabilities {
+	TPM2_CAP_COMMANDS	= 2,
 	TPM2_CAP_PCRS		= 5,
 	TPM2_CAP_TPM_PROPERTIES = 6,
 };
 
+enum tpm2_properties {
+	TPM_PT_TOTAL_COMMANDS	= 0x0129,
+};
+
 enum tpm2_startup_types {
 	TPM2_SU_CLEAR	= 0x0000,
 	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
@@ -199,6 +210,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)
@@ -555,4 +569,5 @@ 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);
 ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
+int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 10f97e6..7a56f44 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -997,6 +997,70 @@ int tpm2_probe(struct tpm_chip *chip)
 }
 EXPORT_SYMBOL_GPL(tpm2_probe);
 
+static int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
+{
+	struct tpm_buf buf;
+	u32 nr_commands;
+	u32 *attrs;
+	u32 cc;
+	int i;
+	int rc;
+
+	rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands, NULL);
+	if (rc)
+		goto out;
+
+	if (nr_commands > 0xFFFFF) {
+		rc = -EFAULT;
+		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, 9 + 4 * nr_commands,
+			      0, NULL);
+	if (rc) {
+		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;
+	}
+
+	chip->nr_commands = nr_commands;
+
+	attrs = (u32 *)&buf.data[TPM_HEADER_SIZE + 9];
+	for (i = 0; i < nr_commands; i++, attrs++) {
+		chip->cc_attrs_tbl[i] = be32_to_cpup(attrs);
+		cc = chip->cc_attrs_tbl[i] & 0xFFFF;
+
+		if (cc == TPM2_CC_CONTEXT_SAVE || cc == TPM2_CC_FLUSH_CONTEXT) {
+			chip->cc_attrs_tbl[i] &=
+				~(GENMASK(2, 0) << TPM2_CC_ATTR_CHANDLES);
+			chip->cc_attrs_tbl[i] |= 1 << TPM2_CC_ATTR_CHANDLES;
+		}
+	}
+
+	tpm_buf_destroy(&buf);
+
+out:
+	if (rc > 0)
+		rc = -ENODEV;
+	return rc;
+}
+
 /**
  * tpm2_auto_startup - Perform the standard automatic TPM initialization
  *                     sequence
@@ -1031,6 +1095,10 @@ int tpm2_auto_startup(struct tpm_chip *chip)
 	}
 
 	rc = tpm2_get_pcr_allocation(chip);
+	if (rc)
+		goto out;
+
+	rc = tpm2_get_cc_attrs_tbl(chip);
 
 out:
 	if (rc > 0)
@@ -1114,3 +1182,14 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 
 	return rc;
 }
+
+int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
+{
+	int i;
+
+	for (i = 0; i < chip->nr_commands; i++)
+		if (cc == (chip->cc_attrs_tbl[i] & GENMASK(15, 0)))
+			return i;
+
+	return -1;
+}
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

[PATCH v2 3/7] tpm: export tpm2_flush_context_cmd

[Jarkko Sakkinen]

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/char/tpm/tpm.h      |  2 ++
 drivers/char/tpm/tpm2-cmd.c | 62 +++++++++++++++++++++------------------------
 2 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 50aebe9..0ec1cf0 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -555,6 +555,8 @@ 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, u32 count,
 		    struct tpm2_digest *digests);
 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 7a56f44..897902a 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -419,6 +419,35 @@ 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);
+
+	(void) tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, flags,
+				"flushing context");
+
+	tpm_buf_destroy(&buf);
+}
+
+/**
  * tpm_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer.
  *
  * @buf: an allocated tpm_buf instance
@@ -628,39 +657,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, 0, 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


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

[PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Jarkko Sakkinen]

Added an ability to virtualize TPM commands into an isolated context
that we call a TPM space because the word context is already heavily
used in the TPM specification. Both the handle areas and bodies (where
necessary) are virtualized.

The mechanism works by adding a new parameter struct tpm_space to the
tpm_transmit() function. This new structure contains the list of virtual
handles and a buffer of page size (currently) for backing storage.

When tpm_transmit() is called with a struct tpm_space instance it will
execute the following sequence:

1. Take locks.
2. Load transient objects from the backing storage by using ContextLoad
   and map virtual handles to physical handles.
3. Perform the transaction.
4. Save transient objects to backing storage by using ContextSave and
   map resulting physical handle to virtual handle if there is such.

This commit does not implement virtualization support for hmac and
policy sessions.

Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 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 |  68 +++---
 drivers/char/tpm/tpm-sysfs.c     |   2 +-
 drivers/char/tpm/tpm.h           |  26 ++-
 drivers/char/tpm/tpm2-cmd.c      |  33 +--
 drivers/char/tpm/tpm2-space.c    | 431 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 520 insertions(+), 51 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2-space.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 3d386a8..8f07fcf 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 \
-		tpm1_eventlog.o tpm2_eventlog.o
+	 tpm1_eventlog.o tpm2_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 02a8850..414553b 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -147,7 +147,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 20b1fe3..db5ffe9 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -376,11 +376,12 @@ 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)
 {
-	const struct tpm_output_header *header = (void *)buf;
-	ssize_t rc;
+	struct tpm_output_header *header = (void *)buf;
+	int rc;
+	ssize_t len = 0;
 	u32 count, ordinal;
 	unsigned long stop;
 
@@ -406,10 +407,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);
+	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;
 	}
 
@@ -442,18 +447,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;
-	} else if (rc < TPM_HEADER_SIZE) {
+	} else if (len < TPM_HEADER_SIZE) {
 		rc = -EFAULT;
 		goto out;
 	}
 
-	if (rc != be32_to_cpu(header->length))
+	if (len != be32_to_cpu(header->length)) {
+		rc = -EFAULT;
 		goto out;
+	}
+
+	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
 
 out:
 	if (chip->dev.parent)
@@ -461,7 +471,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
 
 	if (!(flags & TPM_TRANSMIT_UNLOCKED))
 		mutex_unlock(&chip->tpm_mutex);
-	return rc;
+	return rc ? rc : len;
 }
 
 /**
@@ -480,15 +490,16 @@ 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 *buf,
-			 size_t bufsiz, size_t min_rsp_body_length,
-			 unsigned int flags, const char *desc)
+ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
+			 const void *buf, size_t bufsiz,
+			 size_t min_rsp_body_length, unsigned int flags,
+			 const char *desc)
 {
 	const struct tpm_output_header *header = buf;
 	int err;
 	ssize_t len;
 
-	len = tpm_transmit(chip, (const u8 *)buf, bufsiz, flags);
+	len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
 	if (len <  0)
 		return len;
 
@@ -541,7 +552,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,
+	rc = tpm_transmit_cmd(chip, NULL, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
 			      min_cap_length, 0, desc);
 	if (!rc)
 		*cap = tpm_cmd.params.getcap_out.cap;
@@ -565,7 +576,8 @@ 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,
+	return tpm_transmit_cmd(chip, NULL, &start_cmd,
+				TPM_INTERNAL_RESULT_SIZE, 0,
 				0, "attempting to start the TPM");
 }
 
@@ -722,8 +734,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, 0,
-			      "continue selftest");
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
+			      0, 0, "continue selftest");
 	return rc;
 }
 
@@ -743,7 +755,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,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, READ_PCR_RESULT_SIZE,
 			      READ_PCR_RESULT_BODY_SIZE, 0,
 			      "attempting to read a pcr value");
 
@@ -855,7 +867,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,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE,
 			      EXTEND_PCR_RESULT_BODY_SIZE, 0,
 			      "attempting extend a PCR value");
 
@@ -960,8 +972,8 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
 	if (chip == NULL)
 		return -ENODEV;
 
-	rc = tpm_transmit_cmd(chip, cmd, buflen, 0, 0, "attempting tpm_cmd");
-
+	rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
+			      "attempting tpm_cmd");
 	tpm_put_ops(chip);
 	return rc;
 }
@@ -1062,16 +1074,16 @@ 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,
-				     EXTEND_PCR_RESULT_BODY_SIZE, 0,
+		rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE,
+				      EXTEND_PCR_RESULT_BODY_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,
-				      0, NULL);
+		rc = tpm_transmit_cmd(chip, NULL, &cmd, SAVESTATE_RESULT_SIZE,
+				      0, 0, NULL);
 
 		/*
 		 * If the TPM indicates that it is too busy to respond to
@@ -1154,7 +1166,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,
 				       offsetof(struct tpm_getrandom_out,
 						rng_data),
diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
index 2f596d7..55405db 100644
--- a/drivers/char/tpm/tpm-sysfs.c
+++ b/drivers/char/tpm/tpm-sysfs.c
@@ -40,7 +40,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,
+	err = tpm_transmit_cmd(chip, NULL, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
 			       READ_PUBEK_RESULT_MIN_BODY_SIZE, 0,
 			       "attempting to read the PUBEK");
 	if (err)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 0ec1cf0..97e48a4 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -89,10 +89,13 @@ 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 */
 	TPM2_RC_DISABLED	= 0x0120,
 	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
+	TPM2_RC_REFERENCE_H0	= 0x0910,
 };
 
 enum tpm2_algorithms {
@@ -114,6 +117,7 @@ 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,
@@ -128,6 +132,7 @@ enum tpm2_permanent_handles {
 };
 
 enum tpm2_capabilities {
+	TPM2_CAP_HANDLES	= 1,
 	TPM2_CAP_COMMANDS	= 2,
 	TPM2_CAP_PCRS		= 5,
 	TPM2_CAP_TPM_PROPERTIES = 6,
@@ -153,6 +158,11 @@ enum tpm2_cc_attrs {
 
 #define TPM_PPI_VERSION_LEN		3
 
+struct tpm_space {
+	u32 context_tbl[3];
+	u8 *context_buf;
+};
+
 enum tpm_chip_flags {
 	TPM_CHIP_FLAG_TPM2		= BIT(1),
 	TPM_CHIP_FLAG_IRQ		= BIT(2),
@@ -211,6 +221,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;
 };
@@ -507,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 *buf, size_t bufsiz,
-			 size_t min_rsp_body_len, 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 tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
+			 const void *buf, size_t bufsiz,
+			 size_t min_rsp_body_length, unsigned int flags,
 			 const char *desc);
 ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
 		   const char *desc, size_t min_cap_length);
@@ -572,4 +584,10 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
 int tpm2_probe(struct tpm_chip *chip);
 ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
 int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
+int tpm2_init_space(struct tpm_space *space);
+void tpm2_del_space(struct tpm_space *space);
+int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
+		       u8 *cmd);
+int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
+		      u32 cc, u8 *buf, size_t *bufsiz);
 #endif
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index 897902a..96121b3 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -266,7 +266,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),
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
 			      TPM2_PCR_READ_RESP_BODY_SIZE,
 			      0, "attempting to read a pcr value");
 	if (rc == 0) {
@@ -333,7 +333,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
 		}
 	}
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, 0,
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
 			      "attempting extend a PCR value");
 
 	tpm_buf_destroy(&buf);
@@ -382,7 +382,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),
+		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
 				       offsetof(struct tpm2_get_random_out,
 						buffer),
 				       0, "attempting get random");
@@ -441,7 +441,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
 
 	tpm_buf_append_u32(&buf, handle);
 
-	(void) tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, flags,
+	(void) tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, flags,
 				"flushing context");
 
 	tpm_buf_destroy(&buf);
@@ -557,7 +557,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 4, 0,
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
 			      "sealing data");
 	if (rc)
 		goto out;
@@ -641,7 +641,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
 		goto out;
 	}
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 4, flags,
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, flags,
 			      "loading blob");
 	if (!rc)
 		*blob_handle = be32_to_cpup(
@@ -693,7 +693,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
 			     options->blobauth /* hmac */,
 			     TPM_DIGEST_SIZE);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 6, flags,
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 6, flags,
 			      "unsealing");
 	if (rc > 0)
 		rc = -EPERM;
@@ -770,7 +770,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),
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
 			      TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
 	if (!rc)
 		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
@@ -805,7 +805,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, 0,
+	return tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
 				"attempting to start the TPM");
 }
 
@@ -834,7 +834,7 @@ 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, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
 			      "stopping the TPM");
 
 	/* In places where shutdown command is sent there's no much we can do
@@ -898,7 +898,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, 0,
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE, 0, 0,
 			      "continue selftest");
 
 	/* At least some prototype chips seem to give RC_TESTING error
@@ -949,7 +949,8 @@ 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, 0, NULL);
+		rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
+				      NULL);
 		if (rc < 0)
 			break;
 
@@ -982,7 +983,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, 0, NULL);
+	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
 	if (rc <  0)
 		return rc;
 
@@ -1022,8 +1023,8 @@ static int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
 	tpm_buf_append_u32(&buf, TPM2_CC_FIRST);
 	tpm_buf_append_u32(&buf, nr_commands);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 9 + 4 * nr_commands,
-			      0, NULL);
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
+			      9 + 4 * nr_commands, 0, NULL);
 	if (rc) {
 		tpm_buf_destroy(&buf);
 		goto out;
@@ -1136,7 +1137,7 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
 	tpm_buf_append_u32(&buf, 0);
 	tpm_buf_append_u32(&buf, 1);
 
-	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 9, 0,
+	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 9, 0,
 			      "get tpm pcr allocation");
 	if (rc)
 		goto out;
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
new file mode 100644
index 0000000..e955548
--- /dev/null
+++ b/drivers/char/tpm/tpm2-space.c
@@ -0,0 +1,431 @@
+/*
+ * Copyright (C) 2016 Intel Corporation
+ *
+ * Authors:
+ * Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
+ *
+ * Maintained by: <tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
+ *
+ * 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 <asm/unaligned.h>
+#include "tpm.h"
+
+enum tpm2_handle_types {
+	TPM2_HT_HMAC_SESSION	= 0x02000000,
+	TPM2_HT_POLICY_SESSION	= 0x03000000,
+	TPM2_HT_TRANSIENT	= 0x80000000,
+};
+
+struct tpm2_context {
+	__be64 sequence;
+	__be32 saved_handle;
+	__be32 hierarchy;
+	__be16 blob_size;
+} __packed;
+
+int tpm2_init_space(struct tpm_space *space)
+{
+	space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!space->context_buf)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void tpm2_del_space(struct tpm_space *space)
+{
+	kfree(space->context_buf);
+}
+
+static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
+			     unsigned int *offset, u32 *handle)
+{
+	struct tpm_buf tbuf;
+	struct tpm2_context *ctx;
+	unsigned int body_size;
+	int rc;
+
+	rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD);
+	if (rc)
+		return rc;
+
+	ctx = (struct tpm2_context *)&buf[*offset];
+	body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
+	tpm_buf_append(&tbuf, &buf[*offset], body_size);
+
+	rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 4,
+			      TPM_TRANSMIT_UNLOCKED, NULL);
+	if (rc < 0) {
+		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
+			 __func__, rc);
+		tpm_buf_destroy(&tbuf);
+		return -EFAULT;
+	} else if (rc > 0) {
+		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
+			 __func__, rc);
+		tpm_buf_destroy(&tbuf);
+		return -EFAULT;
+	}
+
+	*handle = be32_to_cpup((__be32 *)&tbuf.data[TPM_HEADER_SIZE]);
+	*offset += body_size;
+
+	tpm_buf_destroy(&tbuf);
+	return 0;
+}
+
+static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
+			     unsigned int buf_size, unsigned int *offset)
+{
+	struct tpm_buf tbuf;
+	unsigned int body_size;
+	int rc;
+
+	rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE);
+	if (rc)
+		return rc;
+
+	tpm_buf_append_u32(&tbuf, handle);
+
+	rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 0,
+			      TPM_TRANSMIT_UNLOCKED, NULL);
+	if (rc < 0) {
+		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
+			 __func__, rc);
+		tpm_buf_destroy(&tbuf);
+		return -EFAULT;
+	} else if (tpm2_rc_value(rc) == TPM2_RC_REFERENCE_H0) {
+		tpm_buf_destroy(&tbuf);
+		return -ENOENT;
+	} else if (rc) {
+		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
+			 __func__, rc);
+		tpm_buf_destroy(&tbuf);
+		return -EFAULT;
+	}
+
+	body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE;
+	if ((*offset + body_size) > buf_size) {
+		dev_warn(&chip->dev, "%s: out of backing storage\n", __func__);
+		tpm_buf_destroy(&tbuf);
+		return -ENOMEM;
+	}
+
+	memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size);
+	tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED);
+	*offset += body_size;
+	tpm_buf_destroy(&tbuf);
+	return 0;
+}
+
+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);
+}
+
+static int tpm2_load_space(struct tpm_chip *chip)
+{
+	struct tpm_space *space = &chip->work_space;
+	unsigned int offset;
+	int i;
+	int rc;
+
+	for (i = 0, offset = 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 = tpm2_load_context(chip, space->context_buf, &offset,
+				       &space->context_tbl[i]);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
+static bool tpm2_map_to_phandle(struct tpm_space *space, void *handle)
+{
+	u32 vhandle = be32_to_cpup((__be32 *)handle);
+	u32 phandle;
+	int i;
+
+	i = 0xFFFFFF - (vhandle & 0xFFFFFF);
+	if (i > ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i])
+		return false;
+
+	phandle = space->context_tbl[i];
+	*((__be32 *)handle) = cpu_to_be32(phandle);
+	return true;
+}
+
+static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd)
+{
+	struct tpm_space *space = &chip->work_space;
+	unsigned int nr_handles;
+	u32 attrs;
+	u32 *handle;
+	int i;
+
+	i = tpm2_find_cc(chip, cc);
+	if (i < 0)
+		return -EINVAL;
+
+	attrs = chip->cc_attrs_tbl[i];
+	nr_handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
+
+	handle = (u32 *)&cmd[TPM_HEADER_SIZE];
+	for (i = 0; i < nr_handles; i++, handle++) {
+		if ((be32_to_cpu(*handle) & 0xFF000000) == TPM2_HT_TRANSIENT) {
+			if (!tpm2_map_to_phandle(space, handle))
+				return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
+		       u8 *cmd)
+{
+	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) {
+		tpm2_flush_space(chip);
+		return rc;
+	}
+
+	rc = tpm2_map_command(chip, cc, cmd);
+	if (rc) {
+		tpm2_flush_space(chip);
+		return rc;
+	}
+
+	return 0;
+}
+
+static u32 tpm2_map_to_vhandle(struct tpm_space *space, u32 phandle, bool alloc)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
+		if (alloc) {
+			if (!space->context_tbl[i]) {
+				space->context_tbl[i] = phandle;
+				break;
+			}
+		} else if (space->context_tbl[i] == phandle)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(space->context_tbl))
+		return 0;
+
+	return TPM2_HT_TRANSIENT | (0xFFFFFF - i);
+}
+
+static int tpm2_map_response_header(struct tpm_chip *chip, u32 cc, u8 *rsp,
+				    size_t len)
+{
+	struct tpm_space *space = &chip->work_space;
+	struct tpm_output_header *header = (void *)rsp;
+	u32 phandle;
+	u32 phandle_type;
+	u32 vhandle;
+	u32 attrs;
+	int i;
+
+	if (be32_to_cpu(header->return_code) != TPM2_RC_SUCCESS)
+		return 0;
+
+	i = tpm2_find_cc(chip, cc);
+	/* sanity check, should never happen */
+	if (i < 0)
+		return -EFAULT;
+
+	attrs = chip->cc_attrs_tbl[i];
+	if (!((attrs >> TPM2_CC_ATTR_RHANDLE) & 1))
+		return 0;
+
+	phandle = be32_to_cpup((__be32 *)&rsp[TPM_HEADER_SIZE]);
+	phandle_type = phandle & 0xFF000000;
+
+	switch (phandle_type) {
+	case TPM2_HT_TRANSIENT:
+		vhandle = tpm2_map_to_vhandle(space, phandle, true);
+		if (!vhandle)
+			goto out_no_slots;
+
+		*(__be32 *)&rsp[TPM_HEADER_SIZE] = cpu_to_be32(vhandle);
+		break;
+	case TPM2_HT_HMAC_SESSION:
+	case TPM2_HT_POLICY_SESSION:
+		break;
+	default:
+		dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
+			__func__, phandle);
+		break;
+	};
+
+	return 0;
+out_no_slots:
+	tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_UNLOCKED);
+	dev_warn(&chip->dev, "%s: out of slots for 0x%08X\n", __func__,
+		 phandle);
+	return -ENOMEM;
+}
+
+struct tpm2_cap_handles {
+	u8 more_data;
+	__be32 capability;
+	__be32 count;
+	__be32 handles[];
+} __packed;
+
+static int tpm2_map_response_body(struct tpm_chip *chip, u32 cc, u8 *rsp,
+				  size_t len)
+{
+	struct tpm_space *space = &chip->work_space;
+	struct tpm_output_header *header = (void *)rsp;
+	struct tpm2_cap_handles *data;
+	u32 phandle;
+	u32 phandle_type;
+	u32 vhandle;
+	int i;
+	int j;
+
+	if (cc != TPM2_CC_GET_CAPABILITY ||
+	    be32_to_cpu(header->return_code) != TPM2_RC_SUCCESS) {
+		return 0;
+	}
+
+	if (len < TPM_HEADER_SIZE + 9)
+		return -EFAULT;
+
+	data = (void *)&rsp[TPM_HEADER_SIZE];
+	if (be32_to_cpu(data->capability) != TPM2_CAP_HANDLES)
+		return 0;
+
+	if (len != TPM_HEADER_SIZE + 9 + 4 * be32_to_cpu(data->count))
+		return -EFAULT;
+
+	for (i = 0, j = 0; i < be32_to_cpu(data->count); i++) {
+		phandle = be32_to_cpup((__be32 *)&data->handles[i]);
+		phandle_type = phandle & 0xFF000000;
+
+		switch (phandle_type) {
+		case TPM2_HT_TRANSIENT:
+			vhandle = tpm2_map_to_vhandle(space, phandle, false);
+			if (!vhandle)
+				break;
+
+			data->handles[j] = cpu_to_be32(vhandle);
+			j++;
+			break;
+		case TPM2_HT_HMAC_SESSION:
+		case TPM2_HT_POLICY_SESSION:
+			data->handles[j] = cpu_to_be32(phandle);
+			j++;
+			break;
+		default:
+			dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
+				__func__, phandle);
+			break;
+		}
+
+	}
+
+	header->length = cpu_to_be32(TPM_HEADER_SIZE + 9 + 4 * j);
+	data->count = cpu_to_be32(j);
+	return 0;
+}
+
+static int tpm2_save_space(struct tpm_chip *chip)
+{
+	struct tpm_space *space = &chip->work_space;
+	unsigned int offset;
+	int i;
+	int rc;
+
+	for (i = 0, offset = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
+		if (!(space->context_tbl[i] && ~space->context_tbl[i]))
+			continue;
+
+		rc = tpm2_save_context(chip, space->context_tbl[i],
+				       space->context_buf, PAGE_SIZE,
+				       &offset);
+		if (rc == -ENOENT) {
+			space->context_tbl[i] = 0;
+			continue;
+		} else if (rc)
+			return rc;
+
+		space->context_tbl[i] = ~0;
+	}
+
+	return 0;
+}
+
+int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
+		      u32 cc, u8 *buf, size_t *bufsiz)
+{
+	struct tpm_output_header *header = (void *)buf;
+	int rc;
+
+	if (!space)
+		return 0;
+
+	rc = tpm2_map_response_header(chip, cc, buf, *bufsiz);
+	if (rc) {
+		tpm2_flush_space(chip);
+		return rc;
+	}
+
+	rc = tpm2_map_response_body(chip, cc, buf, *bufsiz);
+	if (rc) {
+		tpm2_flush_space(chip);
+		return rc;
+	}
+
+	rc = tpm2_save_space(chip);
+	if (rc) {
+		tpm2_flush_space(chip);
+		return rc;
+	}
+
+	*bufsiz = be32_to_cpu(header->length);
+
+	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


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

[PATCH v2 5/7] tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c

[Jarkko Sakkinen]

From: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>

Signed-off-by: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
---
 drivers/char/tpm/Makefile         |   2 +-
 drivers/char/tpm/tpm-dev-common.c | 148 ++++++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm-dev.c        | 143 ++++--------------------------------
 drivers/char/tpm/tpm-dev.h        |  27 +++++++
 4 files changed, 190 insertions(+), 130 deletions(-)
 create mode 100644 drivers/char/tpm/tpm-dev-common.c
 create mode 100644 drivers/char/tpm/tpm-dev.h

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 8f07fcf..10e5827 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 \
-	 tpm1_eventlog.o tpm2_eventlog.o tpm2-space.o
+	 tpm-dev-common.o tpm1_eventlog.o tpm2_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-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
new file mode 100644
index 0000000..610638a
--- /dev/null
+++ b/drivers/char/tpm/tpm-dev-common.c
@@ -0,0 +1,148 @@
+/*
+ * Copyright (C) 2004 IBM Corporation
+ * Authors:
+ * Leendert van Doorn <leendert-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
+ * Dave Safford <safford-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
+ * Reiner Sailer <sailer-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
+ * Kylene Hall <kjhall-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
+ *
+ * Copyright (C) 2013 Obsidian Research Corp
+ * Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
+ *
+ * Device file system interface to the TPM
+ *
+ * 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/slab.h>
+#include <linux/uaccess.h>
+#include "tpm.h"
+#include "tpm-dev.h"
+
+static void user_reader_timeout(unsigned long ptr)
+{
+	struct file_priv *priv = (struct file_priv *)ptr;
+
+	pr_warn("TPM user space timeout is deprecated (pid=%d)\n",
+		task_tgid_nr(current));
+
+	schedule_work(&priv->work);
+}
+
+static void timeout_work(struct work_struct *work)
+{
+	struct file_priv *priv = container_of(work, struct file_priv, work);
+
+	mutex_lock(&priv->buffer_mutex);
+	atomic_set(&priv->data_pending, 0);
+	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
+	mutex_unlock(&priv->buffer_mutex);
+}
+
+void tpm_common_open(struct file *file, struct tpm_chip *chip,
+		     struct file_priv *priv)
+{
+	priv->chip = chip;
+	atomic_set(&priv->data_pending, 0);
+	mutex_init(&priv->buffer_mutex);
+	setup_timer(&priv->user_read_timer, user_reader_timeout,
+			(unsigned long)priv);
+	INIT_WORK(&priv->work, timeout_work);
+
+	file->private_data = priv;
+}
+
+ssize_t tpm_common_read(struct file *file, char __user *buf,
+			size_t size, loff_t *off)
+{
+	struct file_priv *priv = file->private_data;
+	ssize_t ret_size;
+	ssize_t orig_ret_size;
+	int rc;
+
+	del_singleshot_timer_sync(&priv->user_read_timer);
+	flush_work(&priv->work);
+	ret_size = atomic_read(&priv->data_pending);
+	if (ret_size > 0) {	/* relay data */
+		orig_ret_size = ret_size;
+		if (size < ret_size)
+			ret_size = size;
+
+		mutex_lock(&priv->buffer_mutex);
+		rc = copy_to_user(buf, priv->data_buffer, ret_size);
+		memset(priv->data_buffer, 0, orig_ret_size);
+		if (rc)
+			ret_size = -EFAULT;
+
+		mutex_unlock(&priv->buffer_mutex);
+	}
+
+	atomic_set(&priv->data_pending, 0);
+
+	return ret_size;
+}
+
+ssize_t tpm_common_write(struct file *file, const char __user *buf,
+			 size_t size, loff_t *off, struct tpm_space *space)
+{
+	struct file_priv *priv = file->private_data;
+	size_t in_size = size;
+	ssize_t out_size;
+
+	/* Cannot perform a write until the read has cleared either via
+	 * tpm_read or a user_read_timer timeout. This also prevents split
+	 * buffered writes from blocking here.
+	 */
+	if (atomic_read(&priv->data_pending) != 0)
+		return -EBUSY;
+
+	if (in_size > TPM_BUFSIZE)
+		return -E2BIG;
+
+	mutex_lock(&priv->buffer_mutex);
+
+	if (copy_from_user
+	    (priv->data_buffer, (void __user *) buf, in_size)) {
+		mutex_unlock(&priv->buffer_mutex);
+		return -EFAULT;
+	}
+
+	/* atomic tpm command send and result receive. We only hold the ops
+	 * lock during this period so that the tpm can be unregistered even if
+	 * the char dev is held open.
+	 */
+	if (tpm_try_get_ops(priv->chip)) {
+		mutex_unlock(&priv->buffer_mutex);
+		return -EPIPE;
+	}
+	out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
+				sizeof(priv->data_buffer), 0);
+
+	tpm_put_ops(priv->chip);
+	if (out_size < 0) {
+		mutex_unlock(&priv->buffer_mutex);
+		return out_size;
+	}
+
+	atomic_set(&priv->data_pending, out_size);
+	mutex_unlock(&priv->buffer_mutex);
+
+	/* Set a timeout by which the reader must come claim the result */
+	mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
+
+	return in_size;
+}
+
+/*
+ * Called on file close
+ */
+void tpm_common_release(struct file *file, struct file_priv *priv)
+{
+	del_singleshot_timer_sync(&priv->user_read_timer);
+	flush_work(&priv->work);
+	file->private_data = NULL;
+	atomic_set(&priv->data_pending, 0);
+}
diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
index 414553b..ebd74ab 100644
--- a/drivers/char/tpm/tpm-dev.c
+++ b/drivers/char/tpm/tpm-dev.c
@@ -18,48 +18,15 @@
  *
  */
 #include <linux/slab.h>
-#include <linux/uaccess.h>
-#include "tpm.h"
-
-struct file_priv {
-	struct tpm_chip *chip;
-
-	/* Data passed to and from the tpm via the read/write calls */
-	atomic_t data_pending;
-	struct mutex buffer_mutex;
-
-	struct timer_list user_read_timer;      /* user needs to claim result */
-	struct work_struct work;
-
-	u8 data_buffer[TPM_BUFSIZE];
-};
-
-static void user_reader_timeout(unsigned long ptr)
-{
-	struct file_priv *priv = (struct file_priv *)ptr;
-
-	pr_warn("TPM user space timeout is deprecated (pid=%d)\n",
-		task_tgid_nr(current));
-
-	schedule_work(&priv->work);
-}
-
-static void timeout_work(struct work_struct *work)
-{
-	struct file_priv *priv = container_of(work, struct file_priv, work);
-
-	mutex_lock(&priv->buffer_mutex);
-	atomic_set(&priv->data_pending, 0);
-	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
-	mutex_unlock(&priv->buffer_mutex);
-}
+#include "tpm-dev.h"
 
 static int tpm_open(struct inode *inode, struct file *file)
 {
-	struct tpm_chip *chip =
-		container_of(inode->i_cdev, struct tpm_chip, cdev);
+	struct tpm_chip *chip;
 	struct file_priv *priv;
 
+	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. */
@@ -69,100 +36,22 @@ static int tpm_open(struct inode *inode, struct file *file)
 	}
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (priv == NULL) {
-		clear_bit(0, &chip->is_open);
-		return -ENOMEM;
-	}
+	if (priv == NULL)
+		goto out;
 
-	priv->chip = chip;
-	atomic_set(&priv->data_pending, 0);
-	mutex_init(&priv->buffer_mutex);
-	setup_timer(&priv->user_read_timer, user_reader_timeout,
-			(unsigned long)priv);
-	INIT_WORK(&priv->work, timeout_work);
+	tpm_common_open(file, chip, priv);
 
-	file->private_data = priv;
 	return 0;
-}
-
-static ssize_t tpm_read(struct file *file, char __user *buf,
-			size_t size, loff_t *off)
-{
-	struct file_priv *priv = file->private_data;
-	ssize_t ret_size;
-	int rc;
 
-	del_singleshot_timer_sync(&priv->user_read_timer);
-	flush_work(&priv->work);
-	ret_size = atomic_read(&priv->data_pending);
-	if (ret_size > 0) {	/* relay data */
-		ssize_t orig_ret_size = ret_size;
-		if (size < ret_size)
-			ret_size = size;
-
-		mutex_lock(&priv->buffer_mutex);
-		rc = copy_to_user(buf, priv->data_buffer, ret_size);
-		memset(priv->data_buffer, 0, orig_ret_size);
-		if (rc)
-			ret_size = -EFAULT;
-
-		mutex_unlock(&priv->buffer_mutex);
-	}
-
-	atomic_set(&priv->data_pending, 0);
-
-	return ret_size;
+ out:
+	clear_bit(0, &chip->is_open);
+	return -ENOMEM;
 }
 
 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;
-	size_t in_size = size;
-	ssize_t out_size;
-
-	/* cannot perform a write until the read has cleared
-	   either via tpm_read or a user_read_timer timeout.
-	   This also prevents splitted buffered writes from blocking here.
-	*/
-	if (atomic_read(&priv->data_pending) != 0)
-		return -EBUSY;
-
-	if (in_size > TPM_BUFSIZE)
-		return -E2BIG;
-
-	mutex_lock(&priv->buffer_mutex);
-
-	if (copy_from_user
-	    (priv->data_buffer, (void __user *) buf, in_size)) {
-		mutex_unlock(&priv->buffer_mutex);
-		return -EFAULT;
-	}
-
-	/* atomic tpm command send and result receive. We only hold the ops
-	 * lock during this period so that the tpm can be unregistered even if
-	 * the char dev is held open.
-	 */
-	if (tpm_try_get_ops(priv->chip)) {
-		mutex_unlock(&priv->buffer_mutex);
-		return -EPIPE;
-	}
-	out_size = tpm_transmit(priv->chip, NULL, priv->data_buffer,
-				sizeof(priv->data_buffer), 0);
-
-	tpm_put_ops(priv->chip);
-	if (out_size < 0) {
-		mutex_unlock(&priv->buffer_mutex);
-		return out_size;
-	}
-
-	atomic_set(&priv->data_pending, out_size);
-	mutex_unlock(&priv->buffer_mutex);
-
-	/* Set a timeout by which the reader must come claim the result */
-	mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
-
-	return in_size;
+	return tpm_common_write(file, buf, size, off, NULL);
 }
 
 /*
@@ -172,12 +61,10 @@ static int tpm_release(struct inode *inode, struct file *file)
 {
 	struct file_priv *priv = file->private_data;
 
-	del_singleshot_timer_sync(&priv->user_read_timer);
-	flush_work(&priv->work);
-	file->private_data = NULL;
-	atomic_set(&priv->data_pending, 0);
+	tpm_common_release(file, priv);
 	clear_bit(0, &priv->chip->is_open);
 	kfree(priv);
+
 	return 0;
 }
 
@@ -185,9 +72,7 @@ const struct file_operations tpm_fops = {
 	.owner = THIS_MODULE,
 	.llseek = no_llseek,
 	.open = tpm_open,
-	.read = tpm_read,
+	.read = tpm_common_read,
 	.write = tpm_write,
 	.release = tpm_release,
 };
-
-
diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
new file mode 100644
index 0000000..ff15cf7
--- /dev/null
+++ b/drivers/char/tpm/tpm-dev.h
@@ -0,0 +1,27 @@
+#ifndef _TPM_DEV_H
+#define _TPM_DEV_H
+
+#include "tpm.h"
+
+struct file_priv {
+	struct tpm_chip *chip;
+
+	/* Data passed to and from the tpm via the read/write calls */
+	atomic_t data_pending;
+	struct mutex buffer_mutex;
+
+	struct timer_list user_read_timer;      /* user needs to claim result */
+	struct work_struct work;
+
+	u8 data_buffer[TPM_BUFSIZE];
+};
+
+void tpm_common_open(struct file *file, struct tpm_chip *chip,
+		     struct file_priv *priv);
+ssize_t tpm_common_read(struct file *file, char __user *buf,
+			size_t size, loff_t *off);
+ssize_t tpm_common_write(struct file *file, const char __user *buf,
+			 size_t size, loff_t *off, struct tpm_space *space);
+void tpm_common_release(struct file *file, struct file_priv *priv);
+
+#endif
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

[PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jarkko Sakkinen]

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

Currently the tpm spaces are 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 space.

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        |  3 +-
 drivers/char/tpm/tpm-chip.c      | 73 ++++++++++++++++++++++++++++++++++++++--
 drivers/char/tpm/tpm-interface.c | 13 +++++--
 drivers/char/tpm/tpm.h           |  4 +++
 drivers/char/tpm/tpms-dev.c      | 65 +++++++++++++++++++++++++++++++++++
 5 files changed, 152 insertions(+), 6 deletions(-)
 create mode 100644 drivers/char/tpm/tpms-dev.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 10e5827..bbe6531 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,8 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-	 tpm-dev-common.o tpm1_eventlog.o tpm2_eventlog.o tpm2-space.o
+	 tpm-dev-common.o tpms-dev.o tpm1_eventlog.o tpm2_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 993b9ae..c71c353 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 *tpms_class;
 dev_t tpm_devt;
 
 /**
@@ -132,6 +133,14 @@ static void tpm_dev_release(struct device *dev)
 	kfree(chip);
 }
 
+static void tpm_devs_release(struct device *dev)
+{
+	struct tpm_chip *chip = container_of(dev, struct tpm_chip, devs);
+
+	/* release the master device reference */
+	put_device(&chip->dev);
+}
+
 /**
  * tpm_chip_alloc() - allocate a new struct tpm_chip instance
  * @pdev: device to which the chip is associated
@@ -168,27 +177,47 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	chip->dev_num = rc;
 
 	device_initialize(&chip->dev);
+	device_initialize(&chip->devs);
 
 	chip->dev.class = tpm_class;
 	chip->dev.release = tpm_dev_release;
 	chip->dev.parent = pdev;
 	chip->dev.groups = chip->groups;
 
+	chip->devs.parent = pdev;
+	chip->devs.class = tpms_class;
+	chip->devs.release = tpm_devs_release;
+	/* get extra reference on main device to hold on
+	 * behalf of devs.  This holds the chip structure
+	 * while cdevs is in use.  The corresponding put
+	 * is in the tpm_devs_release
+	 */
+	get_device(&chip->dev);
+
 	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->devs.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->devs, "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->cdevs, &tpms_fops);
 	chip->cdev.owner = THIS_MODULE;
+	chip->cdevs.owner = THIS_MODULE;
 	chip->cdev.kobj.parent = &chip->dev.kobj;
+	chip->cdevs.kobj.parent = &chip->devs.kobj;
 
 	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!chip->work_space.context_buf) {
@@ -199,6 +228,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 	return chip;
 
 out:
+	put_device(&chip->devs);
 	put_device(&chip->dev);
 	return ERR_PTR(rc);
 }
@@ -244,7 +274,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 +284,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->cdevs, chip->devs.devt, 1);
+	if (rc) {
+		dev_err(&chip->dev,
+			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devs), MAJOR(chip->devs.devt),
+			MINOR(chip->devs.devt), rc);
+
+		goto err_3;
 	}
 
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		rc = device_add(&chip->devs);
+	if (rc) {
+		dev_err(&chip->dev,
+			"unable to device_register() %s, major %d, minor %d, err=%d\n",
+			dev_name(&chip->devs), MAJOR(chip->devs.devt),
+			MINOR(chip->devs.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->cdevs);
+ 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 +329,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->cdevs);
+		device_del(&chip->devs);
+	}
+
 	/* Make the chip unavailable. */
 	mutex_lock(&idr_lock);
 	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
@@ -282,6 +345,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
 		tpm2_shutdown(chip, TPM2_SU_CLEAR);
 	chip->ops = NULL;
 	up_write(&chip->ops_sem);
+	/* will release the devs reference to the chip->dev unless
+	 * something has cdevs open
+	 */
+	put_device(&chip->devs);
 }
 
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index db5ffe9..deb2021 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1257,9 +1257,17 @@ static int __init tpm_init(void)
 		return PTR_ERR(tpm_class);
 	}
 
-	rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES, "tpm");
+	tpms_class = class_create(THIS_MODULE, "tpms");
+	if (IS_ERR(tpms_class)) {
+		pr_err("couldn't create tpms class\n");
+		class_destroy(tpm_class);
+		return PTR_ERR(tpms_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(tpms_class);
 		class_destroy(tpm_class);
 		return rc;
 	}
@@ -1271,7 +1279,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(tpms_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 97e48a4..822ca67 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -182,7 +182,9 @@ struct tpm_chip_seqops {
 
 struct tpm_chip {
 	struct device dev;
+	struct device devs;
 	struct cdev cdev;
+	struct cdev cdevs;
 
 	/* 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 +512,10 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 }
 
 extern struct class *tpm_class;
+extern struct class *tpms_class;
 extern dev_t tpm_devt;
 extern const struct file_operations tpm_fops;
+extern const struct file_operations tpms_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..5720885
--- /dev/null
+++ b/drivers/char/tpm/tpms-dev.c
@@ -0,0 +1,65 @@
+/*
+ * 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;
+	int rc;
+
+	chip = container_of(inode->i_cdev, struct tpm_chip, cdevs);
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (priv == NULL)
+		return -ENOMEM;
+
+	rc = tpm2_init_space(&priv->space);
+	if (rc) {
+		kfree(priv);
+		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);
+	tpm2_del_space(&priv->space);
+	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 tpms_fops = {
+	.owner = THIS_MODULE,
+	.llseek = no_llseek,
+	.open = tpms_open,
+	.read = tpm_common_read,
+	.write = tpms_write,
+	.release = tpms_release,
+};
+
-- 
2.9.3

[PATCH v2 7/7] tpm2: add session handle context saving and restoring to the space code

[Jarkko Sakkinen]

From: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>

Sessions are different from transient objects in that their handles
may not be virtualized (because they're used for some hmac
calculations).  Additionally when a session is context saved, a
vestigial memory remains in the TPM and if it is also flushed, that
will be lost and the session context will refuse to load next time, so
the code is updated to flush only transient objects after a context
save.  Add a separate array (chip->session_tbl) to save and restore
sessions by handle.  Use the failure of a context save or load to
signal that the session has been flushed from the TPM and we can
remove its memory from chip->session_tbl.

Sessions are also isolated during each instance of a tpm space.  This
means that spaces shouldn't be able to see each other's sessions and
is enforced by ensuring that a space user may only refer to sessions
handles that are present in their own chip->session_tbl.  Finally when
a space is closed, all the sessions belonging to it should be flushed
so the handles may be re-used by other spaces.

Note that if we get a session save or load error, all sessions are
effectively flushed.  Even though we restore the session buffer, all
the old sessions will refuse to load after the flush and they'll be
purged from our session memory.  This means that while transient
context handling is still soft in the face of errors, session handling
is hard (any failure of the model means all sessions are lost).

Signed-off-by: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
---
 drivers/char/tpm/tpm-chip.c   |   6 +++
 drivers/char/tpm/tpm.h        |   4 +-
 drivers/char/tpm/tpm2-space.c | 105 +++++++++++++++++++++++++++++++++++++++++-
 drivers/char/tpm/tpms-dev.c   |   2 +-
 4 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index c71c353..e8536ab 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -130,6 +130,7 @@ static void tpm_dev_release(struct device *dev)
 
 	kfree(chip->log.bios_event_log);
 	kfree(chip->work_space.context_buf);
+	kfree(chip->work_space.session_buf);
 	kfree(chip);
 }
 
@@ -224,6 +225,11 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
 		rc = -ENOMEM;
 		goto out;
 	}
+	chip->work_space.session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!chip->work_space.session_buf) {
+		rc = -ENOMEM;
+		goto out;
+	}
 
 	return chip;
 
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 822ca67..ffee8b5 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -161,6 +161,8 @@ enum tpm2_cc_attrs {
 struct tpm_space {
 	u32 context_tbl[3];
 	u8 *context_buf;
+	u32 session_tbl[3];
+	u8 *session_buf;
 };
 
 enum tpm_chip_flags {
@@ -589,7 +591,7 @@ int tpm2_probe(struct tpm_chip *chip);
 ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
 int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
 int tpm2_init_space(struct tpm_space *space);
-void tpm2_del_space(struct tpm_space *space);
+void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
 int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 		       u8 *cmd);
 int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
index e955548..d36d81e 100644
--- a/drivers/char/tpm/tpm2-space.c
+++ b/drivers/char/tpm/tpm2-space.c
@@ -32,18 +32,39 @@ struct tpm2_context {
 	__be16 blob_size;
 } __packed;
 
+static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
+		if (space->session_tbl[i])
+			tpm2_flush_context_cmd(chip, space->session_tbl[i],
+					       TPM_TRANSMIT_UNLOCKED);
+	}
+}
+
 int tpm2_init_space(struct tpm_space *space)
 {
 	space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
 	if (!space->context_buf)
 		return -ENOMEM;
 
+	space->session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
+	if (space->session_buf == NULL) {
+		kfree(space->context_buf);
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
-void tpm2_del_space(struct tpm_space *space)
+void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
 {
+	mutex_lock(&chip->tpm_mutex);
+	tpm2_flush_sessions(chip, space);
+	mutex_unlock(&chip->tpm_mutex);
 	kfree(space->context_buf);
+	kfree(space->session_buf);
 }
 
 static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
@@ -69,6 +90,20 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
 			 __func__, rc);
 		tpm_buf_destroy(&tbuf);
 		return -EFAULT;
+	} else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE ||
+		   rc == TPM2_RC_REFERENCE_H0) {
+		/*
+		 * TPM_RC_HANDLE means that the session context can't
+		 * be loaded because of an internal counter mismatch
+		 * that makes the TPM think there might have been a
+		 * replay.  This might happen if the context was saved
+		 * and loaded outside the space.
+		 *
+		 * TPM_RC_REFERENCE_H0 means the session has been
+		 * flushed outside the space
+		 */
+		rc = -ENOENT;
+		tpm_buf_destroy(&tbuf);
 	} else if (rc > 0) {
 		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
 			 __func__, rc);
@@ -121,7 +156,6 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
 	}
 
 	memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size);
-	tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED);
 	*offset += body_size;
 	tpm_buf_destroy(&tbuf);
 	return 0;
@@ -136,6 +170,8 @@ static void tpm2_flush_space(struct tpm_chip *chip)
 		if (space->context_tbl[i] && ~space->context_tbl[i])
 			tpm2_flush_context_cmd(chip, space->context_tbl[i],
 					       TPM_TRANSMIT_UNLOCKED);
+
+	tpm2_flush_sessions(chip, space);
 }
 
 static int tpm2_load_space(struct tpm_chip *chip)
@@ -161,6 +197,28 @@ static int tpm2_load_space(struct tpm_chip *chip)
 			return rc;
 	}
 
+	for (i = 0, offset = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
+		u32 handle;
+
+		if (!space->session_tbl[i])
+			continue;
+
+		rc = tpm2_load_context(chip, space->session_buf,
+				       &offset, &handle);
+		if (rc == -ENOENT) {
+			/* load failed, just forget session */
+			space->session_tbl[i] = 0;
+		} else if (rc) {
+			tpm2_flush_space(chip);
+			return rc;
+		}
+		if (handle != space->session_tbl[i]) {
+			dev_warn(&chip->dev, "session restored to wrong handle\n");
+			tpm2_flush_space(chip);
+			return -EFAULT;
+		}
+	}
+
 	return 0;
 }
 
@@ -215,7 +273,10 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 
 	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
 	       sizeof(space->context_tbl));
+	memcpy(&chip->work_space.session_tbl, &space->session_tbl,
+	       sizeof(space->session_tbl));
 	memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
+	memcpy(chip->work_space.session_buf, space->session_buf, PAGE_SIZE);
 
 	rc = tpm2_load_space(chip);
 	if (rc) {
@@ -232,6 +293,22 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
 	return 0;
 }
 
+static bool tpm2_add_session(struct tpm_chip *chip, u32 handle)
+{
+	struct tpm_space *space = &chip->work_space;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++)
+		if (space->session_tbl[i] == 0)
+			break;
+
+	if (i == ARRAY_SIZE(space->session_tbl))
+		return false;
+
+	space->session_tbl[i] = handle;
+	return true;
+}
+
 static u32 tpm2_map_to_vhandle(struct tpm_space *space, u32 phandle, bool alloc)
 {
 	int i;
@@ -288,6 +365,8 @@ static int tpm2_map_response_header(struct tpm_chip *chip, u32 cc, u8 *rsp,
 		break;
 	case TPM2_HT_HMAC_SESSION:
 	case TPM2_HT_POLICY_SESSION:
+		if (!tpm2_add_session(chip, phandle))
+			goto out_no_slots;
 		break;
 	default:
 		dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
@@ -388,9 +467,28 @@ static int tpm2_save_space(struct tpm_chip *chip)
 		} else if (rc)
 			return rc;
 
+		tpm2_flush_context_cmd(chip, space->context_tbl[i],
+				       TPM_TRANSMIT_UNLOCKED);
 		space->context_tbl[i] = ~0;
 	}
 
+	for (i = 0, offset = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
+		if (!space->session_tbl[i])
+			continue;
+
+		rc = tpm2_save_context(chip, space->session_tbl[i],
+				       space->session_buf, PAGE_SIZE,
+				       &offset);
+
+		if (rc == -ENOENT) {
+			/* handle error saving session, just forget it */
+			space->session_tbl[i] = 0;
+		} else if (rc < 0) {
+			tpm2_flush_space(chip);
+			return rc;
+		}
+	}
+
 	return 0;
 }
 
@@ -425,7 +523,10 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
 
 	memcpy(&space->context_tbl, &chip->work_space.context_tbl,
 	       sizeof(space->context_tbl));
+	memcpy(&space->session_tbl, &chip->work_space.session_tbl,
+	       sizeof(space->session_tbl));
 	memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
+	memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE);
 
 	return 0;
 }
diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms-dev.c
index 5720885..4c98db8 100644
--- a/drivers/char/tpm/tpms-dev.c
+++ b/drivers/char/tpm/tpms-dev.c
@@ -39,7 +39,7 @@ static int tpms_release(struct inode *inode, struct file *file)
 	struct tpms_priv *priv = container_of(fpriv, struct tpms_priv, priv);
 
 	tpm_common_release(file, fpriv);
-	tpm2_del_space(&priv->space);
+	tpm2_del_space(fpriv->chip, &priv->space);
 	kfree(priv);
 
 	return 0;
-- 
2.9.3


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Nayna]

On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
> Added an ability to virtualize TPM commands into an isolated context
> that we call a TPM space because the word context is already heavily
> used in the TPM specification. Both the handle areas and bodies (where
> necessary) are virtualized.
>
> The mechanism works by adding a new parameter struct tpm_space to the
> tpm_transmit() function. This new structure contains the list of virtual
> handles and a buffer of page size (currently) for backing storage.
>
> When tpm_transmit() is called with a struct tpm_space instance it will
> execute the following sequence:
>
> 1. Take locks.
> 2. Load transient objects from the backing storage by using ContextLoad
>     and map virtual handles to physical handles.
> 3. Perform the transaction.
> 4. Save transient objects to backing storage by using ContextSave and
>     map resulting physical handle to virtual handle if there is such.
>
> This commit does not implement virtualization support for hmac and
> policy sessions.
>

If I have understood discussions correctly, I assume, that kernel TPM 
operations will also be routed via RM. And I think that is not happening 
now with these patches.

Am I missing something ?

Thanks & Regards,
    - Nayna


> 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 |  68 +++---
>   drivers/char/tpm/tpm-sysfs.c     |   2 +-
>   drivers/char/tpm/tpm.h           |  26 ++-
>   drivers/char/tpm/tpm2-cmd.c      |  33 +--
>   drivers/char/tpm/tpm2-space.c    | 431 +++++++++++++++++++++++++++++++++++++++
>   8 files changed, 520 insertions(+), 51 deletions(-)
>   create mode 100644 drivers/char/tpm/tpm2-space.c
>
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 3d386a8..8f07fcf 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 \
> -		tpm1_eventlog.o tpm2_eventlog.o
> +	 tpm1_eventlog.o tpm2_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 02a8850..414553b 100644
> --- a/drivers/char/tpm/tpm-dev.c
> +++ b/drivers/char/tpm/tpm-dev.c
> @@ -147,7 +147,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 20b1fe3..db5ffe9 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -376,11 +376,12 @@ 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)
>   {
> -	const struct tpm_output_header *header = (void *)buf;
> -	ssize_t rc;
> +	struct tpm_output_header *header = (void *)buf;
> +	int rc;
> +	ssize_t len = 0;
>   	u32 count, ordinal;
>   	unsigned long stop;
>
> @@ -406,10 +407,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);
> +	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;
>   	}
>
> @@ -442,18 +447,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;
> -	} else if (rc < TPM_HEADER_SIZE) {
> +	} else if (len < TPM_HEADER_SIZE) {
>   		rc = -EFAULT;
>   		goto out;
>   	}
>
> -	if (rc != be32_to_cpu(header->length))
> +	if (len != be32_to_cpu(header->length)) {
> +		rc = -EFAULT;
>   		goto out;
> +	}
> +
> +	rc = tpm2_commit_space(chip, space, ordinal, buf, &len);
>
>   out:
>   	if (chip->dev.parent)
> @@ -461,7 +471,7 @@ ssize_t tpm_transmit(struct tpm_chip *chip, const u8 *buf, size_t bufsiz,
>
>   	if (!(flags & TPM_TRANSMIT_UNLOCKED))
>   		mutex_unlock(&chip->tpm_mutex);
> -	return rc;
> +	return rc ? rc : len;
>   }
>
>   /**
> @@ -480,15 +490,16 @@ 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 *buf,
> -			 size_t bufsiz, size_t min_rsp_body_length,
> -			 unsigned int flags, const char *desc)
> +ssize_t tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> +			 const void *buf, size_t bufsiz,
> +			 size_t min_rsp_body_length, unsigned int flags,
> +			 const char *desc)
>   {
>   	const struct tpm_output_header *header = buf;
>   	int err;
>   	ssize_t len;
>
> -	len = tpm_transmit(chip, (const u8 *)buf, bufsiz, flags);
> +	len = tpm_transmit(chip, space, (u8 *)buf, bufsiz, flags);
>   	if (len <  0)
>   		return len;
>
> @@ -541,7 +552,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,
> +	rc = tpm_transmit_cmd(chip, NULL, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
>   			      min_cap_length, 0, desc);
>   	if (!rc)
>   		*cap = tpm_cmd.params.getcap_out.cap;
> @@ -565,7 +576,8 @@ 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,
> +	return tpm_transmit_cmd(chip, NULL, &start_cmd,
> +				TPM_INTERNAL_RESULT_SIZE, 0,
>   				0, "attempting to start the TPM");
>   }
>
> @@ -722,8 +734,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, 0,
> -			      "continue selftest");
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, CONTINUE_SELFTEST_RESULT_SIZE,
> +			      0, 0, "continue selftest");
>   	return rc;
>   }
>
> @@ -743,7 +755,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,
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, READ_PCR_RESULT_SIZE,
>   			      READ_PCR_RESULT_BODY_SIZE, 0,
>   			      "attempting to read a pcr value");
>
> @@ -855,7 +867,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,
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE,
>   			      EXTEND_PCR_RESULT_BODY_SIZE, 0,
>   			      "attempting extend a PCR value");
>
> @@ -960,8 +972,8 @@ int tpm_send(u32 chip_num, void *cmd, size_t buflen)
>   	if (chip == NULL)
>   		return -ENODEV;
>
> -	rc = tpm_transmit_cmd(chip, cmd, buflen, 0, 0, "attempting tpm_cmd");
> -
> +	rc = tpm_transmit_cmd(chip, NULL, cmd, buflen, 0, 0,
> +			      "attempting tpm_cmd");
>   	tpm_put_ops(chip);
>   	return rc;
>   }
> @@ -1062,16 +1074,16 @@ 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,
> -				     EXTEND_PCR_RESULT_BODY_SIZE, 0,
> +		rc = tpm_transmit_cmd(chip, NULL, &cmd, EXTEND_PCR_RESULT_SIZE,
> +				      EXTEND_PCR_RESULT_BODY_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,
> -				      0, NULL);
> +		rc = tpm_transmit_cmd(chip, NULL, &cmd, SAVESTATE_RESULT_SIZE,
> +				      0, 0, NULL);
>
>   		/*
>   		 * If the TPM indicates that it is too busy to respond to
> @@ -1154,7 +1166,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,
>   				       offsetof(struct tpm_getrandom_out,
>   						rng_data),
> diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c
> index 2f596d7..55405db 100644
> --- a/drivers/char/tpm/tpm-sysfs.c
> +++ b/drivers/char/tpm/tpm-sysfs.c
> @@ -40,7 +40,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,
> +	err = tpm_transmit_cmd(chip, NULL, &tpm_cmd, READ_PUBEK_RESULT_SIZE,
>   			       READ_PUBEK_RESULT_MIN_BODY_SIZE, 0,
>   			       "attempting to read the PUBEK");
>   	if (err)
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 0ec1cf0..97e48a4 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -89,10 +89,13 @@ 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 */
>   	TPM2_RC_DISABLED	= 0x0120,
>   	TPM2_RC_TESTING		= 0x090A, /* RC_WARN */
> +	TPM2_RC_REFERENCE_H0	= 0x0910,
>   };
>
>   enum tpm2_algorithms {
> @@ -114,6 +117,7 @@ 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,
> @@ -128,6 +132,7 @@ enum tpm2_permanent_handles {
>   };
>
>   enum tpm2_capabilities {
> +	TPM2_CAP_HANDLES	= 1,
>   	TPM2_CAP_COMMANDS	= 2,
>   	TPM2_CAP_PCRS		= 5,
>   	TPM2_CAP_TPM_PROPERTIES = 6,
> @@ -153,6 +158,11 @@ enum tpm2_cc_attrs {
>
>   #define TPM_PPI_VERSION_LEN		3
>
> +struct tpm_space {
> +	u32 context_tbl[3];
> +	u8 *context_buf;
> +};
> +
>   enum tpm_chip_flags {
>   	TPM_CHIP_FLAG_TPM2		= BIT(1),
>   	TPM_CHIP_FLAG_IRQ		= BIT(2),
> @@ -211,6 +221,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;
>   };
> @@ -507,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 *buf, size_t bufsiz,
> -			 size_t min_rsp_body_len, 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 tpm_transmit_cmd(struct tpm_chip *chip, struct tpm_space *space,
> +			 const void *buf, size_t bufsiz,
> +			 size_t min_rsp_body_length, unsigned int flags,
>   			 const char *desc);
>   ssize_t tpm_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>   		   const char *desc, size_t min_cap_length);
> @@ -572,4 +584,10 @@ unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
>   int tpm2_probe(struct tpm_chip *chip);
>   ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>   int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
> +int tpm2_init_space(struct tpm_space *space);
> +void tpm2_del_space(struct tpm_space *space);
> +int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
> +		       u8 *cmd);
> +int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> +		      u32 cc, u8 *buf, size_t *bufsiz);
>   #endif
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 897902a..96121b3 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -266,7 +266,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),
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
>   			      TPM2_PCR_READ_RESP_BODY_SIZE,
>   			      0, "attempting to read a pcr value");
>   	if (rc == 0) {
> @@ -333,7 +333,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, int pcr_idx, u32 count,
>   		}
>   	}
>
> -	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, 0,
>   			      "attempting extend a PCR value");
>
>   	tpm_buf_destroy(&buf);
> @@ -382,7 +382,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),
> +		err = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
>   				       offsetof(struct tpm2_get_random_out,
>   						buffer),
>   				       0, "attempting get random");
> @@ -441,7 +441,7 @@ void tpm2_flush_context_cmd(struct tpm_chip *chip, u32 handle,
>
>   	tpm_buf_append_u32(&buf, handle);
>
> -	(void) tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 0, flags,
> +	(void) tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 0, flags,
>   				"flushing context");
>
>   	tpm_buf_destroy(&buf);
> @@ -557,7 +557,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip,
>   		goto out;
>   	}
>
> -	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 4, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, 0,
>   			      "sealing data");
>   	if (rc)
>   		goto out;
> @@ -641,7 +641,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
>   		goto out;
>   	}
>
> -	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 4, flags,
> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 4, flags,
>   			      "loading blob");
>   	if (!rc)
>   		*blob_handle = be32_to_cpup(
> @@ -693,7 +693,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip,
>   			     options->blobauth /* hmac */,
>   			     TPM_DIGEST_SIZE);
>
> -	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 6, flags,
> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 6, flags,
>   			      "unsealing");
>   	if (rc > 0)
>   		rc = -EPERM;
> @@ -770,7 +770,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),
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd),
>   			      TPM2_GET_TPM_PT_OUT_BODY_SIZE, 0, desc);
>   	if (!rc)
>   		*value = be32_to_cpu(cmd.params.get_tpm_pt_out.value);
> @@ -805,7 +805,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, 0,
> +	return tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
>   				"attempting to start the TPM");
>   }
>
> @@ -834,7 +834,7 @@ 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, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
>   			      "stopping the TPM");
>
>   	/* In places where shutdown command is sent there's no much we can do
> @@ -898,7 +898,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, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, TPM2_SELF_TEST_IN_SIZE, 0, 0,
>   			      "continue selftest");
>
>   	/* At least some prototype chips seem to give RC_TESTING error
> @@ -949,7 +949,8 @@ 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, 0, NULL);
> +		rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0,
> +				      NULL);
>   		if (rc < 0)
>   			break;
>
> @@ -982,7 +983,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, 0, NULL);
> +	rc = tpm_transmit_cmd(chip, NULL, &cmd, sizeof(cmd), 0, 0, NULL);
>   	if (rc <  0)
>   		return rc;
>
> @@ -1022,8 +1023,8 @@ static int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip)
>   	tpm_buf_append_u32(&buf, TPM2_CC_FIRST);
>   	tpm_buf_append_u32(&buf, nr_commands);
>
> -	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 9 + 4 * nr_commands,
> -			      0, NULL);
> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE,
> +			      9 + 4 * nr_commands, 0, NULL);
>   	if (rc) {
>   		tpm_buf_destroy(&buf);
>   		goto out;
> @@ -1136,7 +1137,7 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
>   	tpm_buf_append_u32(&buf, 0);
>   	tpm_buf_append_u32(&buf, 1);
>
> -	rc = tpm_transmit_cmd(chip, buf.data, PAGE_SIZE, 9, 0,
> +	rc = tpm_transmit_cmd(chip, NULL, buf.data, PAGE_SIZE, 9, 0,
>   			      "get tpm pcr allocation");
>   	if (rc)
>   		goto out;
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> new file mode 100644
> index 0000000..e955548
> --- /dev/null
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -0,0 +1,431 @@
> +/*
> + * 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 <asm/unaligned.h>
> +#include "tpm.h"
> +
> +enum tpm2_handle_types {
> +	TPM2_HT_HMAC_SESSION	= 0x02000000,
> +	TPM2_HT_POLICY_SESSION	= 0x03000000,
> +	TPM2_HT_TRANSIENT	= 0x80000000,
> +};
> +
> +struct tpm2_context {
> +	__be64 sequence;
> +	__be32 saved_handle;
> +	__be32 hierarchy;
> +	__be16 blob_size;
> +} __packed;
> +
> +int tpm2_init_space(struct tpm_space *space)
> +{
> +	space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (!space->context_buf)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +void tpm2_del_space(struct tpm_space *space)
> +{
> +	kfree(space->context_buf);
> +}
> +
> +static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
> +			     unsigned int *offset, u32 *handle)
> +{
> +	struct tpm_buf tbuf;
> +	struct tpm2_context *ctx;
> +	unsigned int body_size;
> +	int rc;
> +
> +	rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD);
> +	if (rc)
> +		return rc;
> +
> +	ctx = (struct tpm2_context *)&buf[*offset];
> +	body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size);
> +	tpm_buf_append(&tbuf, &buf[*offset], body_size);
> +
> +	rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 4,
> +			      TPM_TRANSMIT_UNLOCKED, NULL);
> +	if (rc < 0) {
> +		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
> +			 __func__, rc);
> +		tpm_buf_destroy(&tbuf);
> +		return -EFAULT;
> +	} else if (rc > 0) {
> +		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
> +			 __func__, rc);
> +		tpm_buf_destroy(&tbuf);
> +		return -EFAULT;
> +	}
> +
> +	*handle = be32_to_cpup((__be32 *)&tbuf.data[TPM_HEADER_SIZE]);
> +	*offset += body_size;
> +
> +	tpm_buf_destroy(&tbuf);
> +	return 0;
> +}
> +
> +static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
> +			     unsigned int buf_size, unsigned int *offset)
> +{
> +	struct tpm_buf tbuf;
> +	unsigned int body_size;
> +	int rc;
> +
> +	rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE);
> +	if (rc)
> +		return rc;
> +
> +	tpm_buf_append_u32(&tbuf, handle);
> +
> +	rc = tpm_transmit_cmd(chip, NULL, tbuf.data, PAGE_SIZE, 0,
> +			      TPM_TRANSMIT_UNLOCKED, NULL);
> +	if (rc < 0) {
> +		dev_warn(&chip->dev, "%s: failed with a system error %d\n",
> +			 __func__, rc);
> +		tpm_buf_destroy(&tbuf);
> +		return -EFAULT;
> +	} else if (tpm2_rc_value(rc) == TPM2_RC_REFERENCE_H0) {
> +		tpm_buf_destroy(&tbuf);
> +		return -ENOENT;
> +	} else if (rc) {
> +		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
> +			 __func__, rc);
> +		tpm_buf_destroy(&tbuf);
> +		return -EFAULT;
> +	}
> +
> +	body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE;
> +	if ((*offset + body_size) > buf_size) {
> +		dev_warn(&chip->dev, "%s: out of backing storage\n", __func__);
> +		tpm_buf_destroy(&tbuf);
> +		return -ENOMEM;
> +	}
> +
> +	memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size);
> +	tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED);
> +	*offset += body_size;
> +	tpm_buf_destroy(&tbuf);
> +	return 0;
> +}
> +
> +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);
> +}
> +
> +static int tpm2_load_space(struct tpm_chip *chip)
> +{
> +	struct tpm_space *space = &chip->work_space;
> +	unsigned int offset;
> +	int i;
> +	int rc;
> +
> +	for (i = 0, offset = 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 = tpm2_load_context(chip, space->context_buf, &offset,
> +				       &space->context_tbl[i]);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static bool tpm2_map_to_phandle(struct tpm_space *space, void *handle)
> +{
> +	u32 vhandle = be32_to_cpup((__be32 *)handle);
> +	u32 phandle;
> +	int i;
> +
> +	i = 0xFFFFFF - (vhandle & 0xFFFFFF);
> +	if (i > ARRAY_SIZE(space->context_tbl) || !space->context_tbl[i])
> +		return false;
> +
> +	phandle = space->context_tbl[i];
> +	*((__be32 *)handle) = cpu_to_be32(phandle);
> +	return true;
> +}
> +
> +static int tpm2_map_command(struct tpm_chip *chip, u32 cc, u8 *cmd)
> +{
> +	struct tpm_space *space = &chip->work_space;
> +	unsigned int nr_handles;
> +	u32 attrs;
> +	u32 *handle;
> +	int i;
> +
> +	i = tpm2_find_cc(chip, cc);
> +	if (i < 0)
> +		return -EINVAL;
> +
> +	attrs = chip->cc_attrs_tbl[i];
> +	nr_handles = (attrs >> TPM2_CC_ATTR_CHANDLES) & GENMASK(2, 0);
> +
> +	handle = (u32 *)&cmd[TPM_HEADER_SIZE];
> +	for (i = 0; i < nr_handles; i++, handle++) {
> +		if ((be32_to_cpu(*handle) & 0xFF000000) == TPM2_HT_TRANSIENT) {
> +			if (!tpm2_map_to_phandle(space, handle))
> +				return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
> +		       u8 *cmd)
> +{
> +	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) {
> +		tpm2_flush_space(chip);
> +		return rc;
> +	}
> +
> +	rc = tpm2_map_command(chip, cc, cmd);
> +	if (rc) {
> +		tpm2_flush_space(chip);
> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +static u32 tpm2_map_to_vhandle(struct tpm_space *space, u32 phandle, bool alloc)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
> +		if (alloc) {
> +			if (!space->context_tbl[i]) {
> +				space->context_tbl[i] = phandle;
> +				break;
> +			}
> +		} else if (space->context_tbl[i] == phandle)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(space->context_tbl))
> +		return 0;
> +
> +	return TPM2_HT_TRANSIENT | (0xFFFFFF - i);
> +}
> +
> +static int tpm2_map_response_header(struct tpm_chip *chip, u32 cc, u8 *rsp,
> +				    size_t len)
> +{
> +	struct tpm_space *space = &chip->work_space;
> +	struct tpm_output_header *header = (void *)rsp;
> +	u32 phandle;
> +	u32 phandle_type;
> +	u32 vhandle;
> +	u32 attrs;
> +	int i;
> +
> +	if (be32_to_cpu(header->return_code) != TPM2_RC_SUCCESS)
> +		return 0;
> +
> +	i = tpm2_find_cc(chip, cc);
> +	/* sanity check, should never happen */
> +	if (i < 0)
> +		return -EFAULT;
> +
> +	attrs = chip->cc_attrs_tbl[i];
> +	if (!((attrs >> TPM2_CC_ATTR_RHANDLE) & 1))
> +		return 0;
> +
> +	phandle = be32_to_cpup((__be32 *)&rsp[TPM_HEADER_SIZE]);
> +	phandle_type = phandle & 0xFF000000;
> +
> +	switch (phandle_type) {
> +	case TPM2_HT_TRANSIENT:
> +		vhandle = tpm2_map_to_vhandle(space, phandle, true);
> +		if (!vhandle)
> +			goto out_no_slots;
> +
> +		*(__be32 *)&rsp[TPM_HEADER_SIZE] = cpu_to_be32(vhandle);
> +		break;
> +	case TPM2_HT_HMAC_SESSION:
> +	case TPM2_HT_POLICY_SESSION:
> +		break;
> +	default:
> +		dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
> +			__func__, phandle);
> +		break;
> +	};
> +
> +	return 0;
> +out_no_slots:
> +	tpm2_flush_context_cmd(chip, phandle, TPM_TRANSMIT_UNLOCKED);
> +	dev_warn(&chip->dev, "%s: out of slots for 0x%08X\n", __func__,
> +		 phandle);
> +	return -ENOMEM;
> +}
> +
> +struct tpm2_cap_handles {
> +	u8 more_data;
> +	__be32 capability;
> +	__be32 count;
> +	__be32 handles[];
> +} __packed;
> +
> +static int tpm2_map_response_body(struct tpm_chip *chip, u32 cc, u8 *rsp,
> +				  size_t len)
> +{
> +	struct tpm_space *space = &chip->work_space;
> +	struct tpm_output_header *header = (void *)rsp;
> +	struct tpm2_cap_handles *data;
> +	u32 phandle;
> +	u32 phandle_type;
> +	u32 vhandle;
> +	int i;
> +	int j;
> +
> +	if (cc != TPM2_CC_GET_CAPABILITY ||
> +	    be32_to_cpu(header->return_code) != TPM2_RC_SUCCESS) {
> +		return 0;
> +	}
> +
> +	if (len < TPM_HEADER_SIZE + 9)
> +		return -EFAULT;
> +
> +	data = (void *)&rsp[TPM_HEADER_SIZE];
> +	if (be32_to_cpu(data->capability) != TPM2_CAP_HANDLES)
> +		return 0;
> +
> +	if (len != TPM_HEADER_SIZE + 9 + 4 * be32_to_cpu(data->count))
> +		return -EFAULT;
> +
> +	for (i = 0, j = 0; i < be32_to_cpu(data->count); i++) {
> +		phandle = be32_to_cpup((__be32 *)&data->handles[i]);
> +		phandle_type = phandle & 0xFF000000;
> +
> +		switch (phandle_type) {
> +		case TPM2_HT_TRANSIENT:
> +			vhandle = tpm2_map_to_vhandle(space, phandle, false);
> +			if (!vhandle)
> +				break;
> +
> +			data->handles[j] = cpu_to_be32(vhandle);
> +			j++;
> +			break;
> +		case TPM2_HT_HMAC_SESSION:
> +		case TPM2_HT_POLICY_SESSION:
> +			data->handles[j] = cpu_to_be32(phandle);
> +			j++;
> +			break;
> +		default:
> +			dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
> +				__func__, phandle);
> +			break;
> +		}
> +
> +	}
> +
> +	header->length = cpu_to_be32(TPM_HEADER_SIZE + 9 + 4 * j);
> +	data->count = cpu_to_be32(j);
> +	return 0;
> +}
> +
> +static int tpm2_save_space(struct tpm_chip *chip)
> +{
> +	struct tpm_space *space = &chip->work_space;
> +	unsigned int offset;
> +	int i;
> +	int rc;
> +
> +	for (i = 0, offset = 0; i < ARRAY_SIZE(space->context_tbl); i++) {
> +		if (!(space->context_tbl[i] && ~space->context_tbl[i]))
> +			continue;
> +
> +		rc = tpm2_save_context(chip, space->context_tbl[i],
> +				       space->context_buf, PAGE_SIZE,
> +				       &offset);
> +		if (rc == -ENOENT) {
> +			space->context_tbl[i] = 0;
> +			continue;
> +		} else if (rc)
> +			return rc;
> +
> +		space->context_tbl[i] = ~0;
> +	}
> +
> +	return 0;
> +}
> +
> +int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> +		      u32 cc, u8 *buf, size_t *bufsiz)
> +{
> +	struct tpm_output_header *header = (void *)buf;
> +	int rc;
> +
> +	if (!space)
> +		return 0;
> +
> +	rc = tpm2_map_response_header(chip, cc, buf, *bufsiz);
> +	if (rc) {
> +		tpm2_flush_space(chip);
> +		return rc;
> +	}
> +
> +	rc = tpm2_map_response_body(chip, cc, buf, *bufsiz);
> +	if (rc) {
> +		tpm2_flush_space(chip);
> +		return rc;
> +	}
> +
> +	rc = tpm2_save_space(chip);
> +	if (rc) {
> +		tpm2_flush_space(chip);
> +		return rc;
> +	}
> +
> +	*bufsiz = be32_to_cpu(header->length);
> +
> +	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;
> +}
>

Re: [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/21/2017 1:24 PM, Nayna wrote:
>
> [snip]
>>
>> 1. Take locks.
>> 2. Load transient objects from the backing storage by using ContextLoad
>>     and map virtual handles to physical handles.
>> 3. Perform the transaction.
>> 4. Save transient objects to backing storage by using ContextSave and
>>     map resulting physical handle to virtual handle if there is such.
>>
>> This commit does not implement virtualization support for hmac and
>> policy sessions.
>>
>
> If I have understood discussions correctly, I assume, that kernel TPM
> operations will also be routed via RM. And I think that is not happening
> now with these patches.

There is one corner case that requires kernel operations to go through
the RM, and that's the session context regapping.  If the kernel did not 
go through the RM, it could not handle TPM_RC_CONTEXT_GAP.

I believe that kernel operations such as PCR extend, that don't require
sessions, do not have to go through the RM.

Kernel operations that use objects perhaps can bypass the RM as long as 
there is a lock and the kernel also context saves objects and returns 
the TPM to the empty state before it releases the lock.

Finally, I suspect that the RM should reserve 3 sessions for kernel
operations, so the kernel can't block because user space applications 
have filled all the session slots.


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[James Bottomley]

On Tue, 2017-02-21 at 23:54 +0530, Nayna wrote:
> 
> On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
> > Added an ability to virtualize TPM commands into an isolated 
> > context that we call a TPM space because the word context is 
> > already heavily used in the TPM specification. Both the handle 
> > areas and bodies (where necessary) are virtualized.
> > 
> > The mechanism works by adding a new parameter struct tpm_space to 
> > the tpm_transmit() function. This new structure contains the list 
> > of virtual handles and a buffer of page size (currently) for
> > backing storage.
> > 
> > When tpm_transmit() is called with a struct tpm_space instance it 
> > will execute the following sequence:
> > 
> > 1. Take locks.
> > 2. Load transient objects from the backing storage by using
> > ContextLoad
> >     and map virtual handles to physical handles.
> > 3. Perform the transaction.
> > 4. Save transient objects to backing storage by using ContextSave
> > and
> >     map resulting physical handle to virtual handle if there is
> > such.
> > 
> > This commit does not implement virtualization support for hmac and
> > policy sessions.
> > 
> 
> If I have understood discussions correctly, I assume, that kernel TPM
> operations will also be routed via RM. And I think that is not 
> happening now with these patches.
> 
> Am I missing something ?

Right at the moment the kernel use of tpm2 looks like

acquire chip->tpm_mutex
load key
process key
unload key
release chip->tpm_mutex

While it does this, there's no need for it to have a RM interface
because what it does between the acquisition and drop of the mutex
can't be seen by or have any effect on userspace (whether it uses the
RM or not).  So currently, the question doesn't arise, which is the
situation you see.

When the kernel needs to use resources that persisted beyond it
dropping the chip->tpm_mutex (say using policy or audit sessions), then
it would need to become a customer of the RM.

James

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/22/2017 12:39 PM, James Bottomley wrote:
>
> Right at the moment the kernel use of tpm2 looks like
>
> acquire chip->tpm_mutex
> load key
> process key
> unload key
> release chip->tpm_mutex

The advantage to context save/ context load over load / flush
is that load requires the parent(s).  The parent chain may be long,
a parent may require authorization, or authorization may be impossible 
because PCRs are no longer in the correct state.

In TPM 1.2, there was a performance difference because load was an 
asymmetric key operation, but it's symmetric in TPM 2.0.

> When the kernel needs to use resources that persisted beyond it
> dropping the chip->tpm_mutex (say using policy or audit sessions),
> then it would need to become a customer of the RM.

BTW, use of an EK private key requires a policy session.

Re: [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Jarkko Sakkinen]

On Tue, Feb 21, 2017 at 11:54:50PM +0530, Nayna wrote:
> 
> 
> On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
> > Added an ability to virtualize TPM commands into an isolated context
> > that we call a TPM space because the word context is already heavily
> > used in the TPM specification. Both the handle areas and bodies (where
> > necessary) are virtualized.
> > 
> > The mechanism works by adding a new parameter struct tpm_space to the
> > tpm_transmit() function. This new structure contains the list of virtual
> > handles and a buffer of page size (currently) for backing storage.
> > 
> > When tpm_transmit() is called with a struct tpm_space instance it will
> > execute the following sequence:
> > 
> > 1. Take locks.
> > 2. Load transient objects from the backing storage by using ContextLoad
> >     and map virtual handles to physical handles.
> > 3. Perform the transaction.
> > 4. Save transient objects to backing storage by using ContextSave and
> >     map resulting physical handle to virtual handle if there is such.
> > 
> > This commit does not implement virtualization support for hmac and
> > policy sessions.
> > 
> 
> If I have understood discussions correctly, I assume, that kernel TPM
> operations will also be routed via RM. And I think that is not happening now
> with these patches.
> 
> Am I missing something ?
> 
> Thanks & Regards,
>    - Nayna

Nope. It's not in the scope of this patch set and there are not
kernel use cases at the moment that would require it.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 7/7] tpm2: add session handle context saving and restoring to the space code

[Jarkko Sakkinen]

On Thu, Feb 16, 2017 at 09:25:20PM +0200, Jarkko Sakkinen wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Sessions are different from transient objects in that their handles
> may not be virtualized (because they're used for some hmac
> calculations).  Additionally when a session is context saved, a
> vestigial memory remains in the TPM and if it is also flushed, that
> will be lost and the session context will refuse to load next time, so
> the code is updated to flush only transient objects after a context
> save.  Add a separate array (chip->session_tbl) to save and restore
> sessions by handle.  Use the failure of a context save or load to
> signal that the session has been flushed from the TPM and we can
> remove its memory from chip->session_tbl.
> 
> Sessions are also isolated during each instance of a tpm space.  This
> means that spaces shouldn't be able to see each other's sessions and
> is enforced by ensuring that a space user may only refer to sessions
> handles that are present in their own chip->session_tbl.  Finally when
> a space is closed, all the sessions belonging to it should be flushed
> so the handles may be re-used by other spaces.
> 
> Note that if we get a session save or load error, all sessions are
> effectively flushed.  Even though we restore the session buffer, all
> the old sessions will refuse to load after the flush and they'll be
> purged from our session memory.  This means that while transient
> context handling is still soft in the face of errors, session handling
> is hard (any failure of the model means all sessions are lost).
> 
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>

I'll give Tested-by separately once I've updated tpm2_smoke.py

/Jarkko

> ---
>  drivers/char/tpm/tpm-chip.c   |   6 +++
>  drivers/char/tpm/tpm.h        |   4 +-
>  drivers/char/tpm/tpm2-space.c | 105 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/char/tpm/tpms-dev.c   |   2 +-
>  4 files changed, 113 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index c71c353..e8536ab 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -130,6 +130,7 @@ static void tpm_dev_release(struct device *dev)
>  
>  	kfree(chip->log.bios_event_log);
>  	kfree(chip->work_space.context_buf);
> +	kfree(chip->work_space.session_buf);
>  	kfree(chip);
>  }
>  
> @@ -224,6 +225,11 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  		rc = -ENOMEM;
>  		goto out;
>  	}
> +	chip->work_space.session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (!chip->work_space.session_buf) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
>  
>  	return chip;
>  
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 822ca67..ffee8b5 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -161,6 +161,8 @@ enum tpm2_cc_attrs {
>  struct tpm_space {
>  	u32 context_tbl[3];
>  	u8 *context_buf;
> +	u32 session_tbl[3];
> +	u8 *session_buf;
>  };
>  
>  enum tpm_chip_flags {
> @@ -589,7 +591,7 @@ int tpm2_probe(struct tpm_chip *chip);
>  ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
>  int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
>  int tpm2_init_space(struct tpm_space *space);
> -void tpm2_del_space(struct tpm_space *space);
> +void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space);
>  int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
>  		       u8 *cmd);
>  int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
> diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c
> index e955548..d36d81e 100644
> --- a/drivers/char/tpm/tpm2-space.c
> +++ b/drivers/char/tpm/tpm2-space.c
> @@ -32,18 +32,39 @@ struct tpm2_context {
>  	__be16 blob_size;
>  } __packed;
>  
> +static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
> +		if (space->session_tbl[i])
> +			tpm2_flush_context_cmd(chip, space->session_tbl[i],
> +					       TPM_TRANSMIT_UNLOCKED);
> +	}
> +}
> +
>  int tpm2_init_space(struct tpm_space *space)
>  {
>  	space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
>  	if (!space->context_buf)
>  		return -ENOMEM;
>  
> +	space->session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
> +	if (space->session_buf == NULL) {
> +		kfree(space->context_buf);
> +		return -ENOMEM;
> +	}
> +
>  	return 0;
>  }
>  
> -void tpm2_del_space(struct tpm_space *space)
> +void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space)
>  {
> +	mutex_lock(&chip->tpm_mutex);
> +	tpm2_flush_sessions(chip, space);
> +	mutex_unlock(&chip->tpm_mutex);
>  	kfree(space->context_buf);
> +	kfree(space->session_buf);
>  }
>  
>  static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
> @@ -69,6 +90,20 @@ static int tpm2_load_context(struct tpm_chip *chip, u8 *buf,
>  			 __func__, rc);
>  		tpm_buf_destroy(&tbuf);
>  		return -EFAULT;
> +	} else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE ||
> +		   rc == TPM2_RC_REFERENCE_H0) {
> +		/*
> +		 * TPM_RC_HANDLE means that the session context can't
> +		 * be loaded because of an internal counter mismatch
> +		 * that makes the TPM think there might have been a
> +		 * replay.  This might happen if the context was saved
> +		 * and loaded outside the space.
> +		 *
> +		 * TPM_RC_REFERENCE_H0 means the session has been
> +		 * flushed outside the space
> +		 */
> +		rc = -ENOENT;
> +		tpm_buf_destroy(&tbuf);
>  	} else if (rc > 0) {
>  		dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n",
>  			 __func__, rc);
> @@ -121,7 +156,6 @@ static int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf,
>  	}
>  
>  	memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size);
> -	tpm2_flush_context_cmd(chip, handle, TPM_TRANSMIT_UNLOCKED);
>  	*offset += body_size;
>  	tpm_buf_destroy(&tbuf);
>  	return 0;
> @@ -136,6 +170,8 @@ static void tpm2_flush_space(struct tpm_chip *chip)
>  		if (space->context_tbl[i] && ~space->context_tbl[i])
>  			tpm2_flush_context_cmd(chip, space->context_tbl[i],
>  					       TPM_TRANSMIT_UNLOCKED);
> +
> +	tpm2_flush_sessions(chip, space);
>  }
>  
>  static int tpm2_load_space(struct tpm_chip *chip)
> @@ -161,6 +197,28 @@ static int tpm2_load_space(struct tpm_chip *chip)
>  			return rc;
>  	}
>  
> +	for (i = 0, offset = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
> +		u32 handle;
> +
> +		if (!space->session_tbl[i])
> +			continue;
> +
> +		rc = tpm2_load_context(chip, space->session_buf,
> +				       &offset, &handle);
> +		if (rc == -ENOENT) {
> +			/* load failed, just forget session */
> +			space->session_tbl[i] = 0;
> +		} else if (rc) {
> +			tpm2_flush_space(chip);
> +			return rc;
> +		}
> +		if (handle != space->session_tbl[i]) {
> +			dev_warn(&chip->dev, "session restored to wrong handle\n");
> +			tpm2_flush_space(chip);
> +			return -EFAULT;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -215,7 +273,10 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
>  
>  	memcpy(&chip->work_space.context_tbl, &space->context_tbl,
>  	       sizeof(space->context_tbl));
> +	memcpy(&chip->work_space.session_tbl, &space->session_tbl,
> +	       sizeof(space->session_tbl));
>  	memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE);
> +	memcpy(chip->work_space.session_buf, space->session_buf, PAGE_SIZE);
>  
>  	rc = tpm2_load_space(chip);
>  	if (rc) {
> @@ -232,6 +293,22 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 cc,
>  	return 0;
>  }
>  
> +static bool tpm2_add_session(struct tpm_chip *chip, u32 handle)
> +{
> +	struct tpm_space *space = &chip->work_space;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(space->session_tbl); i++)
> +		if (space->session_tbl[i] == 0)
> +			break;
> +
> +	if (i == ARRAY_SIZE(space->session_tbl))
> +		return false;
> +
> +	space->session_tbl[i] = handle;
> +	return true;
> +}
> +
>  static u32 tpm2_map_to_vhandle(struct tpm_space *space, u32 phandle, bool alloc)
>  {
>  	int i;
> @@ -288,6 +365,8 @@ static int tpm2_map_response_header(struct tpm_chip *chip, u32 cc, u8 *rsp,
>  		break;
>  	case TPM2_HT_HMAC_SESSION:
>  	case TPM2_HT_POLICY_SESSION:
> +		if (!tpm2_add_session(chip, phandle))
> +			goto out_no_slots;
>  		break;
>  	default:
>  		dev_err(&chip->dev, "%s: unknown handle 0x%08X\n",
> @@ -388,9 +467,28 @@ static int tpm2_save_space(struct tpm_chip *chip)
>  		} else if (rc)
>  			return rc;
>  
> +		tpm2_flush_context_cmd(chip, space->context_tbl[i],
> +				       TPM_TRANSMIT_UNLOCKED);
>  		space->context_tbl[i] = ~0;
>  	}
>  
> +	for (i = 0, offset = 0; i < ARRAY_SIZE(space->session_tbl); i++) {
> +		if (!space->session_tbl[i])
> +			continue;
> +
> +		rc = tpm2_save_context(chip, space->session_tbl[i],
> +				       space->session_buf, PAGE_SIZE,
> +				       &offset);
> +
> +		if (rc == -ENOENT) {
> +			/* handle error saving session, just forget it */
> +			space->session_tbl[i] = 0;
> +		} else if (rc < 0) {
> +			tpm2_flush_space(chip);
> +			return rc;
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -425,7 +523,10 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space,
>  
>  	memcpy(&space->context_tbl, &chip->work_space.context_tbl,
>  	       sizeof(space->context_tbl));
> +	memcpy(&space->session_tbl, &chip->work_space.session_tbl,
> +	       sizeof(space->session_tbl));
>  	memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE);
> +	memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE);
>  
>  	return 0;
>  }
> diff --git a/drivers/char/tpm/tpms-dev.c b/drivers/char/tpm/tpms-dev.c
> index 5720885..4c98db8 100644
> --- a/drivers/char/tpm/tpms-dev.c
> +++ b/drivers/char/tpm/tpms-dev.c
> @@ -39,7 +39,7 @@ static int tpms_release(struct inode *inode, struct file *file)
>  	struct tpms_priv *priv = container_of(fpriv, struct tpms_priv, priv);
>  
>  	tpm_common_release(file, fpriv);
> -	tpm2_del_space(&priv->space);
> +	tpm2_del_space(fpriv->chip, &priv->space);
>  	kfree(priv);
>  
>  	return 0;
> -- 
> 2.9.3
> 

Re: [PATCH v2 5/7] tpm: split out tpm-dev.c into tpm-dev.c and tpm-common-dev.c

[Jarkko Sakkinen]

On Thu, Feb 16, 2017 at 09:25:18PM +0200, Jarkko Sakkinen wrote:
> From: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
> 
> Signed-off-by: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkine-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Tested-by: Jarkko Sakkinen <jarkko.sakkine-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

/Jarkko

> ---
>  drivers/char/tpm/Makefile         |   2 +-
>  drivers/char/tpm/tpm-dev-common.c | 148 ++++++++++++++++++++++++++++++++++++++
>  drivers/char/tpm/tpm-dev.c        | 143 ++++--------------------------------
>  drivers/char/tpm/tpm-dev.h        |  27 +++++++
>  4 files changed, 190 insertions(+), 130 deletions(-)
>  create mode 100644 drivers/char/tpm/tpm-dev-common.c
>  create mode 100644 drivers/char/tpm/tpm-dev.h
> 
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 8f07fcf..10e5827 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 \
> -	 tpm1_eventlog.o tpm2_eventlog.o tpm2-space.o
> +	 tpm-dev-common.o tpm1_eventlog.o tpm2_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-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> new file mode 100644
> index 0000000..610638a
> --- /dev/null
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -0,0 +1,148 @@
> +/*
> + * Copyright (C) 2004 IBM Corporation
> + * Authors:
> + * Leendert van Doorn <leendert-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
> + * Dave Safford <safford-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
> + * Reiner Sailer <sailer-aZOuKsOsJu3MbYB6QlFGEg@public.gmane.org>
> + * Kylene Hall <kjhall-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> + *
> + * Copyright (C) 2013 Obsidian Research Corp
> + * Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> + *
> + * Device file system interface to the TPM
> + *
> + * 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/slab.h>
> +#include <linux/uaccess.h>
> +#include "tpm.h"
> +#include "tpm-dev.h"
> +
> +static void user_reader_timeout(unsigned long ptr)
> +{
> +	struct file_priv *priv = (struct file_priv *)ptr;
> +
> +	pr_warn("TPM user space timeout is deprecated (pid=%d)\n",
> +		task_tgid_nr(current));
> +
> +	schedule_work(&priv->work);
> +}
> +
> +static void timeout_work(struct work_struct *work)
> +{
> +	struct file_priv *priv = container_of(work, struct file_priv, work);
> +
> +	mutex_lock(&priv->buffer_mutex);
> +	atomic_set(&priv->data_pending, 0);
> +	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
> +	mutex_unlock(&priv->buffer_mutex);
> +}
> +
> +void tpm_common_open(struct file *file, struct tpm_chip *chip,
> +		     struct file_priv *priv)
> +{
> +	priv->chip = chip;
> +	atomic_set(&priv->data_pending, 0);
> +	mutex_init(&priv->buffer_mutex);
> +	setup_timer(&priv->user_read_timer, user_reader_timeout,
> +			(unsigned long)priv);
> +	INIT_WORK(&priv->work, timeout_work);
> +
> +	file->private_data = priv;
> +}
> +
> +ssize_t tpm_common_read(struct file *file, char __user *buf,
> +			size_t size, loff_t *off)
> +{
> +	struct file_priv *priv = file->private_data;
> +	ssize_t ret_size;
> +	ssize_t orig_ret_size;
> +	int rc;
> +
> +	del_singleshot_timer_sync(&priv->user_read_timer);
> +	flush_work(&priv->work);
> +	ret_size = atomic_read(&priv->data_pending);
> +	if (ret_size > 0) {	/* relay data */
> +		orig_ret_size = ret_size;
> +		if (size < ret_size)
> +			ret_size = size;
> +
> +		mutex_lock(&priv->buffer_mutex);
> +		rc = copy_to_user(buf, priv->data_buffer, ret_size);
> +		memset(priv->data_buffer, 0, orig_ret_size);
> +		if (rc)
> +			ret_size = -EFAULT;
> +
> +		mutex_unlock(&priv->buffer_mutex);
> +	}
> +
> +	atomic_set(&priv->data_pending, 0);
> +
> +	return ret_size;
> +}
> +
> +ssize_t tpm_common_write(struct file *file, const char __user *buf,
> +			 size_t size, loff_t *off, struct tpm_space *space)
> +{
> +	struct file_priv *priv = file->private_data;
> +	size_t in_size = size;
> +	ssize_t out_size;
> +
> +	/* Cannot perform a write until the read has cleared either via
> +	 * tpm_read or a user_read_timer timeout. This also prevents split
> +	 * buffered writes from blocking here.
> +	 */
> +	if (atomic_read(&priv->data_pending) != 0)
> +		return -EBUSY;
> +
> +	if (in_size > TPM_BUFSIZE)
> +		return -E2BIG;
> +
> +	mutex_lock(&priv->buffer_mutex);
> +
> +	if (copy_from_user
> +	    (priv->data_buffer, (void __user *) buf, in_size)) {
> +		mutex_unlock(&priv->buffer_mutex);
> +		return -EFAULT;
> +	}
> +
> +	/* atomic tpm command send and result receive. We only hold the ops
> +	 * lock during this period so that the tpm can be unregistered even if
> +	 * the char dev is held open.
> +	 */
> +	if (tpm_try_get_ops(priv->chip)) {
> +		mutex_unlock(&priv->buffer_mutex);
> +		return -EPIPE;
> +	}
> +	out_size = tpm_transmit(priv->chip, space, priv->data_buffer,
> +				sizeof(priv->data_buffer), 0);
> +
> +	tpm_put_ops(priv->chip);
> +	if (out_size < 0) {
> +		mutex_unlock(&priv->buffer_mutex);
> +		return out_size;
> +	}
> +
> +	atomic_set(&priv->data_pending, out_size);
> +	mutex_unlock(&priv->buffer_mutex);
> +
> +	/* Set a timeout by which the reader must come claim the result */
> +	mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
> +
> +	return in_size;
> +}
> +
> +/*
> + * Called on file close
> + */
> +void tpm_common_release(struct file *file, struct file_priv *priv)
> +{
> +	del_singleshot_timer_sync(&priv->user_read_timer);
> +	flush_work(&priv->work);
> +	file->private_data = NULL;
> +	atomic_set(&priv->data_pending, 0);
> +}
> diff --git a/drivers/char/tpm/tpm-dev.c b/drivers/char/tpm/tpm-dev.c
> index 414553b..ebd74ab 100644
> --- a/drivers/char/tpm/tpm-dev.c
> +++ b/drivers/char/tpm/tpm-dev.c
> @@ -18,48 +18,15 @@
>   *
>   */
>  #include <linux/slab.h>
> -#include <linux/uaccess.h>
> -#include "tpm.h"
> -
> -struct file_priv {
> -	struct tpm_chip *chip;
> -
> -	/* Data passed to and from the tpm via the read/write calls */
> -	atomic_t data_pending;
> -	struct mutex buffer_mutex;
> -
> -	struct timer_list user_read_timer;      /* user needs to claim result */
> -	struct work_struct work;
> -
> -	u8 data_buffer[TPM_BUFSIZE];
> -};
> -
> -static void user_reader_timeout(unsigned long ptr)
> -{
> -	struct file_priv *priv = (struct file_priv *)ptr;
> -
> -	pr_warn("TPM user space timeout is deprecated (pid=%d)\n",
> -		task_tgid_nr(current));
> -
> -	schedule_work(&priv->work);
> -}
> -
> -static void timeout_work(struct work_struct *work)
> -{
> -	struct file_priv *priv = container_of(work, struct file_priv, work);
> -
> -	mutex_lock(&priv->buffer_mutex);
> -	atomic_set(&priv->data_pending, 0);
> -	memset(priv->data_buffer, 0, sizeof(priv->data_buffer));
> -	mutex_unlock(&priv->buffer_mutex);
> -}
> +#include "tpm-dev.h"
>  
>  static int tpm_open(struct inode *inode, struct file *file)
>  {
> -	struct tpm_chip *chip =
> -		container_of(inode->i_cdev, struct tpm_chip, cdev);
> +	struct tpm_chip *chip;
>  	struct file_priv *priv;
>  
> +	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. */
> @@ -69,100 +36,22 @@ static int tpm_open(struct inode *inode, struct file *file)
>  	}
>  
>  	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> -	if (priv == NULL) {
> -		clear_bit(0, &chip->is_open);
> -		return -ENOMEM;
> -	}
> +	if (priv == NULL)
> +		goto out;
>  
> -	priv->chip = chip;
> -	atomic_set(&priv->data_pending, 0);
> -	mutex_init(&priv->buffer_mutex);
> -	setup_timer(&priv->user_read_timer, user_reader_timeout,
> -			(unsigned long)priv);
> -	INIT_WORK(&priv->work, timeout_work);
> +	tpm_common_open(file, chip, priv);
>  
> -	file->private_data = priv;
>  	return 0;
> -}
> -
> -static ssize_t tpm_read(struct file *file, char __user *buf,
> -			size_t size, loff_t *off)
> -{
> -	struct file_priv *priv = file->private_data;
> -	ssize_t ret_size;
> -	int rc;
>  
> -	del_singleshot_timer_sync(&priv->user_read_timer);
> -	flush_work(&priv->work);
> -	ret_size = atomic_read(&priv->data_pending);
> -	if (ret_size > 0) {	/* relay data */
> -		ssize_t orig_ret_size = ret_size;
> -		if (size < ret_size)
> -			ret_size = size;
> -
> -		mutex_lock(&priv->buffer_mutex);
> -		rc = copy_to_user(buf, priv->data_buffer, ret_size);
> -		memset(priv->data_buffer, 0, orig_ret_size);
> -		if (rc)
> -			ret_size = -EFAULT;
> -
> -		mutex_unlock(&priv->buffer_mutex);
> -	}
> -
> -	atomic_set(&priv->data_pending, 0);
> -
> -	return ret_size;
> + out:
> +	clear_bit(0, &chip->is_open);
> +	return -ENOMEM;
>  }
>  
>  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;
> -	size_t in_size = size;
> -	ssize_t out_size;
> -
> -	/* cannot perform a write until the read has cleared
> -	   either via tpm_read or a user_read_timer timeout.
> -	   This also prevents splitted buffered writes from blocking here.
> -	*/
> -	if (atomic_read(&priv->data_pending) != 0)
> -		return -EBUSY;
> -
> -	if (in_size > TPM_BUFSIZE)
> -		return -E2BIG;
> -
> -	mutex_lock(&priv->buffer_mutex);
> -
> -	if (copy_from_user
> -	    (priv->data_buffer, (void __user *) buf, in_size)) {
> -		mutex_unlock(&priv->buffer_mutex);
> -		return -EFAULT;
> -	}
> -
> -	/* atomic tpm command send and result receive. We only hold the ops
> -	 * lock during this period so that the tpm can be unregistered even if
> -	 * the char dev is held open.
> -	 */
> -	if (tpm_try_get_ops(priv->chip)) {
> -		mutex_unlock(&priv->buffer_mutex);
> -		return -EPIPE;
> -	}
> -	out_size = tpm_transmit(priv->chip, NULL, priv->data_buffer,
> -				sizeof(priv->data_buffer), 0);
> -
> -	tpm_put_ops(priv->chip);
> -	if (out_size < 0) {
> -		mutex_unlock(&priv->buffer_mutex);
> -		return out_size;
> -	}
> -
> -	atomic_set(&priv->data_pending, out_size);
> -	mutex_unlock(&priv->buffer_mutex);
> -
> -	/* Set a timeout by which the reader must come claim the result */
> -	mod_timer(&priv->user_read_timer, jiffies + (120 * HZ));
> -
> -	return in_size;
> +	return tpm_common_write(file, buf, size, off, NULL);
>  }
>  
>  /*
> @@ -172,12 +61,10 @@ static int tpm_release(struct inode *inode, struct file *file)
>  {
>  	struct file_priv *priv = file->private_data;
>  
> -	del_singleshot_timer_sync(&priv->user_read_timer);
> -	flush_work(&priv->work);
> -	file->private_data = NULL;
> -	atomic_set(&priv->data_pending, 0);
> +	tpm_common_release(file, priv);
>  	clear_bit(0, &priv->chip->is_open);
>  	kfree(priv);
> +
>  	return 0;
>  }
>  
> @@ -185,9 +72,7 @@ const struct file_operations tpm_fops = {
>  	.owner = THIS_MODULE,
>  	.llseek = no_llseek,
>  	.open = tpm_open,
> -	.read = tpm_read,
> +	.read = tpm_common_read,
>  	.write = tpm_write,
>  	.release = tpm_release,
>  };
> -
> -
> diff --git a/drivers/char/tpm/tpm-dev.h b/drivers/char/tpm/tpm-dev.h
> new file mode 100644
> index 0000000..ff15cf7
> --- /dev/null
> +++ b/drivers/char/tpm/tpm-dev.h
> @@ -0,0 +1,27 @@
> +#ifndef _TPM_DEV_H
> +#define _TPM_DEV_H
> +
> +#include "tpm.h"
> +
> +struct file_priv {
> +	struct tpm_chip *chip;
> +
> +	/* Data passed to and from the tpm via the read/write calls */
> +	atomic_t data_pending;
> +	struct mutex buffer_mutex;
> +
> +	struct timer_list user_read_timer;      /* user needs to claim result */
> +	struct work_struct work;
> +
> +	u8 data_buffer[TPM_BUFSIZE];
> +};
> +
> +void tpm_common_open(struct file *file, struct tpm_chip *chip,
> +		     struct file_priv *priv);
> +ssize_t tpm_common_read(struct file *file, char __user *buf,
> +			size_t size, loff_t *off);
> +ssize_t tpm_common_write(struct file *file, const char __user *buf,
> +			 size_t size, loff_t *off, struct tpm_space *space);
> +void tpm_common_release(struct file *file, struct file_priv *priv);
> +
> +#endif
> -- 
> 2.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jarkko Sakkinen]

On Thu, Feb 16, 2017 at 09:25:19PM +0200, Jarkko Sakkinen wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Currently the tpm spaces are 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 space.
> 
> 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>

Reviewed-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>

Nitpicking but I've been thinking about naming. What about calling the
device as tpmrc0 as in resource context. I think that would be a better
name than TPM space. You do not mix it up with namespaces and/or
virtualization. With resource in front it cannot be easily mixed up with
TPM contexts either.

This does not require any effort from your side. I could do the
renaming.

PS. Could you go through my commits and test and review them at some
point so we would have the whole patch set peer tested?

/Jarkko

> ---
>  drivers/char/tpm/Makefile        |  3 +-
>  drivers/char/tpm/tpm-chip.c      | 73 ++++++++++++++++++++++++++++++++++++++--
>  drivers/char/tpm/tpm-interface.c | 13 +++++--
>  drivers/char/tpm/tpm.h           |  4 +++
>  drivers/char/tpm/tpms-dev.c      | 65 +++++++++++++++++++++++++++++++++++
>  5 files changed, 152 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/char/tpm/tpms-dev.c
> 
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 10e5827..bbe6531 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -3,7 +3,8 @@
>  #
>  obj-$(CONFIG_TCG_TPM) += tpm.o
>  tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
> -	 tpm-dev-common.o tpm1_eventlog.o tpm2_eventlog.o tpm2-space.o
> +	 tpm-dev-common.o tpms-dev.o tpm1_eventlog.o tpm2_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 993b9ae..c71c353 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 *tpms_class;
>  dev_t tpm_devt;
>  
>  /**
> @@ -132,6 +133,14 @@ static void tpm_dev_release(struct device *dev)
>  	kfree(chip);
>  }
>  
> +static void tpm_devs_release(struct device *dev)
> +{
> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip, devs);
> +
> +	/* release the master device reference */
> +	put_device(&chip->dev);
> +}
> +
>  /**
>   * tpm_chip_alloc() - allocate a new struct tpm_chip instance
>   * @pdev: device to which the chip is associated
> @@ -168,27 +177,47 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  	chip->dev_num = rc;
>  
>  	device_initialize(&chip->dev);
> +	device_initialize(&chip->devs);
>  
>  	chip->dev.class = tpm_class;
>  	chip->dev.release = tpm_dev_release;
>  	chip->dev.parent = pdev;
>  	chip->dev.groups = chip->groups;
>  
> +	chip->devs.parent = pdev;
> +	chip->devs.class = tpms_class;
> +	chip->devs.release = tpm_devs_release;
> +	/* get extra reference on main device to hold on
> +	 * behalf of devs.  This holds the chip structure
> +	 * while cdevs is in use.  The corresponding put
> +	 * is in the tpm_devs_release
> +	 */
> +	get_device(&chip->dev);
> +
>  	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->devs.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->devs, "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->cdevs, &tpms_fops);
>  	chip->cdev.owner = THIS_MODULE;
> +	chip->cdevs.owner = THIS_MODULE;
>  	chip->cdev.kobj.parent = &chip->dev.kobj;
> +	chip->cdevs.kobj.parent = &chip->devs.kobj;
>  
>  	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
>  	if (!chip->work_space.context_buf) {
> @@ -199,6 +228,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>  	return chip;
>  
>  out:
> +	put_device(&chip->devs);
>  	put_device(&chip->dev);
>  	return ERR_PTR(rc);
>  }
> @@ -244,7 +274,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 +284,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->cdevs, chip->devs.devt, 1);
> +	if (rc) {
> +		dev_err(&chip->dev,
> +			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
> +			dev_name(&chip->devs), MAJOR(chip->devs.devt),
> +			MINOR(chip->devs.devt), rc);
> +
> +		goto err_3;
>  	}
>  
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		rc = device_add(&chip->devs);
> +	if (rc) {
> +		dev_err(&chip->dev,
> +			"unable to device_register() %s, major %d, minor %d, err=%d\n",
> +			dev_name(&chip->devs), MAJOR(chip->devs.devt),
> +			MINOR(chip->devs.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->cdevs);
> + 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 +329,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->cdevs);
> +		device_del(&chip->devs);
> +	}
> +
>  	/* Make the chip unavailable. */
>  	mutex_lock(&idr_lock);
>  	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
> @@ -282,6 +345,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>  		tpm2_shutdown(chip, TPM2_SU_CLEAR);
>  	chip->ops = NULL;
>  	up_write(&chip->ops_sem);
> +	/* will release the devs reference to the chip->dev unless
> +	 * something has cdevs open
> +	 */
> +	put_device(&chip->devs);
>  }
>  
>  static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index db5ffe9..deb2021 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -1257,9 +1257,17 @@ static int __init tpm_init(void)
>  		return PTR_ERR(tpm_class);
>  	}
>  
> -	rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES, "tpm");
> +	tpms_class = class_create(THIS_MODULE, "tpms");
> +	if (IS_ERR(tpms_class)) {
> +		pr_err("couldn't create tpms class\n");
> +		class_destroy(tpm_class);
> +		return PTR_ERR(tpms_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(tpms_class);
>  		class_destroy(tpm_class);
>  		return rc;
>  	}
> @@ -1271,7 +1279,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(tpms_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 97e48a4..822ca67 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -182,7 +182,9 @@ struct tpm_chip_seqops {
>  
>  struct tpm_chip {
>  	struct device dev;
> +	struct device devs;
>  	struct cdev cdev;
> +	struct cdev cdevs;
>  
>  	/* 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 +512,10 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
>  }
>  
>  extern struct class *tpm_class;
> +extern struct class *tpms_class;
>  extern dev_t tpm_devt;
>  extern const struct file_operations tpm_fops;
> +extern const struct file_operations tpms_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..5720885
> --- /dev/null
> +++ b/drivers/char/tpm/tpms-dev.c
> @@ -0,0 +1,65 @@
> +/*
> + * 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;
> +	int rc;
> +
> +	chip = container_of(inode->i_cdev, struct tpm_chip, cdevs);
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (priv == NULL)
> +		return -ENOMEM;
> +
> +	rc = tpm2_init_space(&priv->space);
> +	if (rc) {
> +		kfree(priv);
> +		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);
> +	tpm2_del_space(&priv->space);
> +	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 tpms_fops = {
> +	.owner = THIS_MODULE,
> +	.llseek = no_llseek,
> +	.open = tpms_open,
> +	.read = tpm_common_read,
> +	.write = tpms_write,
> +	.release = tpms_release,
> +};
> +
> -- 
> 2.9.3
> 

Re: [tpmdd-devel] [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Nayna]

On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
> From: James Bottomley <James.Bottomley@HansenPartnership.com>
>
> Currently the tpm spaces are 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 space.
>
> 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.

To understand, I have two questions:

1. How would a userspace application using TPM know whether to use 
/dev/tpm0 or /dev/tpms0 ?

2. How would a userspace RM know to build on top of /dev/tpm0 or 
/dev/tpms0. And if it is built on top of /dev/tpms0, can there be issues 
with one RM on top of other RM.

Thanks & Regards,
    - Nayna


>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>   drivers/char/tpm/Makefile        |  3 +-
>   drivers/char/tpm/tpm-chip.c      | 73 ++++++++++++++++++++++++++++++++++++++--
>   drivers/char/tpm/tpm-interface.c | 13 +++++--
>   drivers/char/tpm/tpm.h           |  4 +++
>   drivers/char/tpm/tpms-dev.c      | 65 +++++++++++++++++++++++++++++++++++
>   5 files changed, 152 insertions(+), 6 deletions(-)
>   create mode 100644 drivers/char/tpm/tpms-dev.c
>
> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> index 10e5827..bbe6531 100644
> --- a/drivers/char/tpm/Makefile
> +++ b/drivers/char/tpm/Makefile
> @@ -3,7 +3,8 @@
>   #
>   obj-$(CONFIG_TCG_TPM) += tpm.o
>   tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
> -	 tpm-dev-common.o tpm1_eventlog.o tpm2_eventlog.o tpm2-space.o
> +	 tpm-dev-common.o tpms-dev.o tpm1_eventlog.o tpm2_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 993b9ae..c71c353 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 *tpms_class;
>   dev_t tpm_devt;
>
>   /**
> @@ -132,6 +133,14 @@ static void tpm_dev_release(struct device *dev)
>   	kfree(chip);
>   }
>
> +static void tpm_devs_release(struct device *dev)
> +{
> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip, devs);
> +
> +	/* release the master device reference */
> +	put_device(&chip->dev);
> +}
> +
>   /**
>    * tpm_chip_alloc() - allocate a new struct tpm_chip instance
>    * @pdev: device to which the chip is associated
> @@ -168,27 +177,47 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>   	chip->dev_num = rc;
>
>   	device_initialize(&chip->dev);
> +	device_initialize(&chip->devs);
>
>   	chip->dev.class = tpm_class;
>   	chip->dev.release = tpm_dev_release;
>   	chip->dev.parent = pdev;
>   	chip->dev.groups = chip->groups;
>
> +	chip->devs.parent = pdev;
> +	chip->devs.class = tpms_class;
> +	chip->devs.release = tpm_devs_release;
> +	/* get extra reference on main device to hold on
> +	 * behalf of devs.  This holds the chip structure
> +	 * while cdevs is in use.  The corresponding put
> +	 * is in the tpm_devs_release
> +	 */
> +	get_device(&chip->dev);
> +
>   	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->devs.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->devs, "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->cdevs, &tpms_fops);
>   	chip->cdev.owner = THIS_MODULE;
> +	chip->cdevs.owner = THIS_MODULE;
>   	chip->cdev.kobj.parent = &chip->dev.kobj;
> +	chip->cdevs.kobj.parent = &chip->devs.kobj;
>
>   	chip->work_space.context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL);
>   	if (!chip->work_space.context_buf) {
> @@ -199,6 +228,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
>   	return chip;
>
>   out:
> +	put_device(&chip->devs);
>   	put_device(&chip->dev);
>   	return ERR_PTR(rc);
>   }
> @@ -244,7 +274,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 +284,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->cdevs, chip->devs.devt, 1);
> +	if (rc) {
> +		dev_err(&chip->dev,
> +			"unable to cdev_add() %s, major %d, minor %d, err=%d\n",
> +			dev_name(&chip->devs), MAJOR(chip->devs.devt),
> +			MINOR(chip->devs.devt), rc);
> +
> +		goto err_3;
>   	}
>
> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> +		rc = device_add(&chip->devs);
> +	if (rc) {
> +		dev_err(&chip->dev,
> +			"unable to device_register() %s, major %d, minor %d, err=%d\n",
> +			dev_name(&chip->devs), MAJOR(chip->devs.devt),
> +			MINOR(chip->devs.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->cdevs);
> + 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 +329,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->cdevs);
> +		device_del(&chip->devs);
> +	}
> +
>   	/* Make the chip unavailable. */
>   	mutex_lock(&idr_lock);
>   	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
> @@ -282,6 +345,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>   		tpm2_shutdown(chip, TPM2_SU_CLEAR);
>   	chip->ops = NULL;
>   	up_write(&chip->ops_sem);
> +	/* will release the devs reference to the chip->dev unless
> +	 * something has cdevs open
> +	 */
> +	put_device(&chip->devs);
>   }
>
>   static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index db5ffe9..deb2021 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -1257,9 +1257,17 @@ static int __init tpm_init(void)
>   		return PTR_ERR(tpm_class);
>   	}
>
> -	rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES, "tpm");
> +	tpms_class = class_create(THIS_MODULE, "tpms");
> +	if (IS_ERR(tpms_class)) {
> +		pr_err("couldn't create tpms class\n");
> +		class_destroy(tpm_class);
> +		return PTR_ERR(tpms_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(tpms_class);
>   		class_destroy(tpm_class);
>   		return rc;
>   	}
> @@ -1271,7 +1279,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(tpms_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 97e48a4..822ca67 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -182,7 +182,9 @@ struct tpm_chip_seqops {
>
>   struct tpm_chip {
>   	struct device dev;
> +	struct device devs;
>   	struct cdev cdev;
> +	struct cdev cdevs;
>
>   	/* 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 +512,10 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
>   }
>
>   extern struct class *tpm_class;
> +extern struct class *tpms_class;
>   extern dev_t tpm_devt;
>   extern const struct file_operations tpm_fops;
> +extern const struct file_operations tpms_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..5720885
> --- /dev/null
> +++ b/drivers/char/tpm/tpms-dev.c
> @@ -0,0 +1,65 @@
> +/*
> + * 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;
> +	int rc;
> +
> +	chip = container_of(inode->i_cdev, struct tpm_chip, cdevs);
> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> +	if (priv == NULL)
> +		return -ENOMEM;
> +
> +	rc = tpm2_init_space(&priv->space);
> +	if (rc) {
> +		kfree(priv);
> +		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);
> +	tpm2_del_space(&priv->space);
> +	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 tpms_fops = {
> +	.owner = THIS_MODULE,
> +	.llseek = no_llseek,
> +	.open = tpms_open,
> +	.read = tpm_common_read,
> +	.write = tpms_write,
> +	.release = tpms_release,
> +};
> +
>

Re: [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[James Bottomley]

On Thu, 2017-02-16 at 21:25 +0200, Jarkko Sakkinen wrote:
> Added an ability to virtualize TPM commands into an isolated context
> that we call a TPM space because the word context is already heavily
> used in the TPM specification. Both the handle areas and bodies
> (where
> necessary) are virtualized.
> 
> The mechanism works by adding a new parameter struct tpm_space to the
> tpm_transmit() function. This new structure contains the list of
> virtual
> handles and a buffer of page size (currently) for backing storage.
> 
> When tpm_transmit() is called with a struct tpm_space instance it
> will
> execute the following sequence:
> 
> 1. Take locks.
> 2. Load transient objects from the backing storage by using
> ContextLoad
>    and map virtual handles to physical handles.
> 3. Perform the transaction.
> 4. Save transient objects to backing storage by using ContextSave and
>    map resulting physical handle to virtual handle if there is such.
> 
> This commit does not implement virtualization support for hmac and
> policy sessions.
> 
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

For patches 1-4 you can add 

Reviewed-by: James Bottomley <James.Bottomley@HansenPartnership.com>

Just re-running a build with the latest kernel for my laptop to add
tested by.

James

Re: [tpmdd-devel] [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[James Bottomley]

On Fri, 2017-02-24 at 12:29 +0530, Nayna wrote:
> 
> On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
> > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > 
> > Currently the tpm spaces are 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
> > space.
> > 
> > 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.
> 
> To understand, I have two questions:
> 
> 1. How would a userspace application using TPM know whether to use 
> /dev/tpm0 or /dev/tpms0 ?

Likely they can't use /dev/tpm0 becuase it will be root only, but the
major indicator will be whether /dev/tpms0 exists or not.

> 2. How would a userspace RM know to build on top of /dev/tpm0 or 
> /dev/tpms0. And if it is built on top of /dev/tpms0, can there be 
> issues with one RM on top of other RM.

There's a known problem with RMs in that they're not fully stackable,
so I suspect the answer is that if tpms0 exists you won't use an RM,
but this is currently an area of active research.  The other potential
problem is that if you build a RM on tpm0 in userspace, it will fight
with the kernel when the kernel uses sessions.

James

> Thanks & Regards,
>     - Nayna
> 
> 
> > 
> > Signed-off-by: James Bottomley <
> > James.Bottomley@HansenPartnership.com>
> > ---
> >   drivers/char/tpm/Makefile        |  3 +-
> >   drivers/char/tpm/tpm-chip.c      | 73
> > ++++++++++++++++++++++++++++++++++++++--
> >   drivers/char/tpm/tpm-interface.c | 13 +++++--
> >   drivers/char/tpm/tpm.h           |  4 +++
> >   drivers/char/tpm/tpms-dev.c      | 65
> > +++++++++++++++++++++++++++++++++++
> >   5 files changed, 152 insertions(+), 6 deletions(-)
> >   create mode 100644 drivers/char/tpm/tpms-dev.c
> > 
> > diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
> > index 10e5827..bbe6531 100644
> > --- a/drivers/char/tpm/Makefile
> > +++ b/drivers/char/tpm/Makefile
> > @@ -3,7 +3,8 @@
> >   #
> >   obj-$(CONFIG_TCG_TPM) += tpm.o
> >   tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2
> > -cmd.o \
> > -	 tpm-dev-common.o tpm1_eventlog.o tpm2_eventlog.o tpm2
> > -space.o
> > +	 tpm-dev-common.o tpms-dev.o tpm1_eventlog.o
> > tpm2_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 993b9ae..c71c353 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 *tpms_class;
> >   dev_t tpm_devt;
> > 
> >   /**
> > @@ -132,6 +133,14 @@ static void tpm_dev_release(struct device
> > *dev)
> >   	kfree(chip);
> >   }
> > 
> > +static void tpm_devs_release(struct device *dev)
> > +{
> > +	struct tpm_chip *chip = container_of(dev, struct tpm_chip,
> > devs);
> > +
> > +	/* release the master device reference */
> > +	put_device(&chip->dev);
> > +}
> > +
> >   /**
> >    * tpm_chip_alloc() - allocate a new struct tpm_chip instance
> >    * @pdev: device to which the chip is associated
> > @@ -168,27 +177,47 @@ struct tpm_chip *tpm_chip_alloc(struct device
> > *pdev,
> >   	chip->dev_num = rc;
> > 
> >   	device_initialize(&chip->dev);
> > +	device_initialize(&chip->devs);
> > 
> >   	chip->dev.class = tpm_class;
> >   	chip->dev.release = tpm_dev_release;
> >   	chip->dev.parent = pdev;
> >   	chip->dev.groups = chip->groups;
> > 
> > +	chip->devs.parent = pdev;
> > +	chip->devs.class = tpms_class;
> > +	chip->devs.release = tpm_devs_release;
> > +	/* get extra reference on main device to hold on
> > +	 * behalf of devs.  This holds the chip structure
> > +	 * while cdevs is in use.  The corresponding put
> > +	 * is in the tpm_devs_release
> > +	 */
> > +	get_device(&chip->dev);
> > +
> >   	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->devs.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->devs, "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->cdevs, &tpms_fops);
> >   	chip->cdev.owner = THIS_MODULE;
> > +	chip->cdevs.owner = THIS_MODULE;
> >   	chip->cdev.kobj.parent = &chip->dev.kobj;
> > +	chip->cdevs.kobj.parent = &chip->devs.kobj;
> > 
> >   	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
> > GFP_KERNEL);
> >   	if (!chip->work_space.context_buf) {
> > @@ -199,6 +228,7 @@ struct tpm_chip *tpm_chip_alloc(struct device
> > *pdev,
> >   	return chip;
> > 
> >   out:
> > +	put_device(&chip->devs);
> >   	put_device(&chip->dev);
> >   	return ERR_PTR(rc);
> >   }
> > @@ -244,7 +274,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 +284,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->cdevs, chip->devs.devt, 1);
> > +	if (rc) {
> > +		dev_err(&chip->dev,
> > +			"unable to cdev_add() %s, major %d, minor
> > %d, err=%d\n",
> > +			dev_name(&chip->devs), MAJOR(chip
> > ->devs.devt),
> > +			MINOR(chip->devs.devt), rc);
> > +
> > +		goto err_3;
> >   	}
> > 
> > +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
> > +		rc = device_add(&chip->devs);
> > +	if (rc) {
> > +		dev_err(&chip->dev,
> > +			"unable to device_register() %s, major %d,
> > minor %d, err=%d\n",
> > +			dev_name(&chip->devs), MAJOR(chip
> > ->devs.devt),
> > +			MINOR(chip->devs.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->cdevs);
> > + 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 +329,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->cdevs);
> > +		device_del(&chip->devs);
> > +	}
> > +
> >   	/* Make the chip unavailable. */
> >   	mutex_lock(&idr_lock);
> >   	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
> > @@ -282,6 +345,10 @@ static void tpm_del_char_device(struct
> > tpm_chip *chip)
> >   		tpm2_shutdown(chip, TPM2_SU_CLEAR);
> >   	chip->ops = NULL;
> >   	up_write(&chip->ops_sem);
> > +	/* will release the devs reference to the chip->dev unless
> > +	 * something has cdevs open
> > +	 */
> > +	put_device(&chip->devs);
> >   }
> > 
> >   static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> > diff --git a/drivers/char/tpm/tpm-interface.c
> > b/drivers/char/tpm/tpm-interface.c
> > index db5ffe9..deb2021 100644
> > --- a/drivers/char/tpm/tpm-interface.c
> > +++ b/drivers/char/tpm/tpm-interface.c
> > @@ -1257,9 +1257,17 @@ static int __init tpm_init(void)
> >   		return PTR_ERR(tpm_class);
> >   	}
> > 
> > -	rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES,
> > "tpm");
> > +	tpms_class = class_create(THIS_MODULE, "tpms");
> > +	if (IS_ERR(tpms_class)) {
> > +		pr_err("couldn't create tpms class\n");
> > +		class_destroy(tpm_class);
> > +		return PTR_ERR(tpms_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(tpms_class);
> >   		class_destroy(tpm_class);
> >   		return rc;
> >   	}
> > @@ -1271,7 +1279,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(tpms_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 97e48a4..822ca67 100644
> > --- a/drivers/char/tpm/tpm.h
> > +++ b/drivers/char/tpm/tpm.h
> > @@ -182,7 +182,9 @@ struct tpm_chip_seqops {
> > 
> >   struct tpm_chip {
> >   	struct device dev;
> > +	struct device devs;
> >   	struct cdev cdev;
> > +	struct cdev cdevs;
> > 
> >   	/* 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 +512,10 @@ static inline void tpm_buf_append_u32(struct
> > tpm_buf *buf, const u32 value)
> >   }
> > 
> >   extern struct class *tpm_class;
> > +extern struct class *tpms_class;
> >   extern dev_t tpm_devt;
> >   extern const struct file_operations tpm_fops;
> > +extern const struct file_operations tpms_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..5720885
> > --- /dev/null
> > +++ b/drivers/char/tpm/tpms-dev.c
> > @@ -0,0 +1,65 @@
> > +/*
> > + * 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;
> > +	int rc;
> > +
> > +	chip = container_of(inode->i_cdev, struct tpm_chip,
> > cdevs);
> > +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> > +	if (priv == NULL)
> > +		return -ENOMEM;
> > +
> > +	rc = tpm2_init_space(&priv->space);
> > +	if (rc) {
> > +		kfree(priv);
> > +		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);
> > +	tpm2_del_space(&priv->space);
> > +	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 tpms_fops = {
> > +	.owner = THIS_MODULE,
> > +	.llseek = no_llseek,
> > +	.open = tpms_open,
> > +	.read = tpm_common_read,
> > +	.write = tpms_write,
> > +	.release = tpms_release,
> > +};
> > +
> > 
> 

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[James Bottomley]

On Thu, 2017-02-23 at 11:09 +0200, Jarkko Sakkinen wrote:
> On Thu, Feb 16, 2017 at 09:25:19PM +0200, Jarkko Sakkinen wrote:
> > From: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
> > 
> > Currently the tpm spaces are 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
> > space.
> > 
> > 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-d9PhHud1Jfhevp4ocFNHEFaTQe2KTcn/@public.gmane.org>

> Reviewed-by: Jarkko Sakkinen <jarkko.sakkine-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Tested-by: Jarkko Sakkinen <jarkko.sakkine-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Thanks!

> Nitpicking but I've been thinking about naming. What about calling 
> the device as tpmrc0 as in resource context. I think that would be a
> better name than TPM space.

Well the original name was tpmrm<n> for TPM with Resource Manager.  You
wanted it to be tpms<n> for TPM with Spaces.

I'm not entirely sold on the Resource Context name ... I think Resource Manager (because it's what the TCG calls it) or Spaces (because it's what all the code comments call it) are better.  Resource Context sounds like what TPM2_SaveContext() creates for you rather than the interface.

>  You do not mix it up with namespaces and/or virtualization. With
> resource in front it cannot be easily mixed up with TPM contexts
> either.

I'm a containers person.  What this set of patches does is precisely OS
level virtualization in my book, so I don't think you need to pretend
it is't; and OS level virtualization is what a namespace does.  The
only difference between this and the other kernel namespaces is that
you get a new namespace automatically when you open the device and you
can't enter an existing namespace.

I think therefore that tpmns<n> for TPM Namespace would be very
appropriate.

> This does not require any effort from your side. I could do the
> renaming.
>
> PS. Could you go through my commits and test and review them at some
> point so we would have the whole patch set peer tested?

Already reviewed, just doing a test build (I'm travelling, so it
actually has to be on my physical laptop).

James



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Jarkko Sakkinen]

On Fri, Feb 24, 2017 at 07:53:22AM -0500, James Bottomley wrote:
> On Thu, 2017-02-16 at 21:25 +0200, Jarkko Sakkinen wrote:
> > Added an ability to virtualize TPM commands into an isolated context
> > that we call a TPM space because the word context is already heavily
> > used in the TPM specification. Both the handle areas and bodies
> > (where
> > necessary) are virtualized.
> > 
> > The mechanism works by adding a new parameter struct tpm_space to the
> > tpm_transmit() function. This new structure contains the list of
> > virtual
> > handles and a buffer of page size (currently) for backing storage.
> > 
> > When tpm_transmit() is called with a struct tpm_space instance it
> > will
> > execute the following sequence:
> > 
> > 1. Take locks.
> > 2. Load transient objects from the backing storage by using
> > ContextLoad
> >    and map virtual handles to physical handles.
> > 3. Perform the transaction.
> > 4. Save transient objects to backing storage by using ContextSave and
> >    map resulting physical handle to virtual handle if there is such.
> > 
> > This commit does not implement virtualization support for hmac and
> > policy sessions.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> For patches 1-4 you can add 
> 
> Reviewed-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> 
> Just re-running a build with the latest kernel for my laptop to add
> tested by.
> 
> James

Thank you!

/Jarkko

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jarkko Sakkinen]

On Fri, Feb 24, 2017 at 08:02:08AM -0500, James Bottomley wrote:
> On Thu, 2017-02-23 at 11:09 +0200, Jarkko Sakkinen wrote:
> > On Thu, Feb 16, 2017 at 09:25:19PM +0200, Jarkko Sakkinen wrote:
> > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > 
> > > Currently the tpm spaces are 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
> > > space.
> > > 
> > > 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@HansenPartnershp.com>
> 
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
> > Tested-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
> 
> Thanks!
> 
> > Nitpicking but I've been thinking about naming. What about calling 
> > the device as tpmrc0 as in resource context. I think that would be a
> > better name than TPM space.
> 
> Well the original name was tpmrm<n> for TPM with Resource Manager.  You
> wanted it to be tpms<n> for TPM with Spaces.
> 
> I'm not entirely sold on the Resource Context name ... I think Resource Manager (because it's what the TCG calls it) or Spaces (because it's what all the code comments call it) are better.  Resource Context sounds like what TPM2_SaveContext() creates for you rather than the interface.
> 
> >  You do not mix it up with namespaces and/or virtualization. With
> > resource in front it cannot be easily mixed up with TPM contexts
> > either.
> 
> I'm a containers person.  What this set of patches does is precisely OS
> level virtualization in my book, so I don't think you need to pretend
> it is't; and OS level virtualization is what a namespace does.  The
> only difference between this and the other kernel namespaces is that
> you get a new namespace automatically when you open the device and you
> can't enter an existing namespace.
> 
> I think therefore that tpmns<n> for TPM Namespace would be very
> appropriate.

Makes sense. We can go with tpmns.

/Jarkko

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jason Gunthorpe]

On Fri, Feb 24, 2017 at 07:39:22PM +0200, Jarkko Sakkinen wrote:

> > I think therefore that tpmns<n> for TPM Namespace would be very
> > appropriate.
> 
> Makes sense. We can go with tpmns.

When we have talked about TPM namespaces in the past it has been
around the idea of restricting which TPMs the namespace has access too
and changing the 'kernel tpm' for that namespace.

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[James Bottomley]

On Fri, 2017-02-24 at 11:11 -0700, Jason Gunthorpe wrote:
> On Fri, Feb 24, 2017 at 07:39:22PM +0200, Jarkko Sakkinen wrote:
> 
> > > I think therefore that tpmns<n> for TPM Namespace would be very
> > > appropriate.
> > 
> > Makes sense. We can go with tpmns.
> 
> When we have talked about TPM namespaces in the past it has been
> around the idea of restricting which TPMs the namespace has access 
> too and changing the 'kernel tpm' for that namespace.

Well, you know, nothing in the TPM Space code prevents us from exposing
the namespace so that it could be shared.  However, I think the
namespace follows connect (device open) paradigm is pretty much the
behaviour everyone (including the kernel) wants, mostly because TPM2
has such a tiny amount of resources that you're always dealing with
loadable keys meaning you don't really want to see anyone else's
volatile state.

James

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jason Gunthorpe]

On Fri, Feb 24, 2017 at 03:29:15PM -0500, James Bottomley wrote:
> On Fri, 2017-02-24 at 11:11 -0700, Jason Gunthorpe wrote:
> > On Fri, Feb 24, 2017 at 07:39:22PM +0200, Jarkko Sakkinen wrote:
> > 
> > > > I think therefore that tpmns<n> for TPM Namespace would be very
> > > > appropriate.
> > > 
> > > Makes sense. We can go with tpmns.
> > 
> > When we have talked about TPM namespaces in the past it has been
> > around the idea of restricting which TPMs the namespace has access 
> > too and changing the 'kernel tpm' for that namespace.
> 
> Well, you know, nothing in the TPM Space code prevents us from exposing
> the namespace so that it could be shared.  However, I think the
> namespace follows connect (device open) paradigm is pretty much the
> behaviour everyone (including the kernel) wants, mostly because TPM2
> has such a tiny amount of resources that you're always dealing with
> loadable keys meaning you don't really want to see anyone else's
> volatile state.

I'm not arguing with that use model, I am asking what do you want to
call the future feature that restricts which TPMs a process can view
if you want to use the word namespace for the resource manager?

This is something Stephen B has been exploring in conjunction with
vtpm. (eg restrict a container to only use a single vtpm and ban it
from the system tpm)

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [tpmdd-devel] [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[James Bottomley]

On Fri, 2017-02-24 at 13:52 -0700, Jason Gunthorpe wrote:
> On Fri, Feb 24, 2017 at 03:29:15PM -0500, James Bottomley wrote:
> > On Fri, 2017-02-24 at 11:11 -0700, Jason Gunthorpe wrote:
> > > On Fri, Feb 24, 2017 at 07:39:22PM +0200, Jarkko Sakkinen wrote:
> > > 
> > > > > I think therefore that tpmns<n> for TPM Namespace would be
> > > > > very
> > > > > appropriate.
> > > > 
> > > > Makes sense. We can go with tpmns.
> > > 
> > > When we have talked about TPM namespaces in the past it has been
> > > around the idea of restricting which TPMs the namespace has
> > > access 
> > > too and changing the 'kernel tpm' for that namespace.
> > 
> > Well, you know, nothing in the TPM Space code prevents us from
> > exposing
> > the namespace so that it could be shared.  However, I think the
> > namespace follows connect (device open) paradigm is pretty much the
> > behaviour everyone (including the kernel) wants, mostly because
> > TPM2
> > has such a tiny amount of resources that you're always dealing with
> > loadable keys meaning you don't really want to see anyone else's
> > volatile state.
> 
> I'm not arguing with that use model, I am asking what do you want to
> call the future feature that restricts which TPMs a process can view
> if you want to use the word namespace for the resource manager?

Well, as a glib answer, I'd say the TPM is a device, so the thing which
restricts device access to containers is the device cgroup ... that's
what we should be plugging into.  I'd have to look, but I suspect the
device cgroup basically operates on device node appearance, so it
should "just work"(tm).  I can explore when I'm back home.

James

> This is something Stephen B has been exploring in conjunction with
> vtpm. (eg restrict a container to only use a single vtpm and ban it
> from the system tpm)
> 
> Jason
> 
> ---------------------------------------------------------------------
> ---------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
> 

Re: [tpmdd-devel] [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jason Gunthorpe]

On Fri, Feb 24, 2017 at 06:01:00PM -0500, James Bottomley wrote:

> Well, as a glib answer, I'd say the TPM is a device, so the thing which
> restricts device access to containers is the device cgroup ... that's
> what we should be plugging into.  I'd have to look, but I suspect the
> device cgroup basically operates on device node appearance, so it
> should "just work"(tm).  I can explore when I'm back home.

Seems reasonable..

It just seems confusing to call something a namespace that isn't also
a CLONE_NEW* option..

FWIW more background on the topic:

Stefan was concerned about information leakage via sysfs of TPM data,
eg that a container could still touch the host's TPM. I wonder if
device cgroup could be extended to block access to the sysfs
directories containing a disallowed 'dev' ?

I was also wondering about kernel use from within the container -
all kernel consumers are locked to physical tpm0.. But maybe the
kernel can consult the right device cgroup to find an allowed TPM?

Jason

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[James Bottomley]

On Fri, 2017-02-24 at 16:23 -0700, Jason Gunthorpe wrote:
> On Fri, Feb 24, 2017 at 06:01:00PM -0500, James Bottomley wrote:
> 
> > Well, as a glib answer, I'd say the TPM is a device, so the thing 
> > which restricts device access to containers is the device cgroup 
> > ... that's what we should be plugging into.  I'd have to look, but 
> > I suspect the device cgroup basically operates on device node 
> > appearance, so it should "just work"(tm).  I can explore when I'm
> > back home.
> 
> Seems reasonable..
> 
> It just seems confusing to call something a namespace that isn't also
> a CLONE_NEW* option..

Well, there's namespace behaviour and then there's how you enter them. 
 We have namespace behaviour with the /dev/tpms<n> but the namespace is
entered on opening the device, even if the same process opens the
device more than once.  So we have namespace behaviour with a non clone
entry mechanism.  Since we're namespaceing a device, that seems to me
to be the correct semantic.

> FWIW more background on the topic:
> 
> Stefan was concerned about information leakage via sysfs of TPM data,
> eg that a container could still touch the host's TPM. I wonder if
> device cgroup could be extended to block access to the sysfs
> directories containing a disallowed 'dev' ?

It doesn't need to.  The sysfs entries (those that ask the TPM
something) are surrounded by chip->tpm_mutex, so when it asks, we know
all the spaces are context saved (i.e. the only TPM visible state is
global not anything space local).

> I was also wondering about kernel use from within the container -
> all kernel consumers are locked to physical tpm0.. But maybe the
> kernel can consult the right device cgroup to find an allowed TPM?

I'd use the device cgroup to determine what's allowable per container
(i.e. what tpm you can see) then within the container I'd open the
tpms<n> device ... because the TPM volatile storage is so tiny its not
inconceivable that multiple processes, even within a single container,
need access ... and they'd each need their own "namespace" (which they
get with the current model).  However, this is opinion ... we should
try it out and see what works best.

James



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [tpmdd-devel] [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jason Gunthorpe]

On Fri, Feb 24, 2017 at 06:43:27PM -0500, James Bottomley wrote:

> > It just seems confusing to call something a namespace that isn't also
> > a CLONE_NEW* option..
> 
> Well, there's namespace behaviour and then there's how you enter them. 
>  We have namespace behaviour with the /dev/tpms<n> but the namespace is
> entered on opening the device, even if the same process opens the
> device more than once.  So we have namespace behaviour with a non clone
> entry mechanism.  Since we're namespaceing a device, that seems to me
> to be the correct semantic.

I'm looking at it from a documentation perspective, look at
namespaces(7) for instance

Lots of FD things have 'namespace behavior' but we don't call
them namespaces..

> > Stefan was concerned about information leakage via sysfs of TPM data,
> > eg that a container could still touch the host's TPM. I wonder if
> > device cgroup could be extended to block access to the sysfs
> > directories containing a disallowed 'dev' ?
> 
> It doesn't need to.  The sysfs entries (those that ask the TPM
> something) are surrounded by chip->tpm_mutex, so when it asks, we know
> all the spaces are context saved (i.e. the only TPM visible state is
> global not anything space local).

Yes, I understand that - the concern is that a container can still
read the global state from tpm0 (eg ek/srk/pcrs) even if it is setup to
exclusively use a vtpm. device cgroup blocks access to the cdevs of
tpm0 but not to the sysfs files.

Maybe we should just make those debug files readable only by root and
forget about that worry.

> > I was also wondering about kernel use from within the container -
> > all kernel consumers are locked to physical tpm0.. But maybe the
> > kernel can consult the right device cgroup to find an allowed TPM?
> 
> I'd use the device cgroup to determine what's allowable per container
> (i.e. what tpm you can see) then within the container I'd open the
> tpms<n> device ...

I am talking about using a situation like kernel IMA or keyring in the
container with a tpm that is not tpm0, eg a vtpm.

Jason

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[James Bottomley]

On Fri, 2017-02-24 at 17:25 -0700, Jason Gunthorpe wrote:
> On Fri, Feb 24, 2017 at 06:43:27PM -0500, James Bottomley wrote:
> 
> > > It just seems confusing to call something a namespace that isn't 
> > > also a CLONE_NEW* option..
> > 
> > Well, there's namespace behaviour and then there's how you enter 
> > them.  We have namespace behaviour with the /dev/tpms<n> but the
> > namespace is entered on opening the device, even if the same 
> > process opens the device more than once.  So we have namespace 
> > behaviour with a non clone entry mechanism.  Since we're 
> > namespaceing a device, that seems to me to be the correct semantic.
> 
> I'm looking at it from a documentation perspective, look at
> namespaces(7) for instance

The term "namespace" is way broader than that

https://en.wikipedia.org/wiki/Namespace

> Lots of FD things have 'namespace behavior' but we don't call
> them namespaces..

At it's simplest, the virtual memory process model of UNIX is a
namespace.  That doesn't make the term inapplicable in this case.

> > > Stefan was concerned about information leakage via sysfs of TPM
> > > data, eg that a container could still touch the host's TPM. I
> > > wonder if device cgroup could be extended to block access to the
> > > sysfs directories containing a disallowed 'dev' ?
> > 
> > It doesn't need to.  The sysfs entries (those that ask the TPM
> > something) are surrounded by chip->tpm_mutex, so when it asks, we 
> > know all the spaces are context saved (i.e. the only TPM visible 
> > state is global not anything space local).
> 
> Yes, I understand that - the concern is that a container can still
> read the global state from tpm0 (eg ek/srk/pcrs) even if it is setup 
> to exclusively use a vtpm.

The TPM2 namespace as laid out by these patches only applies to objects
of type 80, 02 and 03.  The Storage and Endorsement seeds can't be
virtualized because they're one per physical instance.  81 objects
could be virtualised, but I don't really think we should. PCR
virtualisation is another whole interesting area of study.

>  device cgroup blocks access to the cdevs of tpm0 but not to the
> sysfs files.

What the device cgroup currently does for us and what it could do are
two different things.  It seems if it exported
__devcgroup_check_permission, we could use that as a check to gate the
sysfs file access.

> Maybe we should just make those debug files readable only by root and
> forget about that worry.
> 
> > > I was also wondering about kernel use from within the container -
> > > all kernel consumers are locked to physical tpm0.. But maybe the
> > > kernel can consult the right device cgroup to find an allowed
> > > TPM?
> > 
> > I'd use the device cgroup to determine what's allowable per 
> > container (i.e. what tpm you can see) then within the container I'd 
> > open the tpms<n> device ...
> 
> I am talking about using a situation like kernel IMA or keyring in 
> the container with a tpm that is not tpm0, eg a vtpm.

a vtpm appears as a tpm device so it can be controlled by the device
cgroup ... I think I'm not seeing the issue.

I should also say that discussion of mechanisms is usually the wrong
way to begin for OS virtualisation.  Almost anything can be virtualised
in a variety of ways, so to find the best way (or indeed if it should
be done at all) it's usually better to start with use cases.  So
instead of saying we need to virtualize the PCRs we should start with X
container has this requirement for attestation of its Y state.  Often
the best way simply is an extension of the multi user model for the
resource ... in this case no-one's really come up with one for PCRs, so
that might be the place to begin.

James


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jarkko Sakkinen]

On Fri, Feb 24, 2017 at 08:02:08AM -0500, James Bottomley wrote:
> On Thu, 2017-02-23 at 11:09 +0200, Jarkko Sakkinen wrote:
> > On Thu, Feb 16, 2017 at 09:25:19PM +0200, Jarkko Sakkinen wrote:
> > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > 
> > > Currently the tpm spaces are 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
> > > space.
> > > 
> > > 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@HansenPartnershp.com>
> 
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
> > Tested-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
> 
> Thanks!
> 
> > Nitpicking but I've been thinking about naming. What about calling 
> > the device as tpmrc0 as in resource context. I think that would be a
> > better name than TPM space.
> 
> Well the original name was tpmrm<n> for TPM with Resource Manager.  You
> wanted it to be tpms<n> for TPM with Spaces.
> 
> I'm not entirely sold on the Resource Context name ... I think
> Resource Manager (because it's what the TCG calls it) or Spaces
> (because it's what all the code comments call it) are better.
> Resource Context sounds like what TPM2_SaveContext() creates for you
> rather than the interface.
> 
> >  You do not mix it up with namespaces and/or virtualization. With
> > resource in front it cannot be easily mixed up with TPM contexts
> > either.
> 
> I'm a containers person.  What this set of patches does is precisely OS
> level virtualization in my book, so I don't think you need to pretend
> it is't; and OS level virtualization is what a namespace does.  The
> only difference between this and the other kernel namespaces is that
> you get a new namespace automatically when you open the device and you
> can't enter an existing namespace.
> 
> I think therefore that tpmns<n> for TPM Namespace would be very
> appropriate.

Sorry for going back and forth with this but I turn it back to your
original tpmrm. It's in the end of the day the least confusing option. I
think this has been anyway useful to trip a bit around the options
because it is hard to rollback API...

> > This does not equire any effort from your side. I could do the
> > renaming.
> >
> > PS. Could you go through my commits and test and review them at some
> > point so we would have the whole patch set peer tested?
> 
> Already reviewed, just doing a test build (I'm travelling, so it
> actually has to be on my physical laptop).
> 
> James

There's now tabrm-v3 branch. I had to tweak error handling in your
device adding patch because of b4e9d7561a70. I hope I didn't break
anything.

/Jarkko

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Dr. Greg Wettstein]

On Sun, Feb 26, 2017 at 01:44:40PM +0200, Jarkko Sakkinen wrote:

Good day, I hope this note finds the week starting well for everyone.

> On Fri, Feb 24, 2017 at 08:02:08AM -0500, James Bottomley wrote:
> > On Thu, 2017-02-23 at 11:09 +0200, Jarkko Sakkinen wrote:
> > > On Thu, Feb 16, 2017 at 09:25:19PM +0200, Jarkko Sakkinen wrote:
> > > > From: James Bottomley <James.Bottomley@HansenPartnership.com>
> > > > 
> > > > Currently the tpm spaces are 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
> > > > space.
> > > > 
> > > > 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@HansenPartnershp.com>
> > 
> > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
> > > Tested-by: Jarkko Sakkinen <jarkko.sakkine@linux.intel.com>
> > 
> > Thanks!
> > 
> > > Nitpicking but I've been thinking about naming. What about calling 
> > > the device as tpmrc0 as in resource context. I think that would be a
> > > better name than TPM space.
> > 
> > Well the original name was tpmrm<n> for TPM with Resource Manager.  You
> > wanted it to be tpms<n> for TPM with Spaces.
> > 
> > I'm not entirely sold on the Resource Context name ... I think
> > Resource Manager (because it's what the TCG calls it) or Spaces
> > (because it's what all the code comments call it) are better.
> > Resource Context sounds like what TPM2_SaveContext() creates for you
> > rather than the interface.
> > 
> > >  You do not mix it up with namespaces and/or virtualization. With
> > > resource in front it cannot be easily mixed up with TPM contexts
> > > either.
> > 
> > I'm a containers person.  What this set of patches does is precisely OS
> > level virtualization in my book, so I don't think you need to pretend
> > it is't; and OS level virtualization is what a namespace does.  The
> > only difference between this and the other kernel namespaces is that
> > you get a new namespace automatically when you open the device and you
> > can't enter an existing namespace.
> > 
> > I think therefore that tpmns<n> for TPM Namespace would be very
> > appropriate.

> Sorry for going back and forth with this but I turn it back to your
> original tpmrm. It's in the end of the day the least confusing
> option. I think this has been anyway useful to trip a bit around the
> options because it is hard to rollback API...

Indeed, which is why we have watched this conversation with some
interest regarding exactly what the tpm{rm,ns} will represent.

In order to get a handle on how the resource manager will affect
Trusted Execution Technology (TXT) environments we downloaded your
branch and experimented with it a bit.  Our overall impression is that
it will be important to educate the user community on exactly what the
'tpm spaces' management device actually represents.

It is not a TPM device as much as it is a method of surfacing a subset
of TPM functionality over the lifespan of a file descriptor.  There is
certainly nothing wrong with this but users and system administrators
will need to understand the implications of this and not confuse the
two devices.

For example, Ken's tools which come in his TSS2 library, don't work
properly with the 'spaces' device due to the virtualization lifetime.
As an example, the getcapability call will 'lie' about the number of
transient handles which are available through the device.  Attempts to
string multiple transaction sequences together will fail as well.

So I think it will be important to stress that the 'spaces' device is
something an individual application will use to gain unimpeded access
to TPM functionality and not a representation of the device itself.
Given the quality of userspace simulators and their flexibility and
operational fidelity, I anticipate we will eventually come to the
conclusion that this problem will have been best solved by creating
and adopting a framework which provides the anchoring and spawning of
userspace TPM2 instances.

Just as an aside we still haven't been able to verify the
inter-operability of the 'spaces' driver with TXT.  There are larger
TXT/TPM2 operational issues which we are working to run down before we
can determine how the two systems will inter-operate or not.

> /Jarkko

Have a good day.

Greg

As always,
Dr. G.W. Wettstein, Ph.D.   Enjellic Systems Development, LLC.
4206 N. 19th Ave.           Specializing in information infra-structure
Fargo, ND  58102            development.
PH: 701-281-1686
FAX: 701-281-3949           EMAIL: greg@enjellic.com
------------------------------------------------------------------------------
"You've got to be kidding me Nate.  You've seen the shit that has come
 through my office in the last two hours.  You think I'm even remotely
 worried about one SATA cable being six inches longer than the other."
                                -- Dr. Greg Wettstein
                                   Resurrection

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Nayna]

On 02/24/2017 06:23 PM, James Bottomley wrote:
> On Fri, 2017-02-24 at 12:29 +0530, Nayna wrote:
>>
>> On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
>>> From: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
>>>
>>> Currently the tpm spaces are 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
>>> space.
>>>
>>> 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.
>>
>> To understand, I have two questions:
>>
>> 1. How would a userspace application using TPM know whether to use
>> /dev/tpm0 or /dev/tpms0 ?
>
> Likely they can't use /dev/tpm0 becuase it will be root only, but the
> major indicator will be whether /dev/tpms0 exists or not.

Thanks James !!
Currently, I see even /dev/tpms0 is also only root accessible. I did see 
the discussion to make it 0666, and I understand adding command 
filtering is part of enabling /dev/tpms0 as all-accessible.

Sorry, I didn't understand when you said, "major indicator will be 
whether /dev/tpms0" exists or not ? I mean in what case it might not 
exist..

>
>> 2. How would a userspace RM know to build on top of /dev/tpm0 or
>> /dev/tpms0. And if it is built on top of /dev/tpms0, can there be
>> issues with one RM on top of other RM.
>
> There's a known problem with RMs in that they're not fully stackable,
> so I suspect the answer is that if tpms0 exists you won't use an RM,
> but this is currently an area of active research.  The other potential
> problem is that if you build a RM on tpm0 in userspace, it will fight
> with the kernel when the kernel uses sessions.

Does it imply that there should be restriction to disallow any RM 
specific commands from userspace on /dev/tpm0 ?

Thanks & Regards,
- Nayna

>
> James
>
>> Thanks & Regards,
>>      - Nayna
>>
>>
>>>
>>> Signed-off-by: James Bottomley <
>>> James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
>>> ---
>>>    drivers/char/tpm/Makefile        |  3 +-
>>>    drivers/char/tpm/tpm-chip.c      | 73
>>> ++++++++++++++++++++++++++++++++++++++--
>>>    drivers/char/tpm/tpm-interface.c | 13 +++++--
>>>    drivers/char/tpm/tpm.h           |  4 +++
>>>    drivers/char/tpm/tpms-dev.c      | 65
>>> +++++++++++++++++++++++++++++++++++
>>>    5 files changed, 152 insertions(+), 6 deletions(-)
>>>    create mode 100644 drivers/char/tpm/tpms-dev.c
>>>
>>> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
>>> index 10e5827..bbe6531 100644
>>> --- a/drivers/char/tpm/Makefile
>>> +++ b/drivers/char/tpm/Makefile
>>> @@ -3,7 +3,8 @@
>>>    #
>>>    obj-$(CONFIG_TCG_TPM) += tpm.o
>>>    tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2
>>> -cmd.o \
>>> -	 tpm-dev-common.o tpm1_eventlog.o tpm2_eventlog.o tpm2
>>> -space.o
>>> +	 tpm-dev-common.o tpms-dev.o tpm1_eventlog.o
>>> tpm2_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 993b9ae..c71c353 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 *tpms_class;
>>>    dev_t tpm_devt;
>>>
>>>    /**
>>> @@ -132,6 +133,14 @@ static void tpm_dev_release(struct device
>>> *dev)
>>>    	kfree(chip);
>>>    }
>>>
>>> +static void tpm_devs_release(struct device *dev)
>>> +{
>>> +	struct tpm_chip *chip = container_of(dev, struct tpm_chip,
>>> devs);
>>> +
>>> +	/* release the master device reference */
>>> +	put_device(&chip->dev);
>>> +}
>>> +
>>>    /**
>>>     * tpm_chip_alloc() - allocate a new struct tpm_chip instance
>>>     * @pdev: device to which the chip is associated
>>> @@ -168,27 +177,47 @@ struct tpm_chip *tpm_chip_alloc(struct device
>>> *pdev,
>>>    	chip->dev_num = rc;
>>>
>>>    	device_initialize(&chip->dev);
>>> +	device_initialize(&chip->devs);
>>>
>>>    	chip->dev.class = tpm_class;
>>>    	chip->dev.release = tpm_dev_release;
>>>    	chip->dev.parent = pdev;
>>>    	chip->dev.groups = chip->groups;
>>>
>>> +	chip->devs.parent = pdev;
>>> +	chip->devs.class = tpms_class;
>>> +	chip->devs.release = tpm_devs_release;
>>> +	/* get extra reference on main device to hold on
>>> +	 * behalf of devs.  This holds the chip structure
>>> +	 * while cdevs is in use.  The corresponding put
>>> +	 * is in the tpm_devs_release
>>> +	 */
>>> +	get_device(&chip->dev);
>>> +
>>>    	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->devs.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->devs, "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->cdevs, &tpms_fops);
>>>    	chip->cdev.owner = THIS_MODULE;
>>> +	chip->cdevs.owner = THIS_MODULE;
>>>    	chip->cdev.kobj.parent = &chip->dev.kobj;
>>> +	chip->cdevs.kobj.parent = &chip->devs.kobj;
>>>
>>>    	chip->work_space.context_buf = kzalloc(PAGE_SIZE,
>>> GFP_KERNEL);
>>>    	if (!chip->work_space.context_buf) {
>>> @@ -199,6 +228,7 @@ struct tpm_chip *tpm_chip_alloc(struct device
>>> *pdev,
>>>    	return chip;
>>>
>>>    out:
>>> +	put_device(&chip->devs);
>>>    	put_device(&chip->dev);
>>>    	return ERR_PTR(rc);
>>>    }
>>> @@ -244,7 +274,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 +284,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->cdevs, chip->devs.devt, 1);
>>> +	if (rc) {
>>> +		dev_err(&chip->dev,
>>> +			"unable to cdev_add() %s, major %d, minor
>>> %d, err=%d\n",
>>> +			dev_name(&chip->devs), MAJOR(chip
>>> ->devs.devt),
>>> +			MINOR(chip->devs.devt), rc);
>>> +
>>> +		goto err_3;
>>>    	}
>>>
>>> +	if (chip->flags & TPM_CHIP_FLAG_TPM2)
>>> +		rc = device_add(&chip->devs);
>>> +	if (rc) {
>>> +		dev_err(&chip->dev,
>>> +			"unable to device_register() %s, major %d,
>>> minor %d, err=%d\n",
>>> +			dev_name(&chip->devs), MAJOR(chip
>>> ->devs.devt),
>>> +			MINOR(chip->devs.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->cdevs);
>>> + 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 +329,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->cdevs);
>>> +		device_del(&chip->devs);
>>> +	}
>>> +
>>>    	/* Make the chip unavailable. */
>>>    	mutex_lock(&idr_lock);
>>>    	idr_replace(&dev_nums_idr, NULL, chip->dev_num);
>>> @@ -282,6 +345,10 @@ static void tpm_del_char_device(struct
>>> tpm_chip *chip)
>>>    		tpm2_shutdown(chip, TPM2_SU_CLEAR);
>>>    	chip->ops = NULL;
>>>    	up_write(&chip->ops_sem);
>>> +	/* will release the devs reference to the chip->dev unless
>>> +	 * something has cdevs open
>>> +	 */
>>> +	put_device(&chip->devs);
>>>    }
>>>
>>>    static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
>>> diff --git a/drivers/char/tpm/tpm-interface.c
>>> b/drivers/char/tpm/tpm-interface.c
>>> index db5ffe9..deb2021 100644
>>> --- a/drivers/char/tpm/tpm-interface.c
>>> +++ b/drivers/char/tpm/tpm-interface.c
>>> @@ -1257,9 +1257,17 @@ static int __init tpm_init(void)
>>>    		return PTR_ERR(tpm_class);
>>>    	}
>>>
>>> -	rc = alloc_chrdev_region(&tpm_devt, 0, TPM_NUM_DEVICES,
>>> "tpm");
>>> +	tpms_class = class_create(THIS_MODULE, "tpms");
>>> +	if (IS_ERR(tpms_class)) {
>>> +		pr_err("couldn't create tpms class\n");
>>> +		class_destroy(tpm_class);
>>> +		return PTR_ERR(tpms_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(tpms_class);
>>>    		class_destroy(tpm_class);
>>>    		return rc;
>>>    	}
>>> @@ -1271,7 +1279,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(tpms_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 97e48a4..822ca67 100644
>>> --- a/drivers/char/tpm/tpm.h
>>> +++ b/drivers/char/tpm/tpm.h
>>> @@ -182,7 +182,9 @@ struct tpm_chip_seqops {
>>>
>>>    struct tpm_chip {
>>>    	struct device dev;
>>> +	struct device devs;
>>>    	struct cdev cdev;
>>> +	struct cdev cdevs;
>>>
>>>    	/* 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 +512,10 @@ static inline void tpm_buf_append_u32(struct
>>> tpm_buf *buf, const u32 value)
>>>    }
>>>
>>>    extern struct class *tpm_class;
>>> +extern struct class *tpms_class;
>>>    extern dev_t tpm_devt;
>>>    extern const struct file_operations tpm_fops;
>>> +extern const struct file_operations tpms_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..5720885
>>> --- /dev/null
>>> +++ b/drivers/char/tpm/tpms-dev.c
>>> @@ -0,0 +1,65 @@
>>> +/*
>>> + * Copyright (C) 2017 James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org
>>> + *
>>> + * 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;
>>> +	int rc;
>>> +
>>> +	chip = container_of(inode->i_cdev, struct tpm_chip,
>>> cdevs);
>>> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>>> +	if (priv == NULL)
>>> +		return -ENOMEM;
>>> +
>>> +	rc = tpm2_init_space(&priv->space);
>>> +	if (rc) {
>>> +		kfree(priv);
>>> +		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);
>>> +	tpm2_del_space(&priv->space);
>>> +	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 tpms_fops = {
>>> +	.owner = THIS_MODULE,
>>> +	.llseek = no_llseek,
>>> +	.open = tpms_open,
>>> +	.read = tpm_common_read,
>>> +	.write = tpms_write,
>>> +	.release = tpms_release,
>>> +};
>>> +
>>>
>>
>


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[James Bottomley]

On Mon, 2017-02-27 at 17:16 +0530, Nayna wrote:
> 
> On 02/24/2017 06:23 PM, James Bottomley wrote:
> > On Fri, 2017-02-24 at 12:29 +0530, Nayna wrote:
> > > 
> > > On 02/17/2017 12:55 AM, Jarkko Sakkinen wrote:
> > > > From: James Bottomley <James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk@public.gmane.org>
> > > > 
> > > > Currently the tpm spaces are 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 space.
> > > > 
> > > > 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.
> > > 
> > > To understand, I have two questions:
> > > 
> > > 1. How would a userspace application using TPM know whether to 
> > > use /dev/tpm0 or /dev/tpms0 ?
> > 
> > Likely they can't use /dev/tpm0 becuase it will be root only, but 
> > the major indicator will be whether /dev/tpms0 exists or not.
> 
> Thanks James !!
> Currently, I see even /dev/tpms0 is also only root accessible. I did 
> see the discussion to make it 0666, and I understand adding command
> filtering is part of enabling /dev/tpms0 as all-accessible.

I don't think we'd ever do that from the kernel.  Accessibility would
be a userspace policy.

This is what I have on my system to make it accessible:

/etc/udev/rules.d/80-tpm-2.rules:
# tpm 2 devices need to be world readable
SUBSYSTEM=="tpms", ACTION=="add", MODE="0666"

> Sorry, I didn't understand when you said, "major indicator will be 
> whether /dev/tpms0" exists or not ? I mean in what case it might not 
> exist..

If the kernel is too old or you have a 1.2 TPM.

> > 
> > > 2. How would a userspace RM know to build on top of /dev/tpm0 or
> > > /dev/tpms0. And if it is built on top of /dev/tpms0, can there be
> > > issues with one RM on top of other RM.
> > 
> > There's a known problem with RMs in that they're not fully 
> > stackable, so I suspect the answer is that if tpms0 exists you 
> > won't use an RM, but this is currently an area of active research. 
> >  The other potential problem is that if you build a RM on tpm0 in 
> > userspace, it will fight with the kernel when the kernel uses
> > sessions.
> 
> Does it imply that there should be restriction to disallow any RM 
> specific commands from userspace on /dev/tpm0 ?

The only such command would be session or policy context save.  It's
debateable whether we should interfere.

James



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jason Gunthorpe]

On Sat, Feb 25, 2017 at 12:04:49PM -0500, James Bottomley wrote:

> >  device cgroup blocks access to the cdevs of tpm0 but not to the
> > sysfs files.
> 
> What the device cgroup currently does for us and what it could do are
> two different things.  It seems if it exported
> __devcgroup_check_permission, we could use that as a check to gate the
> sysfs file access.

Make sense, maybe we should be doing that..

Stefan, are you still interested in this? This seems like a fairly
simple solution to your problem???

> > I am talking about using a situation like kernel IMA or keyring in 
> > the container with a tpm that is not tpm0, eg a vtpm.
> 
> a vtpm appears as a tpm device so it can be controlled by the device
> cgroup ... I think I'm not seeing the issue.

When an in-kernel call opens the TPM it does not go through the cdev,
it does something like this:

extern int tpm_pcr_read(u32 chip_num, int pcr_idx, u8 *res_buf);

And hardwires 'chip_num' to TPM_ANY_NUM. Keyring does the same (see
trusted_instantiate)

Practically speaking this means in-kernel callers pretty much always
operate on tpm0.

I think we need to change TPM_ANY_NUM to something more container
friendly, but I'm not sure what that should be.

> be done at all) it's usually better to start with use cases.  So
> instead of saying we need to virtualize the PCRs we should start with X
> container has this requirement for attestation of its Y state.  Often
> the best way simply is an extension of the multi user model for the
> resource ... in this case no-one's really come up with one for PCRs, so
> that might be the place to begin.

Broadly makes sense to me.

Maybe kernel keyring is a better example, it already has a multi-user
model.

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Jason Gunthorpe]

On Sun, Feb 26, 2017 at 01:44:40PM +0200, Jarkko Sakkinen wrote:
 
> There's now tabrm-v3 branch. I had to tweak error handling in your
> device adding patch because of b4e9d7561a70. I hope I didn't break
> anything.

Just looked, the flow seemed like it works to me. Just confusing that
tpm_add_char_device isn't undone by tpm_del_char_device

Jason

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 6/7] tpm: expose spaces via a device link /dev/tpms<n>

[Ken Goldman]

On 2/26/2017 1:30 PM, Dr. Greg Wettstein wrote:
>
> For example, Ken's tools which come in his TSS2 library, don't work
> properly with the 'spaces' device due to the virtualization lifetime.
> As an example, the getcapability call will 'lie' about the number of
> transient handles which are available through the device.  Attempts to
> string multiple transaction sequences together will fail as well.

Two comments:

1 = The intent of the command line tools was for rapid prototyping 
scripts against a SW TPM, and then as sample code for writing the 
application.

2 - If you really want to script against a hardware TPM, it can be done. 
  Simply place a proxy between the TSS and the TPM device driver.  The 
proxy passes commands from the TCP socket to the TPM device driver.  It 
keeps the connection open so the resource manager doesn't flush between 
transactions.

The proxy can be obtained from here.  It's from TPM 1.2 days, but it 
works for TPM 2.0 as well.

https://sourceforge.net/projects/ibmswtpm/files/?source=navbar


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Ken Goldman]

On 2/22/2017 12:39 PM, James Bottomley wrote:
>
> Right at the moment the kernel use of tpm2 looks like
>
> acquire chip->tpm_mutex
> load key
> process key
> unload key
> release chip->tpm_mutex
>
> While it does this, there's no need for it to have a RM interface
> because what it does between the acquisition and drop of the mutex
> can't be seen by or have any effect on userspace (whether it uses the
> RM or not).  So currently, the question doesn't arise, which is the
> situation you see.

1 - This appears to depend on the RM not releasing the mutex until all 
objects are swapped out.  Correct?  Same for sessions?

2 - A startauthsession can cause a regap error.  Does the above depend 
on the RM doing early regapping so the RM won't see that error?

3 - There's also the problem where the TPM saved session slots 
(typically 64) are full.  My intuition is that the best solution is for 
the RM to reserve 3 slots for the kernel.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [tpmdd-devel] [PATCH v2 4/7] tpm: infrastructure for TPM spaces

[Jarkko Sakkinen]

On Wed, Mar 22, 2017 at 04:09:21PM -0400, Ken Goldman wrote:
> On 2/22/2017 12:39 PM, James Bottomley wrote:
> > 
> > Right at the moment the kernel use of tpm2 looks like
> > 
> > acquire chip->tpm_mutex
> > load key
> > process key
> > unload key
> > release chip->tpm_mutex
> > 
> > While it does this, there's no need for it to have a RM interface
> > because what it does between the acquisition and drop of the mutex
> > can't be seen by or have any effect on userspace (whether it uses the
> > RM or not).  So currently, the question doesn't arise, which is the
> > situation you see.
> 
> 1 - This appears to depend on the RM not releasing the mutex until all
> objects are swapped out.  Correct?  Same for sessions?

Yes.

> 2 - A startauthsession can cause a regap error.  Does the above depend on
> the RM doing early regapping so the RM won't see that error?

We are not trying to resolve that for 4.12. It can happen.

> 3 - There's also the problem where the TPM saved session slots (typically
> 64) are full.  My intuition is that the best solution is for the RM to
> reserve 3 slots for the kernel.

Maybe but at the moment kernel does not use sessions.

/Jarkko