Re: [tpmdd-devel] [PATCH v2 3/3] TPM2.0:Adds securityfs support for TPM2.0 eventlog

[Jarkko Sakkinen]

On Wed, Aug 10, 2016 at 02:25:30PM +0300, Jarkko Sakkinen wrote:
> On Tue, Aug 09, 2016 at 03:34:55PM -0400, Nayna Jain wrote:
> > Adds securityfs support for TPM2.0.
> > This patch currently supports only binary_bios_measurements.
> > 
> > Changelog v2:
> > * Single tpm_of.c for reading both tpm and vtpm device tree values.
> > * Some of the issues are fixed in Patch 1 itself.
> > * Comments in tpm2.h give reference to the standard from where 
> > structs
> > are taken.
> > * Now, tpm_of.c has same code applied for both tpm and vtpm, so I think
> > that now it is needed to have generic types rather than endian specific 
> > type.
> > 
> > There are few preexisting issues as being mentioned in feedback and are not 
> > addressed in this patch. Reason being, I don't have much expertise of ACPI 
> > side as of now, 
> > and these changes will affect acpi,tpm,vtpm, all paths, so I would like to 
> > go slow
> > and fix them as different patch later after better understanding.
> > Hope this sounds ok to have them as different patch.
> > 
> > Issues which are not addressed are as below:
> > * tpm_eventlog.h still has #ifdef defined, for tpm_bios_log_setup()
> > * tpm_bios_log_setup is still being called in tpm-chip register 
> > function.
> 
> I do not understand your changelog entries. Please provide a nice one or
> most two paragraph english language description and keep breakdowns and
> changelogs in the cover letter.
> 
> Commit message does not equal to a discussion forum and I do not have
> any idea what feedback you are talking about...
> 
> Not reviewing this further. This is just terrible.

Whoops, my email client did tricks for me. Please ignore the two other
responses from me.

/Jarkko

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Re: [tpmdd-devel] [PATCH v2 3/3] TPM2.0:Adds securityfs support for TPM2.0 eventlog

[Jason Gunthorpe]

On Tue, Aug 09, 2016 at 03:34:55PM -0400, Nayna Jain wrote:
> Adds securityfs support for TPM2.0.
> This patch currently supports only binary_bios_measurements.
> 
> Changelog v2:
> * Single tpm_of.c for reading both tpm and vtpm device tree values.
> * Some of the issues are fixed in Patch 1 itself.
> * Comments in tpm2.h give reference to the standard from where structs
>   are taken.
>   * Now, tpm_of.c has same code applied for both tpm and vtpm, so I think
>   that now it is needed to have generic types rather than endian specific 
> type.
> 
> There are few preexisting issues as being mentioned in feedback and are not 
> addressed in this patch. Reason being, I don't have much expertise of ACPI 
> side as of now, 
> and these changes will affect acpi,tpm,vtpm, all paths, so I would like to go 
> slow
> and fix them as different patch later after better understanding.
> Hope this sounds ok to have them as different patch.

Yes, but do the patches first please, this stuff is obviously broken,
do not make the mess larger.

You are the only person ever to come up who can test the _of side of
this, so the rest of us have the opposite problem to you.

If you have doubts about the acpi stuff then ask.

> + /*
> +  * NOTE: Currently, support is added only for binary_bios_measurements
> +  * for TPM2.0. And so it is required to check that if device is TPM2,
> +  * only 2 entries are allocated i.e. tpm0 dir and
> +  * binary_bios_measurements file.
> +  */
> +
> + num_files = chip->flags & TPM_CHIP_FLAG_TPM2 ? 2 : 3;

?? just ++ as you go like sysfs does

>   const u32 *sizep;
>   const u64 *basep;
> + u32 log_size;
>  
>   if (log->bios_event_log != NULL) {
>   pr_err("%s: ERROR - Eventlog already initialized\n", __func__);
> @@ -52,6 +53,11 @@ int read_log(struct tpm_bios_log *log, struct tpm_chip 
> *chip)
>   goto cleanup_eio;
>   }
>  
> + if (!strncmp(np->name, "tpm", 3))
> + log_size = be32_to_cpup(sizep);
> + else
> + log_size = *sizep;

?? That deserves a comment, one of your systems has swapped
endianness?? 'np->name' is not an appropriate way to detect
that. compatible string would be better, or something else..

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev
___
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

[tpmdd-devel] [PATCH v2 3/3] TPM2.0:Adds securityfs support for TPM2.0 eventlog

[Nayna Jain]

Adds securityfs support for TPM2.0.
This patch currently supports only binary_bios_measurements.

Changelog v2:
* Single tpm_of.c for reading both tpm and vtpm device tree values.
* Some of the issues are fixed in Patch 1 itself.
* Comments in tpm2.h give reference to the standard from where structs
are taken.
* Now, tpm_of.c has same code applied for both tpm and vtpm, so I think
that now it is needed to have generic types rather than endian specific 
type.

There are few preexisting issues as being mentioned in feedback and are not 
addressed in this patch. Reason being, I don't have much expertise of ACPI side 
as of now, 
and these changes will affect acpi,tpm,vtpm, all paths, so I would like to go 
slow
and fix them as different patch later after better understanding.
Hope this sounds ok to have them as different patch.

Issues which are not addressed are as below:
* tpm_eventlog.h still has #ifdef defined, for tpm_bios_log_setup()
* tpm_bios_log_setup is still being called in tpm-chip register 
function.

Signed-off-by: Nayna Jain 
---
 drivers/char/tpm/Makefile|   8 +-
 drivers/char/tpm/tpm-chip.c  |  20 +---
 drivers/char/tpm/tpm2.h  |  85 +
 drivers/char/tpm/tpm2_eventlog.c | 224 +++
 drivers/char/tpm/tpm_eventlog.h  |   2 +-
 drivers/char/tpm/tpm_eventlog_init.c |  43 +--
 drivers/char/tpm/tpm_of.c|  17 ++-
 7 files changed, 366 insertions(+), 33 deletions(-)
 create mode 100644 drivers/char/tpm/tpm2.h
 create mode 100644 drivers/char/tpm/tpm2_eventlog.c

diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
index 9136762..509ace2 100644
--- a/drivers/char/tpm/Makefile
+++ b/drivers/char/tpm/Makefile
@@ -6,10 +6,14 @@ tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o 
tpm2-cmd.o
 tpm-$(CONFIG_ACPI) += tpm_ppi.o
 
 ifdef CONFIG_ACPI
-   tpm-y += tpm_eventlog_init.o tpm_eventlog.o tpm_acpi.o
+   tpm-y += tpm_eventlog_init.o tpm2_eventlog.o tpm_eventlog.o tpm_acpi.o
 else
 ifdef CONFIG_TCG_IBMVTPM
-   tpm-y += tpm_eventlog_init.o tpm_eventlog.o tpm_of.o
+   tpm-y += tpm_eventlog_init.o tpm2_eventlog.o tpm_eventlog.o tpm_of.o
+else
+ifdef CONFIG_PPC64
+   tpm-y += tpm_eventlog_init.o tpm2_eventlog.o tpm_eventlog.o tpm_of.o
+endif
 endif
 endif
 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 7f6cdab..3f1c489 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -285,20 +285,9 @@ static int tpm1_chip_register(struct tpm_chip *chip)
 
tpm_sysfs_add_device(chip);
 
-   tpm_bios_log_setup(chip);
-
return 0;
 }
 
-static void tpm1_chip_unregister(struct tpm_chip *chip)
-{
-   if (chip->flags & TPM_CHIP_FLAG_TPM2)
-   return;
-
-   if (chip->bios_dir)
-   tpm_bios_log_teardown(chip);
-}
-
 static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
 {
struct attribute **i;
@@ -372,10 +361,8 @@ int tpm_chip_register(struct tpm_chip *chip)
tpm_add_ppi(chip);
 
rc = tpm_add_char_device(chip);
-   if (rc) {
-   tpm1_chip_unregister(chip);
+   if (rc)
return rc;
-   }
 
chip->flags |= TPM_CHIP_FLAG_REGISTERED;
 
@@ -385,6 +372,8 @@ int tpm_chip_register(struct tpm_chip *chip)
return rc;
}
 
+   tpm_bios_log_setup(chip);
+
return 0;
 }
 EXPORT_SYMBOL_GPL(tpm_chip_register);
@@ -409,7 +398,8 @@ void tpm_chip_unregister(struct tpm_chip *chip)
 
tpm_del_legacy_sysfs(chip);
 
-   tpm1_chip_unregister(chip);
+   tpm_bios_log_teardown(chip);
+
tpm_del_char_device(chip);
 }
 EXPORT_SYMBOL_GPL(tpm_chip_unregister);
diff --git a/drivers/char/tpm/tpm2.h b/drivers/char/tpm/tpm2.h
new file mode 100644
index 000..38a8073
--- /dev/null
+++ b/drivers/char/tpm/tpm2.h
@@ -0,0 +1,85 @@
+#ifndef __TPM2_H__
+#define __TPM2_H__
+
+#define TPM_ALG_SHA1_DIGEST_SIZE   20
+#define TPM_ALG_SHA256_DIGEST_SIZE 32
+#define TPM_ALG_SHA384_DIGEST_SIZE 48
+
+#define HASH_COUNT 3
+#define MAX_TPM_LOG_MSG128
+
+/**
+ * All the structures related to Event Log are taken from TCG EFI 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 TPM2.0 are defined in document
+ * Trusted Platform Module Library Part 2: Structures, Family "2.0".
+ */
+
+/* Event log header algorithm spec. */
+struct tcg_efispecideventalgorithmsize {
+   u16 algorithm_id;
+   u16 digest_size;
+} __packed;
+
+/* Event log header data. */
+struct tcg_efispecideventstruct {
+   u8