linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] tpm: securityfs support for TPM 2.0 firmware event log
@ 2016-11-23 17:27 Nayna Jain
  2016-11-23 17:27 ` [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c Nayna Jain
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Nayna Jain @ 2016-11-23 17:27 UTC (permalink / raw)
  To: tpmdd-devel
  Cc: peterhuewe, tpmdd, jarkko.sakkinen, jgunthorpe,
	linux-security-module, linux-kernel, Nayna Jain

The TPM device driver defines ascii and binary methods for
displaying the TPM 1.2 event log via securityfs files, which are
needed for validating a TPM quote. The device driver for TPM 2.0
does not have similar support for displaying the TPM 2.0
event log. This patch set adds the support for displaying
TPM 2.0 event log in binary format.

The parsing mechanism to display the TPM 2.0 event log in binary
format is implemented as defined in the TPM 2.0 TCG specification[1].
If the firmware event log support exists and is successfully read,
the securityfs file is created to provide the event log in binary
format for both the OF device tree and ACPI.

   - Patch 1 moves the common TPM 1.2 and 2.0 event log initialization
     functions to a new file named tpm_eventlog_init.c.
   - Patch 2 adds the device tree bindings support for Physical TPM.
   - Patch 3 adds the support for creating securityfs files and for
     displaying the TPM 2.0 crypto agile event log in binary format.

With that, this patch set splits the event log functionality into
initialization functions, TPM 1.2 event log parsing and TPM 2.0 event
log parsing.

[1] TCG EFI Protocol Specification, Family "2.0" - Section 5 "Event
Log Structure"

Changelog History

v5:

- Upstreamed cleanup and fixes as different patchset
- Rebased to the Jarkko's latest master branch (e5be084 tpm: vtpm_proxy:
  Do not access host's event log)
- Patch "tpm: enhance read_log_of() to support Physical TPM event log
  - New Patch.
- Patch "tpm: add securityfs support for TPM 2.0 firmware event log"
  - Moved the changes in read_log_of() to a different patch
  - TPM 2.0 event log data types are declared in tpm_eventlog.h, tpm2.h
  is removed.
  - Included other feedbacks also from Jarkko on aligment and extra
    line

v4:

- Includes feedbacks from Jarkko and Jason.
- Patch "tpm: define a generic open() method for ascii & bios
measurements".
  - Fix indentation issue.
- Patch "tpm: replace the dynamically allocated bios_dir as
  struct dentry array".
  - Continue to use bios_dir_count variable to use is_bad() checks and
    to maintain correct order for securityfs_remove() during teardown.
  - Reset chip->bios_dir_count in teardown() function.
- Patch "tpm: validate the eventlog access before tpm_bios_log_setup".
  - Retain TPM2 check which was removed in previous patch.
  - Add tpm_bios_log_setup failure handling.
  - Remove use of private data from v3 version of patch. Add a
  new member to struct tpm_chip to achieve the same purpose.
- Patch "tpm: redefine the read_log method to check for ACPI/OF 
properties sequentially".
  - Move replacement of CONFIG_TCG_IBMVTPM with CONFIG_OF to this
    patch from patch 3.
  - Replace -1 error code with -ENODEV.
- Patch "tpm: replace the of_find_node_by_name() with dev of_node
property".
  - Uses chip->dev.parent->of_node.
  - Created separate patch for cleanup of pr_err messages.
- Patch "tpm: remove printk error messages".
  - New Patch.
- Patch "tpm: add the securityfs file support for TPM 2.0 eventlog".
  - Parses event digests using event alg_id rather than event log header
    alg_id.
  - Uses of_property_match_string to differentiate tpm/vtpm compatible

v3:

- Includes the review feedbacks as suggested by Jason.
- Split of patches into one patch per idea.
- Generic open() method for ascii/bios measurements.
- Replacement of of **bios_dir with *bios_dir[3].
- Verifying readlog() is successful before creating securityfs entries.
- Generic readlog() to check for ACPI/OF in sequence.
- read_log_of() method now uses of_node propertry rather than
calling find_device_by_name.
- read_log differentiates vtpm/tpm using its compatible property.
- Cleans pr_err with dev_dbg.
- Commit msgs subject line prefixed with tpm.

v2:

- Fixes issues as given in feedback by Jason.
- Adds documentation for device tree.

Nayna Jain (3):
  tpm: move event log init functions to tpm_eventlog_init.c
  tpm: enhance read_log_of() to support Physical TPM event log
  tpm: add securityfs support for TPM 2.0 firmware event log

 drivers/char/tpm/Makefile            |   2 +-
 drivers/char/tpm/tpm2_eventlog.c     | 214 +++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_eventlog.c      | 165 +--------------------------
 drivers/char/tpm/tpm_eventlog.h      |  72 ++++++++++++
 drivers/char/tpm/tpm_eventlog_init.c | 194 +++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_of.c            |  27 ++++-
 6 files changed, 508 insertions(+), 166 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2_eventlog.c
 create mode 100644 drivers/char/tpm/tpm_eventlog_init.c

-- 
2.5.0

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

* [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c
  2016-11-23 17:27 [PATCH v5 0/3] tpm: securityfs support for TPM 2.0 firmware event log Nayna Jain
@ 2016-11-23 17:27 ` Nayna Jain
  2016-11-23 19:38   ` Jason Gunthorpe
  2016-11-23 17:27 ` [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log Nayna Jain
  2016-11-23 17:27 ` [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware " Nayna Jain
  2 siblings, 1 reply; 19+ messages in thread
From: Nayna Jain @ 2016-11-23 17:27 UTC (permalink / raw)
  To: tpmdd-devel
  Cc: peterhuewe, tpmdd, jarkko.sakkinen, jgunthorpe,
	linux-security-module, linux-kernel, Nayna Jain

The device driver code for the event log has the init functions and
TPM 1.2 parsing logic both defined in same file(tpm_eventlog.c).

Since the initialization functions are common with the TPM 2.0 event
log support, this patch splits tpm_eventlog.c to have only TPM 1.2
event log parsing logic and moves the init functions into
tpm_eventlog_init.c.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/Makefile            |   2 +-
 drivers/char/tpm/tpm_eventlog.c      | 165 +-----------------------------
 drivers/char/tpm/tpm_eventlog.h      |   3 +
 drivers/char/tpm/tpm_eventlog_init.c | 189 +++++++++++++++++++++++++++++++++++
 4 files changed, 197 insertions(+), 162 deletions(-)
 create mode 100644 drivers/char/tpm/tpm_eventlog_init.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index a05b1eb..1dc2671 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-		tpm_eventlog.o
+		tpm_eventlog.o tpm_eventlog_init.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_eventlog.c b/drivers/char/tpm/tpm_eventlog.c
index 2a15b86..86f7fe3 100644
--- a/drivers/char/tpm/tpm_eventlog.c
+++ b/drivers/char/tpm/tpm_eventlog.c
@@ -11,7 +11,8 @@
  *
  * Maintained by: <tpmdd-devel@lists.sourceforge.net>
  *
- * Access to the event log created by a system's firmware / BIOS
+ * Access to the TPM 1.2 event log created by a system's
+ * firmware / BIOS
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -260,17 +261,6 @@ static int tpm_binary_bios_measurements_show(struct seq_file *m, void *v)
 
 }
 
-static int tpm_bios_measurements_release(struct inode *inode,
-					 struct file *file)
-{
-	struct seq_file *seq = (struct seq_file *)file->private_data;
-	struct tpm_chip *chip = (struct tpm_chip *)seq->private;
-
-	put_device(&chip->dev);
-
-	return seq_release(inode, file);
-}
-
 static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
 {
 	int len = 0;
@@ -304,163 +294,16 @@ static int tpm_ascii_bios_measurements_show(struct seq_file *m, void *v)
 	return 0;
 }
 
-static const struct seq_operations tpm_ascii_b_measurements_seqops = {
+const struct seq_operations tpm_ascii_b_measurements_seqops = {
 	.start = tpm_bios_measurements_start,
 	.next = tpm_bios_measurements_next,
 	.stop = tpm_bios_measurements_stop,
 	.show = tpm_ascii_bios_measurements_show,
 };
 
-static const struct seq_operations tpm_binary_b_measurements_seqops = {
+const struct seq_operations tpm_binary_b_measurements_seqops = {
 	.start = tpm_bios_measurements_start,
 	.next = tpm_bios_measurements_next,
 	.stop = tpm_bios_measurements_stop,
 	.show = tpm_binary_bios_measurements_show,
 };
-
-static int tpm_bios_measurements_open(struct inode *inode,
-					    struct file *file)
-{
-	int err;
-	struct seq_file *seq;
-	struct tpm_chip_seqops *chip_seqops;
-	const struct seq_operations *seqops;
-	struct tpm_chip *chip;
-
-	inode_lock(inode);
-	if (!inode->i_private) {
-		inode_unlock(inode);
-		return -ENODEV;
-	}
-	chip_seqops = (struct tpm_chip_seqops *)inode->i_private;
-	seqops = chip_seqops->seqops;
-	chip = chip_seqops->chip;
-	get_device(&chip->dev);
-	inode_unlock(inode);
-
-	/* now register seq file */
-	err = seq_open(file, seqops);
-	if (!err) {
-		seq = file->private_data;
-		seq->private = chip;
-	}
-
-	return err;
-}
-
-static const struct file_operations tpm_bios_measurements_ops = {
-	.owner = THIS_MODULE,
-	.open = tpm_bios_measurements_open,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = tpm_bios_measurements_release,
-};
-
-static int is_bad(void *p)
-{
-	if (!p)
-		return 1;
-	if (IS_ERR(p) && (PTR_ERR(p) != -ENODEV))
-		return 1;
-	return 0;
-}
-
-static int tpm_read_log(struct tpm_chip *chip)
-{
-	int rc;
-
-	if (chip->log.bios_event_log != NULL) {
-		dev_dbg(&chip->dev,
-			"%s: ERROR - event log already initialized\n",
-			__func__);
-		return -EFAULT;
-	}
-
-	rc = tpm_read_log_acpi(chip);
-	if (rc != -ENODEV)
-		return rc;
-
-	return tpm_read_log_of(chip);
-}
-
-/*
- * tpm_bios_log_setup() - Read the event log from the firmware
- * @chip: TPM chip to use.
- *
- * If an event log is found then the securityfs files are setup to
- * export it to userspace, otherwise nothing is done.
- *
- * Returns -ENODEV if the firmware has no event log.
- */
-int tpm_bios_log_setup(struct tpm_chip *chip)
-{
-	const char *name = dev_name(&chip->dev);
-	unsigned int cnt;
-	int rc = 0;
-
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return 0;
-
-	rc = tpm_read_log(chip);
-	if (rc)
-		return rc;
-
-	cnt = 0;
-	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
-	if (is_bad(chip->bios_dir[cnt]))
-		goto err;
-	cnt++;
-
-	chip->bin_log_seqops.chip = chip;
-	chip->bin_log_seqops.seqops = &tpm_binary_b_measurements_seqops;
-
-	chip->bios_dir[cnt] =
-	    securityfs_create_file("binary_bios_measurements",
-				   0440, chip->bios_dir[0],
-				   (void *)&chip->bin_log_seqops,
-				   &tpm_bios_measurements_ops);
-	if (is_bad(chip->bios_dir[cnt]))
-		goto err;
-	cnt++;
-
-	chip->ascii_log_seqops.chip = chip;
-	chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops;
-
-	chip->bios_dir[cnt] =
-	    securityfs_create_file("ascii_bios_measurements",
-				   0440, chip->bios_dir[0],
-				   (void *)&chip->ascii_log_seqops,
-				   &tpm_bios_measurements_ops);
-	if (is_bad(chip->bios_dir[cnt]))
-		goto err;
-	cnt++;
-
-	return 0;
-
-err:
-	chip->bios_dir[cnt] = NULL;
-	tpm_bios_log_teardown(chip);
-	return -EIO;
-}
-
-void tpm_bios_log_teardown(struct tpm_chip *chip)
-{
-	int i;
-	struct inode *inode;
-
-	/* securityfs_remove currently doesn't take care of handling sync
-	 * between removal and opening of pseudo files. To handle this, a
-	 * workaround is added by making i_private = NULL here during removal
-	 * and to check it during open(), both within inode_lock()/unlock().
-	 * This design ensures that open() either safely gets kref or fails.
-	 */
-	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {
-		if (chip->bios_dir[i]) {
-			inode = d_inode(chip->bios_dir[i]);
-			inode_lock(inode);
-			inode->i_private = NULL;
-			inode_unlock(inode);
-			securityfs_remove(chip->bios_dir[i]);
-		}
-	}
-}
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 1660d74..155467d 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -73,6 +73,9 @@ enum tcpa_pc_event_ids {
 	HOST_TABLE_OF_DEVICES,
 };
 
+extern const struct seq_operations tpm_ascii_b_measurements_seqops;
+extern const struct seq_operations tpm_binary_b_measurements_seqops;
+
 #if defined(CONFIG_ACPI)
 int tpm_read_log_acpi(struct tpm_chip *chip);
 #else
diff --git a/drivers/char/tpm/tpm_eventlog_init.c b/drivers/char/tpm/tpm_eventlog_init.c
new file mode 100644
index 0000000..ae42f46
--- /dev/null
+++ b/drivers/char/tpm/tpm_eventlog_init.c
@@ -0,0 +1,189 @@
+/*
+ * Copyright (C) 2005, 2012 IBM Corporation
+ *
+ * Authors:
+ *	Kent Yoder <key@linux.vnet.ibm.com>
+ *	Seiji Munetoh <munetoh@jp.ibm.com>
+ *	Stefan Berger <stefanb@us.ibm.com>
+ *	Reiner Sailer <sailer@watson.ibm.com>
+ *	Kylene Hall <kjhall@us.ibm.com>
+ *	Nayna Jain <nayna@linux.vnet.ibm.com>
+ *
+ * Maintained by: <tpmdd-devel@lists.sourceforge.net>
+ *
+ * Defines common initialization functions to access
+ * firmware event log for TPM 1.2 and TPM 2.0
+ *
+ * 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; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ */
+
+#include <linux/seq_file.h>
+#include <linux/fs.h>
+#include <linux/security.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "tpm.h"
+#include "tpm_eventlog.h"
+
+static int tpm_bios_measurements_release(struct inode *inode,
+					 struct file *file)
+{
+	struct seq_file *seq = (struct seq_file *)file->private_data;
+	struct tpm_chip *chip = (struct tpm_chip *)seq->private;
+
+	put_device(&chip->dev);
+
+	return seq_release(inode, file);
+}
+
+static int tpm_bios_measurements_open(struct inode *inode,
+					    struct file *file)
+{
+	int err;
+	struct seq_file *seq;
+	struct tpm_chip_seqops *chip_seqops;
+	const struct seq_operations *seqops;
+	struct tpm_chip *chip;
+
+	inode_lock(inode);
+	if (!inode->i_private) {
+		inode_unlock(inode);
+		return -ENODEV;
+	}
+	chip_seqops = (struct tpm_chip_seqops *)inode->i_private;
+	seqops = chip_seqops->seqops;
+	chip = chip_seqops->chip;
+	get_device(&chip->dev);
+	inode_unlock(inode);
+
+	/* now register seq file */
+	err = seq_open(file, seqops);
+	if (!err) {
+		seq = file->private_data;
+		seq->private = chip;
+	}
+
+	return err;
+}
+
+static const struct file_operations tpm_bios_measurements_ops = {
+	.owner = THIS_MODULE,
+	.open = tpm_bios_measurements_open,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = tpm_bios_measurements_release,
+};
+
+static int is_bad(void *p)
+{
+	if (!p)
+		return 1;
+	if (IS_ERR(p) && (PTR_ERR(p) != -ENODEV))
+		return 1;
+	return 0;
+}
+
+static int tpm_read_log(struct tpm_chip *chip)
+{
+	int rc;
+
+	if (chip->log.bios_event_log != NULL) {
+		dev_dbg(&chip->dev,
+			"%s: ERROR - event log already initialized\n",
+			__func__);
+		return -EFAULT;
+	}
+
+	rc = tpm_read_log_acpi(chip);
+	if (rc != -ENODEV)
+		return rc;
+
+	return tpm_read_log_of(chip);
+}
+
+/*
+ * tpm_bios_log_setup() - Read the event log from the firmware
+ * @chip: TPM chip to use.
+ *
+ * If an event log is found then the securityfs files are setup to
+ * export it to userspace, otherwise nothing is done.
+ *
+ * Returns -ENODEV if the firmware has no event log.
+ */
+int tpm_bios_log_setup(struct tpm_chip *chip)
+{
+	const char *name = dev_name(&chip->dev);
+	unsigned int cnt;
+	int rc = 0;
+
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		return 0;
+
+	rc = tpm_read_log(chip);
+	if (rc)
+		return rc;
+
+	cnt = 0;
+	chip->bios_dir[cnt] = securityfs_create_dir(name, NULL);
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
+
+	chip->bin_log_seqops.chip = chip;
+	chip->bin_log_seqops.seqops = &tpm_binary_b_measurements_seqops;
+
+	chip->bios_dir[cnt] =
+	    securityfs_create_file("binary_bios_measurements",
+				   0440, chip->bios_dir[0],
+				   (void *)&chip->bin_log_seqops,
+				   &tpm_bios_measurements_ops);
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
+
+	chip->ascii_log_seqops.chip = chip;
+	chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops;
+
+	chip->bios_dir[cnt] =
+	    securityfs_create_file("ascii_bios_measurements",
+				   0440, chip->bios_dir[0],
+				   (void *)&chip->ascii_log_seqops,
+				   &tpm_bios_measurements_ops);
+	if (is_bad(chip->bios_dir[cnt]))
+		goto err;
+	cnt++;
+
+	return 0;
+
+err:
+	chip->bios_dir[cnt] = NULL;
+	tpm_bios_log_teardown(chip);
+	return -EIO;
+}
+
+void tpm_bios_log_teardown(struct tpm_chip *chip)
+{
+	int i;
+	struct inode *inode;
+
+	/* securityfs_remove currently doesn't take care of handling sync
+	 * between removal and opening of pseudo files. To handle this, a
+	 * workaround is added by making i_private = NULL here during removal
+	 * and to check it during open(), both within inode_lock()/unlock().
+	 * This design ensures that open() either safely gets kref or fails.
+	 */
+	for (i = (TPM_NUM_EVENT_LOG_FILES - 1); i >= 0; i--) {
+		if (chip->bios_dir[i]) {
+			inode = d_inode(chip->bios_dir[i]);
+			inode_lock(inode);
+			inode->i_private = NULL;
+			inode_unlock(inode);
+			securityfs_remove(chip->bios_dir[i]);
+		}
+	}
+}
-- 
2.5.0

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

* [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log
  2016-11-23 17:27 [PATCH v5 0/3] tpm: securityfs support for TPM 2.0 firmware event log Nayna Jain
  2016-11-23 17:27 ` [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c Nayna Jain
@ 2016-11-23 17:27 ` Nayna Jain
  2016-11-23 19:37   ` Jason Gunthorpe
  2016-11-24 21:06   ` Jarkko Sakkinen
  2016-11-23 17:27 ` [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware " Nayna Jain
  2 siblings, 2 replies; 19+ messages in thread
From: Nayna Jain @ 2016-11-23 17:27 UTC (permalink / raw)
  To: tpmdd-devel
  Cc: peterhuewe, tpmdd, jarkko.sakkinen, jgunthorpe,
	linux-security-module, linux-kernel, Nayna Jain

Physical TPMs use Open Firmware Device Tree bindings that are similar
to the IBM Power virtual TPM to support event log. However, these
properties store the values in different endianness for Physical
and Virtual TPM.

This patch fixes the endianness issue by doing appropriate conversion
based on Physical or Virtual TPM.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/tpm_of.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
index 7dee42d7..646aec1 100644
--- a/drivers/char/tpm/tpm_of.c
+++ b/drivers/char/tpm/tpm_of.c
@@ -27,6 +27,7 @@ int tpm_read_log_of(struct tpm_chip *chip)
 	const u32 *sizep;
 	const u64 *basep;
 	struct tpm_bios_log *log;
+	u32 log_size;
 
 	log = &chip->log;
 	if (chip->dev.parent && chip->dev.parent->of_node)
@@ -34,25 +35,43 @@ int tpm_read_log_of(struct tpm_chip *chip)
 	else
 		return -ENODEV;
 
+	/*
+	 * For both vtpm/tpm, firmware has log addr and log size in big
+	 * endian format. But in case of vtpm, there is a method called
+	 * sml-handover which is run during kernel init even before
+	 * device tree is setup. This sml-handover function takes care
+	 * of endianness and writes to sml-base and sml-size in little
+	 * endian format. For this reason, vtpm doesn't need conversion
+	 * but physical tpm needs the conversion.
+	 */
 	sizep = of_get_property(np, "linux,sml-size", NULL);
+	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0)
+		log_size = be32_to_cpup(sizep);
+	else
+		log_size = *sizep;
+
 	basep = of_get_property(np, "linux,sml-base", NULL);
 	if (sizep == NULL && basep == NULL)
 		return -ENODEV;
 	if (sizep == NULL || basep == NULL)
 		return -EIO;
 
-	if (*sizep == 0) {
+	if (log_size == 0) {
 		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
 		return -EIO;
 	}
 
-	log->bios_event_log = kmalloc(*sizep, GFP_KERNEL);
+	log->bios_event_log = kmalloc(log_size, GFP_KERNEL);
 	if (!log->bios_event_log)
 		return -ENOMEM;
 
-	log->bios_event_log_end = log->bios_event_log + *sizep;
+	log->bios_event_log_end = log->bios_event_log + log_size;
 
-	memcpy(log->bios_event_log, __va(*basep), *sizep);
+	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0)
+		memcpy(chip->log.bios_event_log, __va(be64_to_cpup(basep)),
+		       log_size);
+	else
+		memcpy(chip->log.bios_event_log, __va(*basep), log_size);
 
 	return 0;
 }
-- 
2.5.0

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

* [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware event log
  2016-11-23 17:27 [PATCH v5 0/3] tpm: securityfs support for TPM 2.0 firmware event log Nayna Jain
  2016-11-23 17:27 ` [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c Nayna Jain
  2016-11-23 17:27 ` [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log Nayna Jain
@ 2016-11-23 17:27 ` Nayna Jain
  2016-11-24 21:10   ` Jarkko Sakkinen
  2 siblings, 1 reply; 19+ messages in thread
From: Nayna Jain @ 2016-11-23 17:27 UTC (permalink / raw)
  To: tpmdd-devel
  Cc: peterhuewe, tpmdd, jarkko.sakkinen, jgunthorpe,
	linux-security-module, linux-kernel, Nayna Jain

Unlike the device driver support for TPM 1.2, the TPM 2.0 does
not support the securityfs pseudo files for displaying the
firmware event log.

This patch enables support for providing the TPM 2.0 event log in
binary form. TPM 2.0 event log supports a crypto agile format that
records multiple digests, which is different from TPM 1.2. This
patch enables the tpm_bios_log_setup for TPM 2.0  and adds the
event log parser which understand the TPM 2.0 crypto agile format.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 drivers/char/tpm/Makefile            |   2 +-
 drivers/char/tpm/tpm2_eventlog.c     | 214 +++++++++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_eventlog.h      |  69 +++++++++++
 drivers/char/tpm/tpm_eventlog_init.c |  43 +++----
 4 files changed, 308 insertions(+), 20 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2_eventlog.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 1dc2671..f185a6a5 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -3,7 +3,7 @@
 #
 obj-$(CONFIG_TCG_TPM) += tpm.o
 tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
-		tpm_eventlog.o tpm_eventlog_init.o
+		tpm_eventlog.o tpm_eventlog_init.o tpm2_eventlog.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/tpm2_eventlog.c b/drivers/char/tpm/tpm2_eventlog.c
new file mode 100644
index 0000000..865e70e
--- /dev/null
+++ b/drivers/char/tpm/tpm2_eventlog.c
@@ -0,0 +1,214 @@
+/*
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * Authors:
+ *      Nayna Jain <nayna@linux.vnet.ibm.com>
+ *
+ * Access to TPM 2.0 event log as written by Firmware.
+ * It assumes that writer of event log has followed TCG Spec 2.0
+ * has written the event struct data in little endian. With that,
+ * it doesn't need any endian conversion for structure content.
+ *
+ * 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; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/seq_file.h>
+#include <linux/fs.h>
+#include <linux/security.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "tpm.h"
+#include "tpm_eventlog.h"
+
+static int calc_tpm2_event_size(struct tcg_pcr_event2 *event,
+		struct tcg_pcr_event *event_header)
+{
+	struct tcg_efi_specid_event *efispecid;
+	struct tcg_event_field *event_field;
+	void *marker, *marker_start;
+	int i, j;
+	u16 halg;
+	u32 halg_size;
+	size_t size = 0;
+
+	/*
+	 * NOTE: TPM 2.0 supports extend to multiple PCR Banks. This implies
+	 * event log also has multiple digest values, one for each PCR Bank.
+	 * This is called Crypto Agile Log Entry Format.
+	 * TCG EFI Protocol Specification defines the procedure to parse
+	 * the event log. Below code implements this procedure to parse
+	 * correctly the Crypto agile log entry format.
+	 * Example of Crypto Agile Log Digests Format :
+	 * digest_values.count = 2;
+	 * digest_values.digest[0].alg_id = sha1;
+	 * digest_values.digest[0].digest.sha1 = {20 bytes raw data};
+	 * digest_values.digest[1].alg_id = sha256;
+	 * digest_values.digest[1].digest.sha256 = {32 bytes raw data};
+	 * Offset of eventsize is sizeof(count) + sizeof(alg_id) + 20
+	 *			+ sizeof(alg_id) + 32;
+	 *
+	 * Since, offset of event_size can vary based on digests count, offset
+	 * has to be calculated at run time. void *marker is used to traverse
+	 * the dynamic structure and calculate the offset of event_size.
+	 */
+
+	marker = event;
+	marker_start = marker;
+	marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
+		+ sizeof(event->digests.count);
+
+	efispecid = (struct tcg_efi_specid_event *) event_header->event;
+
+	for (i = 0; (i < event->digests.count) && (i < HASH_COUNT); i++) {
+		halg_size = sizeof(event->digests.digests[i].alg_id);
+		memcpy(&halg, marker, halg_size);
+		marker = marker + halg_size;
+		for (j = 0; (j < efispecid->num_algs); j++) {
+			if (halg == efispecid->digest_sizes[j].alg_id) {
+				marker = marker +
+					efispecid->digest_sizes[j].digest_size;
+				break;
+			}
+		}
+	}
+
+	event_field = (struct tcg_event_field *) marker;
+	marker = marker + sizeof(event_field->event_size)
+		+ event_field->event_size;
+	size = marker - marker_start;
+
+	if ((event->event_type == 0) && (event_field->event_size == 0))
+		return 0;
+
+	return size;
+}
+
+static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
+{
+	struct tpm_chip *chip = m->private;
+	struct tpm_bios_log *log = &chip->log;
+	void *addr = log->bios_event_log;
+	void *limit = log->bios_event_log_end;
+	struct tcg_pcr_event *event_header;
+	struct tcg_pcr_event2 *event;
+	int i;
+	size_t size = 0;
+
+	event_header = addr;
+
+	size = sizeof(struct tcg_pcr_event) - sizeof(event_header->event)
+		+ event_header->event_size;
+
+
+	if (*pos == 0) {
+		if (addr + size < limit) {
+			if ((event_header->event_type == 0) &&
+					(event_header->event_size == 0))
+				return NULL;
+			return SEQ_START_TOKEN;
+		}
+	}
+
+	if (*pos > 0) {
+		addr += size;
+		event = addr;
+		size = calc_tpm2_event_size(event, event_header);
+		if ((addr + size >=  limit) || (size == 0))
+			return NULL;
+	}
+
+	/* read over *pos measurements */
+	for (i = 0; i < (*pos - 1); i++) {
+		event = addr;
+		size = calc_tpm2_event_size(event, event_header);
+
+		if ((addr + size >= limit) || (size == 0))
+			return NULL;
+		addr += size;
+	}
+
+	return addr;
+}
+
+static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
+		loff_t *pos)
+{
+	struct tcg_pcr_event *event_header;
+	struct tcg_pcr_event2 *event;
+	struct tpm_chip *chip = m->private;
+	struct tpm_bios_log *log = &chip->log;
+	void *limit = log->bios_event_log_end;
+	void *marker;
+	size_t event_size = 0;
+
+	event_header = log->bios_event_log;
+
+	if (v == SEQ_START_TOKEN) {
+		event_size = sizeof(struct tcg_pcr_event)
+			- sizeof(event_header->event)
+			+ event_header->event_size;
+		marker = event_header;
+	} else {
+		event = v;
+		event_size = calc_tpm2_event_size(event, event_header);
+		if (event_size == 0)
+			return NULL;
+		marker =  event;
+	}
+
+	marker = marker + event_size;
+	if (marker >= limit)
+		return NULL;
+	v = marker;
+	event = v;
+
+	event_size = calc_tpm2_event_size(event, event_header);
+	if (((v + event_size) >= limit) || (event_size == 0))
+		return NULL;
+
+	(*pos)++;
+	return v;
+}
+
+static void tpm2_bios_measurements_stop(struct seq_file *m, void *v)
+{
+}
+
+static int tpm2_binary_bios_measurements_show(struct seq_file *m, void *v)
+{
+	struct tpm_chip *chip = m->private;
+	struct tpm_bios_log *log = &chip->log;
+	struct tcg_pcr_event *event_header = log->bios_event_log;
+	struct tcg_pcr_event2 *event = v;
+	void *temp_ptr;
+	size_t size = 0;
+
+	if (v == SEQ_START_TOKEN) {
+		size = sizeof(struct tcg_pcr_event)
+			- sizeof(event_header->event)
+			+ event_header->event_size;
+
+		temp_ptr = event_header;
+
+		if (size > 0)
+			seq_write(m, temp_ptr, size);
+	} else {
+		size = calc_tpm2_event_size(event, event_header);
+		temp_ptr = event;
+		if (size > 0)
+			seq_write(m, temp_ptr, size);
+	}
+
+	return 0;
+}
+
+const struct seq_operations tpm2_binary_b_measurements_seqops = {
+	.start = tpm2_bios_measurements_start,
+	.next = tpm2_bios_measurements_next,
+	.stop = tpm2_bios_measurements_stop,
+	.show = tpm2_binary_bios_measurements_show,
+};
diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h
index 155467d..0780f1e 100644
--- a/drivers/char/tpm/tpm_eventlog.h
+++ b/drivers/char/tpm/tpm_eventlog.h
@@ -5,6 +5,9 @@
 #define TCG_EVENT_NAME_LEN_MAX	255
 #define MAX_TEXT_EVENT		1000	/* Max event string length */
 #define ACPI_TCPA_SIG		"TCPA"	/* 0x41504354 /'TCPA' */
+#define HASH_COUNT		3
+#define MAX_TPM_LOG_MSG		128
+#define MAX_DIGEST_SIZE		64
 
 #ifdef CONFIG_PPC64
 #define do_endian_conversion(x) be32_to_cpu(x)
@@ -73,8 +76,74 @@ enum tcpa_pc_event_ids {
 	HOST_TABLE_OF_DEVICES,
 };
 
+/*
+ * All the structures related to TPM 2.0 Event Log are taken from TCG EFIi
+ * Protocol * Specification, Family "2.0". Document is available on link
+ * http://www.trustedcomputinggroup.org/tcg-efi-protocol-specification/
+ * Information is also available on TCG PC Client Platform Firmware Profile
+ * Specification, Family "2.0"
+ * Detailed digest structures for TPM 2.0 are defined in document
+ * Trusted Platform Module Library Part 2: Structures, Family "2.0".
+ */
+
+/* TPM 2.0 Event log header algorithm spec. */
+struct tcg_efi_specid_event_algs {
+	u16     alg_id;
+	u16     digest_size;
+} __packed;
+
+/* TPM 2.0 Event log header data. */
+struct tcg_efi_specid_event {
+	u8      signature[16];
+	u32     platform_class;
+	u8      spec_version_minor;
+	u8      spec_version_major;
+	u8      spec_errata;
+	u8      uintnsize;
+	u32     num_algs;
+	struct tcg_efi_specid_event_algs   digest_sizes[HASH_COUNT];
+	u8      vendor_info_size;
+	u8      vendor_info[0];
+} __packed;
+
+/* TPM 2.0 Event Log Header. */
+struct tcg_pcr_event {
+	u32     pcr_idx;
+	u32     event_type;
+	u8      digest[20];
+	u32     event_size;
+	u8      event[MAX_TPM_LOG_MSG];
+} __packed;
+
+/* TPM 2.0 Crypto agile algorithm and respective digest. */
+struct tpmt_ha {
+	u16     alg_id;
+	u8      digest[MAX_DIGEST_SIZE];
+} __packed;
+
+/* TPM 2.0 Crypto agile digests list. */
+struct tpml_digest_values {
+	u32     count;
+	struct tpmt_ha  digests[HASH_COUNT];
+} __packed;
+
+/* TPM 2.0 Event field structure. */
+struct tcg_event_field {
+	u32     event_size;
+	u8      event[MAX_TPM_LOG_MSG];
+} __packed;
+
+/* TPM 2.0 Crypto agile log entry format. */
+struct tcg_pcr_event2 {
+	u32     pcr_idx;
+	u32     event_type;
+	struct tpml_digest_values digests;
+	struct tcg_event_field  event;
+} __packed;
+
 extern const struct seq_operations tpm_ascii_b_measurements_seqops;
 extern const struct seq_operations tpm_binary_b_measurements_seqops;
+extern const struct seq_operations tpm2_binary_b_measurements_seqops;
 
 #if defined(CONFIG_ACPI)
 int tpm_read_log_acpi(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm_eventlog_init.c b/drivers/char/tpm/tpm_eventlog_init.c
index ae42f46..d1fe83e 100644
--- a/drivers/char/tpm/tpm_eventlog_init.c
+++ b/drivers/char/tpm/tpm_eventlog_init.c
@@ -121,9 +121,6 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 	unsigned int cnt;
 	int rc = 0;
 
-	if (chip->flags & TPM_CHIP_FLAG_TPM2)
-		return 0;
-
 	rc = tpm_read_log(chip);
 	if (rc)
 		return rc;
@@ -135,28 +132,36 @@ int tpm_bios_log_setup(struct tpm_chip *chip)
 	cnt++;
 
 	chip->bin_log_seqops.chip = chip;
-	chip->bin_log_seqops.seqops = &tpm_binary_b_measurements_seqops;
+	if (chip->flags & TPM_CHIP_FLAG_TPM2)
+		chip->bin_log_seqops.seqops =
+			&tpm2_binary_b_measurements_seqops;
+	else
+		chip->bin_log_seqops.seqops =
+			&tpm_binary_b_measurements_seqops;
 
 	chip->bios_dir[cnt] =
-	    securityfs_create_file("binary_bios_measurements",
-				   0440, chip->bios_dir[0],
-				   (void *)&chip->bin_log_seqops,
-				   &tpm_bios_measurements_ops);
+		securityfs_create_file("binary_bios_measurements",
+				       0440, chip->bios_dir[0],
+				       (void *)&chip->bin_log_seqops,
+				       &tpm_bios_measurements_ops);
 	if (is_bad(chip->bios_dir[cnt]))
 		goto err;
 	cnt++;
 
-	chip->ascii_log_seqops.chip = chip;
-	chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops;
-
-	chip->bios_dir[cnt] =
-	    securityfs_create_file("ascii_bios_measurements",
-				   0440, chip->bios_dir[0],
-				   (void *)&chip->ascii_log_seqops,
-				   &tpm_bios_measurements_ops);
-	if (is_bad(chip->bios_dir[cnt]))
-		goto err;
-	cnt++;
+	if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
+		chip->ascii_log_seqops.chip = chip;
+		chip->ascii_log_seqops.seqops =
+			&tpm_ascii_b_measurements_seqops;
+
+		chip->bios_dir[cnt] =
+			securityfs_create_file("ascii_bios_measurements",
+					       0440, chip->bios_dir[0],
+					       (void *)&chip->ascii_log_seqops,
+					       &tpm_bios_measurements_ops);
+		if (is_bad(chip->bios_dir[cnt]))
+			goto err;
+		cnt++;
+	}
 
 	return 0;
 
-- 
2.5.0

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

* Re: [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log
  2016-11-23 17:27 ` [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log Nayna Jain
@ 2016-11-23 19:37   ` Jason Gunthorpe
  2016-11-24  7:53     ` Nayna
  2016-11-24 21:06   ` Jarkko Sakkinen
  1 sibling, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2016-11-23 19:37 UTC (permalink / raw)
  To: Nayna Jain
  Cc: tpmdd-devel, peterhuewe, tpmdd, jarkko.sakkinen,
	linux-security-module, linux-kernel

On Wed, Nov 23, 2016 at 12:27:36PM -0500, Nayna Jain wrote:
>  	sizep = of_get_property(np, "linux,sml-size", NULL);
> +	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0)
> +		log_size = be32_to_cpup(sizep);
> +	else
> +		log_size = *sizep;
> +

Uh, no, sizep can be null at this point:

>  	basep = of_get_property(np, "linux,sml-base", NULL);
>  	if (sizep == NULL && basep == NULL)
>  		return -ENODEV;
>  	if (sizep == NULL || basep == NULL)
>  		return -EIO;

Move the if here.

> -	if (*sizep == 0) {
> +	if (log_size == 0) {
>  		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
>  		return -EIO;
>  	}
>  
> -	log->bios_event_log = kmalloc(*sizep, GFP_KERNEL);
> +	log->bios_event_log = kmalloc(log_size, GFP_KERNEL);
>  	if (!log->bios_event_log)
>  		return -ENOMEM;
>  
> -	log->bios_event_log_end = log->bios_event_log + *sizep;
> +	log->bios_event_log_end = log->bios_event_log + log_size;
>  
> -	memcpy(log->bios_event_log, __va(*basep), *sizep);
> +	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0)
> +		memcpy(chip->log.bios_event_log, __va(be64_to_cpup(basep)),
> +		       log_size);
> +	else
> +		memcpy(chip->log.bios_event_log, __va(*basep),
> log_size);

And move the conditional swap of basep up to be along side sizep as
well (ie get rid of the second of_property_match_string)

Jason

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

* Re: [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c
  2016-11-23 17:27 ` [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c Nayna Jain
@ 2016-11-23 19:38   ` Jason Gunthorpe
  2016-11-24  8:01     ` Nayna
  2016-11-24 21:01     ` Jarkko Sakkinen
  0 siblings, 2 replies; 19+ messages in thread
From: Jason Gunthorpe @ 2016-11-23 19:38 UTC (permalink / raw)
  To: Nayna Jain
  Cc: tpmdd-devel, peterhuewe, tpmdd, jarkko.sakkinen,
	linux-security-module, linux-kernel

On Wed, Nov 23, 2016 at 12:27:35PM -0500, Nayna Jain wrote:
> The device driver code for the event log has the init functions and
> TPM 1.2 parsing logic both defined in same file(tpm_eventlog.c).
> 
> Since the initialization functions are common with the TPM 2.0 event
> log support, this patch splits tpm_eventlog.c to have only TPM 1.2
> event log parsing logic and moves the init functions into
> tpm_eventlog_init.c.

I think I'd rather see a tpm_eventlog1.c/tpm_eventlog2.c than this
_init thing..

Jason

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

* Re: [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log
  2016-11-23 19:37   ` Jason Gunthorpe
@ 2016-11-24  7:53     ` Nayna
  0 siblings, 0 replies; 19+ messages in thread
From: Nayna @ 2016-11-24  7:53 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: tpmdd-devel, peterhuewe, tpmdd, jarkko.sakkinen,
	linux-security-module, linux-kernel



On 11/24/2016 01:07 AM, Jason Gunthorpe wrote:
> On Wed, Nov 23, 2016 at 12:27:36PM -0500, Nayna Jain wrote:
>>   	sizep = of_get_property(np, "linux,sml-size", NULL);
>> +	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0)
>> +		log_size = be32_to_cpup(sizep);
>> +	else
>> +		log_size = *sizep;
>> +
>
> Uh, no, sizep can be null at this point:

Oops !! Missed the basic. Will fix it. Sorry, and Thanks for noticing.

>
>>   	basep = of_get_property(np, "linux,sml-base", NULL);
>>   	if (sizep == NULL && basep == NULL)
>>   		return -ENODEV;
>>   	if (sizep == NULL || basep == NULL)
>>   		return -EIO;
>
> Move the if here.

Yeah..

>
>> -	if (*sizep == 0) {
>> +	if (log_size == 0) {
>>   		dev_warn(&chip->dev, "%s: Event log area empty\n", __func__);
>>   		return -EIO;
>>   	}
>>
>> -	log->bios_event_log = kmalloc(*sizep, GFP_KERNEL);
>> +	log->bios_event_log = kmalloc(log_size, GFP_KERNEL);
>>   	if (!log->bios_event_log)
>>   		return -ENOMEM;
>>
>> -	log->bios_event_log_end = log->bios_event_log + *sizep;
>> +	log->bios_event_log_end = log->bios_event_log + log_size;
>>
>> -	memcpy(log->bios_event_log, __va(*basep), *sizep);
>> +	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0)
>> +		memcpy(chip->log.bios_event_log, __va(be64_to_cpup(basep)),
>> +		       log_size);
>> +	else
>> +		memcpy(chip->log.bios_event_log, __va(*basep),
>> log_size);
>
> And move the conditional swap of basep up to be along side sizep as
> well (ie get rid of the second of_property_match_string)
>

Yeah.. true.. Will fix this also.

Thanks & Regards,
    - Nayna

> Jason
>

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

* Re: [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c
  2016-11-23 19:38   ` Jason Gunthorpe
@ 2016-11-24  8:01     ` Nayna
  2016-11-24 16:43       ` Jason Gunthorpe
  2016-11-24 21:01     ` Jarkko Sakkinen
  1 sibling, 1 reply; 19+ messages in thread
From: Nayna @ 2016-11-24  8:01 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: tpmdd-devel, peterhuewe, tpmdd, jarkko.sakkinen,
	linux-security-module, linux-kernel



On 11/24/2016 01:08 AM, Jason Gunthorpe wrote:
> On Wed, Nov 23, 2016 at 12:27:35PM -0500, Nayna Jain wrote:
>> The device driver code for the event log has the init functions and
>> TPM 1.2 parsing logic both defined in same file(tpm_eventlog.c).
>>
>> Since the initialization functions are common with the TPM 2.0 event
>> log support, this patch splits tpm_eventlog.c to have only TPM 1.2
>> event log parsing logic and moves the init functions into
>> tpm_eventlog_init.c.
>
> I think I'd rather see a tpm_eventlog1.c/tpm_eventlog2.c than this
> _init thing..

Do you mean tpm_eventlog1.c for TPM 1.2 and tpm_eventlog2.c for TPM 2.0 
  event log specific parsing ?

And if so, then which one should have the common functions for TPM 1.2 
and TPM 2.0 ?

Thanks & Regards,
    - Nayna

>
> Jason
>

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

* Re: [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c
  2016-11-24  8:01     ` Nayna
@ 2016-11-24 16:43       ` Jason Gunthorpe
  2016-11-24 16:50         ` Nayna
  0 siblings, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2016-11-24 16:43 UTC (permalink / raw)
  To: Nayna
  Cc: tpmdd-devel, peterhuewe, tpmdd, jarkko.sakkinen,
	linux-security-module, linux-kernel

On Thu, Nov 24, 2016 at 01:31:03PM +0530, Nayna wrote:

> >>Since the initialization functions are common with the TPM 2.0 event
> >>log support, this patch splits tpm_eventlog.c to have only TPM 1.2
> >>event log parsing logic and moves the init functions into
> >>tpm_eventlog_init.c.
> >
> >I think I'd rather see a tpm_eventlog1.c/tpm_eventlog2.c than this
> >_init thing..
> 
> Do you mean tpm_eventlog1.c for TPM 1.2 and tpm_eventlog2.c for TPM 2.0
> event log specific parsing ?
> 
> And if so, then which one should have the common functions for TPM 1.2 and
> TPM 2.0?

Leave them in tpm_eventlog.c ..

Jason

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

* Re: [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c
  2016-11-24 16:43       ` Jason Gunthorpe
@ 2016-11-24 16:50         ` Nayna
  0 siblings, 0 replies; 19+ messages in thread
From: Nayna @ 2016-11-24 16:50 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: tpmdd-devel, peterhuewe, tpmdd, jarkko.sakkinen,
	linux-security-module, linux-kernel



On 11/24/2016 10:13 PM, Jason Gunthorpe wrote:
> On Thu, Nov 24, 2016 at 01:31:03PM +0530, Nayna wrote:
>
>>>> Since the initialization functions are common with the TPM 2.0 event
>>>> log support, this patch splits tpm_eventlog.c to have only TPM 1.2
>>>> event log parsing logic and moves the init functions into
>>>> tpm_eventlog_init.c.
>>>
>>> I think I'd rather see a tpm_eventlog1.c/tpm_eventlog2.c than this
>>> _init thing..
>>
>> Do you mean tpm_eventlog1.c for TPM 1.2 and tpm_eventlog2.c for TPM 2.0
>> event log specific parsing ?
>>
>> And if so, then which one should have the common functions for TPM 1.2 and
>> TPM 2.0?
>
> Leave them in tpm_eventlog.c ..

Sure. I am fine with that. I just feel that it bit of mixed up the scope 
of tpm_eventlog.c file, where it now also refers to TPM2, but have 
corresponding seq functions in separate file.

Thanks & Regards,
    - Nayna

>
> Jason
>

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

* Re: [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c
  2016-11-23 19:38   ` Jason Gunthorpe
  2016-11-24  8:01     ` Nayna
@ 2016-11-24 21:01     ` Jarkko Sakkinen
  2016-11-25 19:43       ` Jason Gunthorpe
  1 sibling, 1 reply; 19+ messages in thread
From: Jarkko Sakkinen @ 2016-11-24 21:01 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Nayna Jain, tpmdd-devel, peterhuewe, tpmdd,
	linux-security-module, linux-kernel

On Wed, Nov 23, 2016 at 12:38:30PM -0700, Jason Gunthorpe wrote:
> On Wed, Nov 23, 2016 at 12:27:35PM -0500, Nayna Jain wrote:
> > The device driver code for the event log has the init functions and
> > TPM 1.2 parsing logic both defined in same file(tpm_eventlog.c).
> > 
> > Since the initialization functions are common with the TPM 2.0 event
> > log support, this patch splits tpm_eventlog.c to have only TPM 1.2
> > event log parsing logic and moves the init functions into
> > tpm_eventlog_init.c.
> 
> I think I'd rather see a tpm_eventlog1.c/tpm_eventlog2.c than this
> _init thing..
> 
> Jason

I would rather see tpm1-eventlog.c and tpm2-eventlog.c as we already
have tpm2-cmd.c for the sake of consistency.

/Jarkko

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

* Re: [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log
  2016-11-23 17:27 ` [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log Nayna Jain
  2016-11-23 19:37   ` Jason Gunthorpe
@ 2016-11-24 21:06   ` Jarkko Sakkinen
  1 sibling, 0 replies; 19+ messages in thread
From: Jarkko Sakkinen @ 2016-11-24 21:06 UTC (permalink / raw)
  To: Nayna Jain
  Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe,
	linux-security-module, linux-kernel

On Wed, Nov 23, 2016 at 12:27:36PM -0500, Nayna Jain wrote:
> -	memcpy(log->bios_event_log, __va(*basep), *sizep);
> +	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0)
> +		memcpy(chip->log.bios_event_log, __va(be64_to_cpup(basep)),
> +		       log_size);
> +	else
> +		memcpy(chip->log.bios_event_log, __va(*basep), log_size);


This does not look good. You should be able rearrange your code in a way
that you only have only one memcpy call.

/Jarkko

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

* Re: [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware event log
  2016-11-23 17:27 ` [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware " Nayna Jain
@ 2016-11-24 21:10   ` Jarkko Sakkinen
  2016-11-25  2:51     ` [tpmdd-devel] " Stefan Berger
  2016-11-25 19:43     ` Jason Gunthorpe
  0 siblings, 2 replies; 19+ messages in thread
From: Jarkko Sakkinen @ 2016-11-24 21:10 UTC (permalink / raw)
  To: Nayna Jain
  Cc: tpmdd-devel, peterhuewe, tpmdd, jgunthorpe,
	linux-security-module, linux-kernel

On Wed, Nov 23, 2016 at 12:27:37PM -0500, Nayna Jain wrote:
> Unlike the device driver support for TPM 1.2, the TPM 2.0 does
> not support the securityfs pseudo files for displaying the
> firmware event log.
> 
> This patch enables support for providing the TPM 2.0 event log in
> binary form. TPM 2.0 event log supports a crypto agile format that
> records multiple digests, which is different from TPM 1.2. This
> patch enables the tpm_bios_log_setup for TPM 2.0  and adds the
> event log parser which understand the TPM 2.0 crypto agile format.
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>

I don't want to say much about this before I've tested it. I wonder
what cheap hardware I could use to test this. Any advice is on this
from anyone is much appreciated.

/Jarkko

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

* Re: [tpmdd-devel] [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware event log
  2016-11-24 21:10   ` Jarkko Sakkinen
@ 2016-11-25  2:51     ` Stefan Berger
  2016-11-25  8:11       ` Jarkko Sakkinen
  2016-11-26 15:24       ` Jarkko Sakkinen
  2016-11-25 19:43     ` Jason Gunthorpe
  1 sibling, 2 replies; 19+ messages in thread
From: Stefan Berger @ 2016-11-25  2:51 UTC (permalink / raw)
  To: Jarkko Sakkinen, Nayna Jain
  Cc: linux-kernel, linux-security-module, tpmdd-devel

On 11/24/2016 04:10 PM, Jarkko Sakkinen wrote:
> On Wed, Nov 23, 2016 at 12:27:37PM -0500, Nayna Jain wrote:
>> Unlike the device driver support for TPM 1.2, the TPM 2.0 does
>> not support the securityfs pseudo files for displaying the
>> firmware event log.
>>
>> This patch enables support for providing the TPM 2.0 event log in
>> binary form. TPM 2.0 event log supports a crypto agile format that
>> records multiple digests, which is different from TPM 1.2. This
>> patch enables the tpm_bios_log_setup for TPM 2.0  and adds the
>> event log parser which understand the TPM 2.0 crypto agile format.
>>
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> I don't want to say much about this before I've tested it. I wonder
> what cheap hardware I could use to test this. Any advice is on this
> from anyone is much appreciated.

Virtual hardware would be cheap :-)

I tested this series with QEMU + vTPM + SeaBIOS with TPM 1.2 + TPM 2 
support (basing the log on ACPI). I had to fix an endianess issue on the 
SeaBIOS side, which made it work. So for this version of the patches I 
can give it my tested-by:

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


>
> /Jarkko
>
> ------------------------------------------------------------------------------
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>

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

* Re: [tpmdd-devel] [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware event log
  2016-11-25  2:51     ` [tpmdd-devel] " Stefan Berger
@ 2016-11-25  8:11       ` Jarkko Sakkinen
  2016-11-26 15:24       ` Jarkko Sakkinen
  1 sibling, 0 replies; 19+ messages in thread
From: Jarkko Sakkinen @ 2016-11-25  8:11 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Nayna Jain, linux-kernel, linux-security-module, tpmdd-devel

On Thu, Nov 24, 2016 at 09:51:03PM -0500, Stefan Berger wrote:
> On 11/24/2016 04:10 PM, Jarkko Sakkinen wrote:
> > On Wed, Nov 23, 2016 at 12:27:37PM -0500, Nayna Jain wrote:
> > > Unlike the device driver support for TPM 1.2, the TPM 2.0 does
> > > not support the securityfs pseudo files for displaying the
> > > firmware event log.
> > > 
> > > This patch enables support for providing the TPM 2.0 event log in
> > > binary form. TPM 2.0 event log supports a crypto agile format that
> > > records multiple digests, which is different from TPM 1.2. This
> > > patch enables the tpm_bios_log_setup for TPM 2.0  and adds the
> > > event log parser which understand the TPM 2.0 crypto agile format.
> > > 
> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > I don't want to say much about this before I've tested it. I wonder
> > what cheap hardware I could use to test this. Any advice is on this
> > from anyone is much appreciated.
> 
> Virtual hardware would be cheap :-)
> 
> I tested this series with QEMU + vTPM + SeaBIOS with TPM 1.2 + TPM 2 support
> (basing the log on ACPI). I had to fix an endianess issue on the SeaBIOS
> side, which made it work. So for this version of the patches I can give it
> my tested-by:
> 
> Tested-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

Thanks.

/Jarkko

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

* Re: [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware event log
  2016-11-24 21:10   ` Jarkko Sakkinen
  2016-11-25  2:51     ` [tpmdd-devel] " Stefan Berger
@ 2016-11-25 19:43     ` Jason Gunthorpe
  2016-11-26 12:59       ` Jarkko Sakkinen
  1 sibling, 1 reply; 19+ messages in thread
From: Jason Gunthorpe @ 2016-11-25 19:43 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Nayna Jain, tpmdd-devel, peterhuewe, tpmdd,
	linux-security-module, linux-kernel

On Thu, Nov 24, 2016 at 11:10:57PM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 23, 2016 at 12:27:37PM -0500, Nayna Jain wrote:
> > Unlike the device driver support for TPM 1.2, the TPM 2.0 does
> > not support the securityfs pseudo files for displaying the
> > firmware event log.
> > 
> > This patch enables support for providing the TPM 2.0 event log in
> > binary form. TPM 2.0 event log supports a crypto agile format that
> > records multiple digests, which is different from TPM 1.2. This
> > patch enables the tpm_bios_log_setup for TPM 2.0  and adds the
> > event log parser which understand the TPM 2.0 crypto agile format.
> > 
> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> 
> I don't want to say much about this before I've tested it. I wonder
> what cheap hardware I could use to test this. Any advice is on this
> from anyone is much appreciated.

If you found a small ARM system with TPM you could customize the uboot
to build an event log and pass it in via DT.

Not sure how much work that would be, does uboot have tpm code
already?

Jason

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

* Re: [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c
  2016-11-24 21:01     ` Jarkko Sakkinen
@ 2016-11-25 19:43       ` Jason Gunthorpe
  0 siblings, 0 replies; 19+ messages in thread
From: Jason Gunthorpe @ 2016-11-25 19:43 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Nayna Jain, tpmdd-devel, peterhuewe, tpmdd,
	linux-security-module, linux-kernel

On Thu, Nov 24, 2016 at 11:01:05PM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 23, 2016 at 12:38:30PM -0700, Jason Gunthorpe wrote:
> > On Wed, Nov 23, 2016 at 12:27:35PM -0500, Nayna Jain wrote:
> > > The device driver code for the event log has the init functions and
> > > TPM 1.2 parsing logic both defined in same file(tpm_eventlog.c).
> > > 
> > > Since the initialization functions are common with the TPM 2.0 event
> > > log support, this patch splits tpm_eventlog.c to have only TPM 1.2
> > > event log parsing logic and moves the init functions into
> > > tpm_eventlog_init.c.
> > 
> > I think I'd rather see a tpm_eventlog1.c/tpm_eventlog2.c than this
> > _init thing..
> 
> I would rather see tpm1-eventlog.c and tpm2-eventlog.c as we already
> have tpm2-cmd.c for the sake of consistency.

+1

Jason

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

* Re: [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware event log
  2016-11-25 19:43     ` Jason Gunthorpe
@ 2016-11-26 12:59       ` Jarkko Sakkinen
  0 siblings, 0 replies; 19+ messages in thread
From: Jarkko Sakkinen @ 2016-11-26 12:59 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Nayna Jain, tpmdd-devel, peterhuewe, tpmdd,
	linux-security-module, linux-kernel

On Fri, Nov 25, 2016 at 12:43:17PM -0700, Jason Gunthorpe wrote:
> On Thu, Nov 24, 2016 at 11:10:57PM +0200, Jarkko Sakkinen wrote:
> > On Wed, Nov 23, 2016 at 12:27:37PM -0500, Nayna Jain wrote:
> > > Unlike the device driver support for TPM 1.2, the TPM 2.0 does
> > > not support the securityfs pseudo files for displaying the
> > > firmware event log.
> > > 
> > > This patch enables support for providing the TPM 2.0 event log in
> > > binary form. TPM 2.0 event log supports a crypto agile format that
> > > records multiple digests, which is different from TPM 1.2. This
> > > patch enables the tpm_bios_log_setup for TPM 2.0  and adds the
> > > event log parser which understand the TPM 2.0 crypto agile format.
> > > 
> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > 
> > I don't want to say much about this before I've tested it. I wonder
> > what cheap hardware I could use to test this. Any advice is on this
> > from anyone is much appreciated.
> 
> If you found a small ARM system with TPM you could customize the uboot
> to build an event log and pass it in via DT.
> 
> Not sure how much work that would be, does uboot have tpm code
> already?

I have BeagleBoard Rev C (omap3530 arm board) and it has GPIOs but at
the moment I do not possess a spare dTPM module. I'll ask my employer
for one. It's quite old (2009), which is a good thing because of range
of support I would presume...

If I get the spare dTPM I can do I2C/SPI through GPIOs as long as I find
a way to inject stuff to DT. Hmm.. that said I've never done this
before. I guess you can ask I2C/SPI driver somehow to use GPIOs?

> Jason

/Jarkko

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

* Re: [tpmdd-devel] [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware event log
  2016-11-25  2:51     ` [tpmdd-devel] " Stefan Berger
  2016-11-25  8:11       ` Jarkko Sakkinen
@ 2016-11-26 15:24       ` Jarkko Sakkinen
  1 sibling, 0 replies; 19+ messages in thread
From: Jarkko Sakkinen @ 2016-11-26 15:24 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Nayna Jain, linux-kernel, linux-security-module, tpmdd-devel

On Thu, Nov 24, 2016 at 09:51:03PM -0500, Stefan Berger wrote:
> On 11/24/2016 04:10 PM, Jarkko Sakkinen wrote:
> > On Wed, Nov 23, 2016 at 12:27:37PM -0500, Nayna Jain wrote:
> > > Unlike the device driver support for TPM 1.2, the TPM 2.0 does
> > > not support the securityfs pseudo files for displaying the
> > > firmware event log.
> > > 
> > > This patch enables support for providing the TPM 2.0 event log in
> > > binary form. TPM 2.0 event log supports a crypto agile format that
> > > records multiple digests, which is different from TPM 1.2. This
> > > patch enables the tpm_bios_log_setup for TPM 2.0  and adds the
> > > event log parser which understand the TPM 2.0 crypto agile format.
> > > 
> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > I don't want to say much about this before I've tested it. I wonder
> > what cheap hardware I could use to test this. Any advice is on this
> > from anyone is much appreciated.
> 
> Virtual hardware would be cheap :-)
> 
> I tested this series with QEMU + vTPM + SeaBIOS with TPM 1.2 + TPM 2 support
> (basing the log on ACPI). I had to fix an endianess issue on the SeaBIOS
> side, which made it work. So for this version of the patches I can give it
> my tested-by:
> 
> Tested-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

Your Tested-by is much appreciated because the 4.10 release cycle has
shown how important it is to exercise code changes with tpm_vtpm_proxy
to catch all the regressions. I still would like to run these changes
with a real hardware to be able to trust them, though.

/Jarkko

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

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

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-23 17:27 [PATCH v5 0/3] tpm: securityfs support for TPM 2.0 firmware event log Nayna Jain
2016-11-23 17:27 ` [PATCH v5 1/3] tpm: move event log init functions to tpm_eventlog_init.c Nayna Jain
2016-11-23 19:38   ` Jason Gunthorpe
2016-11-24  8:01     ` Nayna
2016-11-24 16:43       ` Jason Gunthorpe
2016-11-24 16:50         ` Nayna
2016-11-24 21:01     ` Jarkko Sakkinen
2016-11-25 19:43       ` Jason Gunthorpe
2016-11-23 17:27 ` [PATCH v5 2/3] tpm: enhance read_log_of() to support Physical TPM event log Nayna Jain
2016-11-23 19:37   ` Jason Gunthorpe
2016-11-24  7:53     ` Nayna
2016-11-24 21:06   ` Jarkko Sakkinen
2016-11-23 17:27 ` [PATCH v5 3/3] tpm: add securityfs support for TPM 2.0 firmware " Nayna Jain
2016-11-24 21:10   ` Jarkko Sakkinen
2016-11-25  2:51     ` [tpmdd-devel] " Stefan Berger
2016-11-25  8:11       ` Jarkko Sakkinen
2016-11-26 15:24       ` Jarkko Sakkinen
2016-11-25 19:43     ` Jason Gunthorpe
2016-11-26 12:59       ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).