2019-02-11 14:33:32

by Bartosz Szczepanek

[permalink] [raw]
Subject: [PATCH 0/5] Add support for TPM event log 2.0 on EFI/ARM

From: Bartosz Szczepanek <[email protected]>

These few patches introduce support for retrieving TPM event log in 2.0
format from EFI structures. Since 2.0 format involves dynamically-sized
records, some more computation is needed. Function responsible for this
has already been in kernel (calc_tpm2_event_size) - it was moved to
library, so it can be used from both EFI stub and device drivers.
Signature is slightly altered, as explained in one of the commits.

EFI stub was enhanced to call EFI GetEventLog function asking for 2.0
event log. If found, it's installed in configuration table similarily
to TPM 1.2 log. In case 2.0 log is not available, it falls back to 1.2
log.

Call to efi_retrieve_tpm2_eventlog() is added at ARM efi_entry to enable
TPM functionality on ARM / ARM64 platforms.

It was tested on two platforms - ARM64 and x86_64, both EFI-based and
equipped with TPM 2.0 module.

Bartosz Szczepanek (5):
tpm: Copy calc_tpm2_event_size() to TPM library
tpm: Change calc_tpm2_event_size signature
tpm: Use library version of calc_tpm2_event_size in sysfs code
efi/libstub/tpm: Retrieve TPM event log in 2.0 format
efi/arm: Retrieve TPM event log at efi_entry

drivers/char/tpm/eventlog/tpm2.c | 89 +++++---------------------
drivers/firmware/efi/libstub/Makefile | 3 +-
drivers/firmware/efi/libstub/arm-stub.c | 1 +
drivers/firmware/efi/libstub/tpm.c | 107 +++++++++++++++++++++++++++++++-
include/linux/tpm_eventlog.h | 3 +
lib/Makefile | 2 +
lib/tpm.c | 78 +++++++++++++++++++++++
7 files changed, 208 insertions(+), 75 deletions(-)
create mode 100644 lib/tpm.c

--
2.14.4



2019-02-11 16:01:27

by Bartosz Szczepanek

[permalink] [raw]
Subject: [PATCH 3/5] tpm: Use library version of calc_tpm2_event_size in sysfs code

From: Bartosz Szczepanek <[email protected]>

Expect negative values from calc_tpm2_event_size as error codes.
Pass efispecid instead of event header to calc_tpm2_event_size.

Also, include tpm library in the build.

Signed-off-by: Bartosz Szczepanek <[email protected]>
---
drivers/char/tpm/eventlog/tpm2.c | 89 ++++++++--------------------------------
include/linux/tpm_eventlog.h | 3 ++
lib/Makefile | 2 +
3 files changed, 22 insertions(+), 72 deletions(-)

diff --git a/drivers/char/tpm/eventlog/tpm2.c b/drivers/char/tpm/eventlog/tpm2.c
index 1b8fa9de2cac..5230821d5b1c 100644
--- a/drivers/char/tpm/eventlog/tpm2.c
+++ b/drivers/char/tpm/eventlog/tpm2.c
@@ -26,80 +26,20 @@
#include "../tpm.h"
#include "common.h"

-/*
- * calc_tpm2_event_size() - calculate the event size, where event
- * is an entry in the TPM 2.0 event log. The event is of type Crypto
- * Agile Log Entry Format as defined in TCG EFI Protocol Specification
- * Family "2.0".
-
- * @event: event whose size is to be calculated.
- * @event_header: the first event in the event log.
- *
- * Returns size of the event. If it is an invalid event, returns 0.
- */
-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;
- void *marker_start;
- u32 halg_size;
- size_t size;
- u16 halg;
- int i;
- int j;
-
- marker = event;
- marker_start = marker;
- marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
- + sizeof(event->count);
-
- efispecid = (struct tcg_efi_specid_event *)event_header->event;
-
- /* Check if event is malformed. */
- if (event->count > efispecid->num_algs)
- return 0;
-
- for (i = 0; i < event->count; i++) {
- halg_size = sizeof(event->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 +=
- efispecid->digest_sizes[j].digest_size;
- break;
- }
- }
- /* Algorithm without known length. Such event is unparseable. */
- if (j == efispecid->num_algs)
- return 0;
- }
-
- 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_efi_specid_event *efispecid;
struct tcg_pcr_event *event_header;
struct tcg_pcr_event2 *event;
- size_t size;
+ ssize_t size;
int i;

event_header = addr;
+ efispecid = (struct tcg_efi_specid_event *) event_header->event;
size = sizeof(struct tcg_pcr_event) - sizeof(event_header->event)
+ event_header->event_size;

@@ -115,16 +55,16 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
if (*pos > 0) {
addr += size;
event = addr;
- size = calc_tpm2_event_size(event, event_header);
- if ((addr + size >= limit) || (size == 0))
+ size = calc_tpm2_event_size(event, efispecid);
+ if ((addr + size >= limit) || (size < 0))
return NULL;
}

for (i = 0; i < (*pos - 1); i++) {
event = addr;
- size = calc_tpm2_event_size(event, event_header);
+ size = calc_tpm2_event_size(event, efispecid);

- if ((addr + size >= limit) || (size == 0))
+ if ((addr + size >= limit) || (size < 0))
return NULL;
addr += size;
}
@@ -135,6 +75,7 @@ static void *tpm2_bios_measurements_start(struct seq_file *m, loff_t *pos)
static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
loff_t *pos)
{
+ struct tcg_efi_specid_event *efispecid;
struct tcg_pcr_event *event_header;
struct tcg_pcr_event2 *event;
struct tpm_chip *chip = m->private;
@@ -144,6 +85,7 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
void *marker;

event_header = log->bios_event_log;
+ efispecid = (struct tcg_efi_specid_event *) event_header->event;

if (v == SEQ_START_TOKEN) {
event_size = sizeof(struct tcg_pcr_event) -
@@ -151,8 +93,8 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
marker = event_header;
} else {
event = v;
- event_size = calc_tpm2_event_size(event, event_header);
- if (event_size == 0)
+ event_size = calc_tpm2_event_size(event, efispecid);
+ if (event_size < 0)
return NULL;
marker = event;
}
@@ -163,8 +105,8 @@ static void *tpm2_bios_measurements_next(struct seq_file *m, void *v,
v = marker;
event = v;

- event_size = calc_tpm2_event_size(event, event_header);
- if (((v + event_size) >= limit) || (event_size == 0))
+ event_size = calc_tpm2_event_size(event, efispecid);
+ if (((v + event_size) >= limit) || (event_size < 0))
return NULL;

(*pos)++;
@@ -180,10 +122,13 @@ 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_efi_specid_event *efispecid;
struct tcg_pcr_event2 *event = v;
void *temp_ptr;
size_t size;

+ efispecid = (struct tcg_efi_specid_event *) event_header->event;
+
if (v == SEQ_START_TOKEN) {
size = sizeof(struct tcg_pcr_event) -
sizeof(event_header->event) + event_header->event_size;
@@ -193,7 +138,7 @@ static int tpm2_binary_bios_measurements_show(struct seq_file *m, void *v)
if (size > 0)
seq_write(m, temp_ptr, size);
} else {
- size = calc_tpm2_event_size(event, event_header);
+ size = calc_tpm2_event_size(event, efispecid);
temp_ptr = event;
if (size > 0)
seq_write(m, temp_ptr, size);
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 20d9da77fc11..872ab1545456 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -121,4 +121,7 @@ struct tcg_pcr_event2 {
struct tcg_event_field event;
} __packed;

+ssize_t calc_tpm2_event_size(struct tcg_pcr_event2 *event,
+ struct tcg_efi_specid_event *efispecid);
+
#endif
diff --git a/lib/Makefile b/lib/Makefile
index e1b59da71418..4458d914f40b 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -276,3 +276,5 @@ obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
obj-$(CONFIG_OBJAGG) += objagg.o
+
+obj-$(CONFIG_TCG_TPM) += tpm.o
--
2.14.4


2019-02-11 16:01:31

by Bartosz Szczepanek

[permalink] [raw]
Subject: [PATCH 4/5] efi/libstub/tpm: Retrieve TPM event log in 2.0 format

From: Bartosz Szczepanek <[email protected]>

Currently, the only way to get TPM 2.0 event log from firmware is to use
device tree. Introduce efi_retrieve_tpm2_eventlog_2 function to enable
retrieving it from EFI structures.

Include lib/tpm.c into EFI stub to calculate event sizes using helper
function.

Signed-off-by: Bartosz Szczepanek <[email protected]>
---
drivers/firmware/efi/libstub/Makefile | 3 +-
drivers/firmware/efi/libstub/tpm.c | 107 +++++++++++++++++++++++++++++++++-
2 files changed, 107 insertions(+), 3 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d9845099635e..0d7d66ad916d 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -38,7 +38,8 @@ OBJECT_FILES_NON_STANDARD := y
# Prevents link failures: __sanitizer_cov_trace_pc() is not linked in.
KCOV_INSTRUMENT := n

-lib-y := efi-stub-helper.o gop.o secureboot.o tpm.o
+lib-y := efi-stub-helper.o gop.o secureboot.o tpm.o \
+ lib-tpm.o

# include the stub's generic dependencies from lib/ when building for ARM/arm64
arm-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
index a90b0b8fc69a..c8c2531be413 100644
--- a/drivers/firmware/efi/libstub/tpm.c
+++ b/drivers/firmware/efi/libstub/tpm.c
@@ -129,8 +129,111 @@ static void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg)
efi_call_early(free_pool, log_tbl);
}

+static efi_status_t
+efi_calc_tpm2_eventlog_2_size(efi_system_table_t *sys_table_arg,
+ void *log, void *last_entry, ssize_t *log_size)
+{
+ struct tcg_pcr_event2 *event = last_entry;
+ struct tcg_efi_specid_event *efispecid;
+ struct tcg_pcr_event *log_header = log;
+ ssize_t last_entry_size;
+
+ efispecid = (struct tcg_efi_specid_event *) log_header->event;
+
+ if (last_entry == NULL || log_size == NULL)
+ return EFI_INVALID_PARAMETER;
+
+ if (log == last_entry) {
+ /*
+ * Only one entry (header) in the log.
+ */
+ *log_size = log_header->event_size +
+ sizeof(struct tcg_pcr_event);
+ return EFI_SUCCESS;
+ }
+
+ if (event->count > efispecid->num_algs) {
+ efi_printk(sys_table_arg,
+ "TCG2 event uses more algorithms than defined\n");
+ return EFI_INVALID_PARAMETER;
+ }
+
+ last_entry_size = calc_tpm2_event_size(last_entry, efispecid);
+ if (last_entry_size < 0) {
+ efi_printk(sys_table_arg,
+ "TCG2 log has invalid last entry size\n");
+ return EFI_INVALID_PARAMETER;
+ }
+
+ *log_size = last_entry + last_entry_size - log;
+ return EFI_SUCCESS;
+}
+
+static efi_status_t efi_retrieve_tpm2_eventlog_2(efi_system_table_t *sys_table_arg)
+{
+ efi_guid_t linux_eventlog_guid = LINUX_EFI_TPM_EVENT_LOG_GUID;
+ efi_physical_addr_t log_location = 0, log_last_entry = 0;
+ efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
+ efi_bool_t truncated;
+ efi_status_t status;
+ struct linux_efi_tpm_eventlog *log_tbl = NULL;
+ void *tcg2_protocol = NULL;
+ ssize_t log_size;
+
+ status = efi_call_early(locate_protocol, &tcg2_guid, NULL,
+ &tcg2_protocol);
+ if (status != EFI_SUCCESS)
+ return status;
+
+ status = efi_call_proto(efi_tcg2_protocol, get_event_log, tcg2_protocol,
+ EFI_TCG2_EVENT_LOG_FORMAT_TCG_2,
+ &log_location, &log_last_entry, &truncated);
+ if (status != EFI_SUCCESS)
+ return status;
+
+ if (!log_location)
+ return EFI_NOT_FOUND;
+
+ status = efi_calc_tpm2_eventlog_2_size(sys_table_arg,
+ (void *)log_location,
+ (void *) log_last_entry,
+ &log_size);
+ if (status != EFI_SUCCESS)
+ return status;
+
+ /* Allocate space for the logs and copy them. */
+ status = efi_call_early(allocate_pool, EFI_LOADER_DATA,
+ sizeof(*log_tbl) + log_size,
+ (void **) &log_tbl);
+
+ if (status != EFI_SUCCESS) {
+ efi_printk(sys_table_arg,
+ "Unable to allocate memory for event log\n");
+ return status;
+ }
+
+ memset(log_tbl, 0, sizeof(*log_tbl) + log_size);
+ log_tbl->size = log_size;
+ log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
+ memcpy(log_tbl->log, (void *) log_location, log_size);
+
+ status = efi_call_early(install_configuration_table,
+ &linux_eventlog_guid, log_tbl);
+ if (status != EFI_SUCCESS)
+ goto err_free;
+
+ return EFI_SUCCESS;
+
+err_free:
+ efi_call_early(free_pool, log_tbl);
+ return status;
+}
+
void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table_arg)
{
- /* Only try to retrieve the logs in 1.2 format. */
- efi_retrieve_tpm2_eventlog_1_2(sys_table_arg);
+ efi_status_t status;
+
+ status = efi_retrieve_tpm2_eventlog_2(sys_table_arg);
+ if (status != EFI_SUCCESS)
+ efi_retrieve_tpm2_eventlog_1_2(sys_table_arg);
}
--
2.14.4


2019-02-11 16:01:50

by Bartosz Szczepanek

[permalink] [raw]
Subject: [PATCH 2/5] tpm: Change calc_tpm2_event_size signature

From: Bartosz Szczepanek <[email protected]>

Pass tcg_efi_specid_event as an argument instead of tcg_pcr_event, as the
former is what is actually needed to compute event size. tcg_pcr_event
structure describes TPM event log header (even though its name), from where
efispecid can be extracted -- it seems cleaner and less misleading to do it
out of calc_tpm2_event_size function.

Also, use ssize_t instead of int for event log size.

Signed-off-by: Bartosz Szczepanek <[email protected]>
---
lib/tpm.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/lib/tpm.c b/lib/tpm.c
index aaeeafe52426..263ccfdaefa5 100644
--- a/lib/tpm.c
+++ b/lib/tpm.c
@@ -15,6 +15,7 @@
#include <linux/export.h>
#include <linux/string.h>
#include <linux/tpm_eventlog.h>
+#include <linux/errno.h>

/*
* calc_tpm2_event_size() - calculate the event size, where event
@@ -23,19 +24,18 @@
* Family "2.0".

* @event: event whose size is to be calculated.
- * @event_header: the first event in the event log.
+ * @efispecid: pointer to structure describing algorithms used.
*
- * Returns size of the event. If it is an invalid event, returns 0.
+ * Returns size of the event. If it is an invalid event, returns -EINVAL.
*/
-int calc_tpm2_event_size(struct tcg_pcr_event2 *event,
- struct tcg_pcr_event *event_header)
+ssize_t calc_tpm2_event_size(struct tcg_pcr_event2 *event,
+ struct tcg_efi_specid_event *efispecid)
{
- struct tcg_efi_specid_event *efispecid;
struct tcg_event_field *event_field;
void *marker;
void *marker_start;
u32 halg_size;
- size_t size;
+ ssize_t size;
u16 halg;
int i;
int j;
@@ -45,11 +45,9 @@ int calc_tpm2_event_size(struct tcg_pcr_event2 *event,
marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
+ sizeof(event->count);

- efispecid = (struct tcg_efi_specid_event *)event_header->event;
-
/* Check if event is malformed. */
if (event->count > efispecid->num_algs)
- return 0;
+ return -EINVAL;

for (i = 0; i < event->count; i++) {
halg_size = sizeof(event->digests[i].alg_id);
@@ -64,7 +62,7 @@ int calc_tpm2_event_size(struct tcg_pcr_event2 *event,
}
/* Algorithm without known length. Such event is unparseable. */
if (j == efispecid->num_algs)
- return 0;
+ return -EINVAL;
}

event_field = (struct tcg_event_field *)marker;
@@ -73,7 +71,7 @@ int calc_tpm2_event_size(struct tcg_pcr_event2 *event,
size = marker - marker_start;

if ((event->event_type == 0) && (event_field->event_size == 0))
- return 0;
+ return -EINVAL;

return size;
}
--
2.14.4


2019-02-11 16:02:18

by Bartosz Szczepanek

[permalink] [raw]
Subject: [PATCH 1/5] tpm: Copy calc_tpm2_event_size() to TPM library

From: Bartosz Szczepanek <[email protected]>

Function to calculate event size in TPM 2.0 log will also be needed in EFI
stub. Separate it to library to make it accessible out of TPM character
driver.

It will be removed from tpm2.c in subsequent commit.

Signed-off-by: Bartosz Szczepanek <[email protected]>
---
lib/tpm.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 80 insertions(+)
create mode 100644 lib/tpm.c

diff --git a/lib/tpm.c b/lib/tpm.c
new file mode 100644
index 000000000000..aaeeafe52426
--- /dev/null
+++ b/lib/tpm.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * Parts of this file based on earlier work by:
+ * Nayna Jain <[email protected]>
+ * Petr Vandrovec <[email protected]>
+ *
+ * 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/types.h>
+#include <linux/export.h>
+#include <linux/string.h>
+#include <linux/tpm_eventlog.h>
+
+/*
+ * calc_tpm2_event_size() - calculate the event size, where event
+ * is an entry in the TPM 2.0 event log. The event is of type Crypto
+ * Agile Log Entry Format as defined in TCG EFI Protocol Specification
+ * Family "2.0".
+
+ * @event: event whose size is to be calculated.
+ * @event_header: the first event in the event log.
+ *
+ * Returns size of the event. If it is an invalid event, returns 0.
+ */
+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;
+ void *marker_start;
+ u32 halg_size;
+ size_t size;
+ u16 halg;
+ int i;
+ int j;
+
+ marker = event;
+ marker_start = marker;
+ marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
+ + sizeof(event->count);
+
+ efispecid = (struct tcg_efi_specid_event *)event_header->event;
+
+ /* Check if event is malformed. */
+ if (event->count > efispecid->num_algs)
+ return 0;
+
+ for (i = 0; i < event->count; i++) {
+ halg_size = sizeof(event->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 +=
+ efispecid->digest_sizes[j].digest_size;
+ break;
+ }
+ }
+ /* Algorithm without known length. Such event is unparseable. */
+ if (j == efispecid->num_algs)
+ return 0;
+ }
+
+ 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;
+}
+EXPORT_SYMBOL(calc_tpm2_event_size);
--
2.14.4


2019-02-11 16:02:27

by Bartosz Szczepanek

[permalink] [raw]
Subject: [PATCH 5/5] efi/arm: Retrieve TPM event log at efi_entry

From: Bartosz Szczepanek <[email protected]>

Add efi_retrieve_tpm2_eventlog() call to ARM efi_entry() function.
There's no reason to assume that TPM2 EFI structures are not available
on ARM architecture.

Signed-off-by: Bartosz Szczepanek <[email protected]>
---
drivers/firmware/efi/libstub/arm-stub.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/efi/libstub/arm-stub.c b/drivers/firmware/efi/libstub/arm-stub.c
index eee42d5e25ee..d3af12ec32e4 100644
--- a/drivers/firmware/efi/libstub/arm-stub.c
+++ b/drivers/firmware/efi/libstub/arm-stub.c
@@ -197,6 +197,7 @@ unsigned long efi_entry(void *handle, efi_system_table_t *sys_table,

/* Ask the firmware to clear memory on unclean shutdown */
efi_enable_reset_attack_mitigation(sys_table);
+ efi_retrieve_tpm2_eventlog(sys_table);

secure_boot = efi_get_secureboot(sys_table);

--
2.14.4


2019-02-13 13:49:31

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 1/5] tpm: Copy calc_tpm2_event_size() to TPM library

On Wed, Feb 13, 2019 at 01:14:43PM +0200, Jarkko Sakkinen wrote:
> On Mon, Feb 11, 2019 at 03:30:48PM +0100, [email protected] wrote:
> > From: Bartosz Szczepanek <[email protected]>
> >
> > Function to calculate event size in TPM 2.0 log will also be needed in EFI
> > stub. Separate it to library to make it accessible out of TPM character
> > driver.
> >
> > It will be removed from tpm2.c in subsequent commit.
> >
> > Signed-off-by: Bartosz Szczepanek <[email protected]>
>
> Collides with Matthew's patch set, which has priority over this because
> it was sent earlier.
>
> Matthew, what you think of this? Maybe could replace 1/4 with this in
> your patch set? Somehow feels a bit cleaner approach.

This commit is in any case broken. It leaves two versions of the
function hanging to the code base.

/Jarkko

2019-02-13 13:50:09

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 2/5] tpm: Change calc_tpm2_event_size signature

On Mon, Feb 11, 2019 at 03:30:49PM +0100, [email protected] wrote:
> From: Bartosz Szczepanek <[email protected]>
>
> Pass tcg_efi_specid_event as an argument instead of tcg_pcr_event, as the
> former is what is actually needed to compute event size. tcg_pcr_event
> structure describes TPM event log header (even though its name), from where
> efispecid can be extracted -- it seems cleaner and less misleading to do it
> out of calc_tpm2_event_size function.
>
> Also, use ssize_t instead of int for event log size.
>
> Signed-off-by: Bartosz Szczepanek <[email protected]>

Unreviewable without call sites. NAK.

/Jarkko

2019-02-13 13:50:19

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 1/5] tpm: Copy calc_tpm2_event_size() to TPM library

On Mon, Feb 11, 2019 at 03:30:48PM +0100, [email protected] wrote:
> From: Bartosz Szczepanek <[email protected]>
>
> Function to calculate event size in TPM 2.0 log will also be needed in EFI
> stub. Separate it to library to make it accessible out of TPM character
> driver.
>
> It will be removed from tpm2.c in subsequent commit.
>
> Signed-off-by: Bartosz Szczepanek <[email protected]>

Collides with Matthew's patch set, which has priority over this because
it was sent earlier.

Matthew, what you think of this? Maybe could replace 1/4 with this in
your patch set? Somehow feels a bit cleaner approach.

> ---
> lib/tpm.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
> create mode 100644 lib/tpm.c
>
> diff --git a/lib/tpm.c b/lib/tpm.c
> new file mode 100644
> index 000000000000..aaeeafe52426
> --- /dev/null
> +++ b/lib/tpm.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2016 IBM Corporation

Do we want copyright statements to new files? I'm sure that this code
would have more copyright holders than just IBM (eg VMWare). Git
documents this anyway. This is something that will be left unmaintained.

> + * Parts of this file based on earlier work by:
> + * Nayna Jain <[email protected]>
> + * Petr Vandrovec <[email protected]>

Please remove these three lines. These type of lists are just inaccurate
presentaion of the commit log.

> + * 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.

You already have SPDX identifier. Unncessary repeat.

> + */
> +#include <linux/types.h>
> +#include <linux/export.h>
> +#include <linux/string.h>
> +#include <linux/tpm_eventlog.h>
> +
> +/*
> + * calc_tpm2_event_size() - calculate the event size, where event
> + * is an entry in the TPM 2.0 event log. The event is of type Crypto
> + * Agile Log Entry Format as defined in TCG EFI Protocol Specification
> + * Family "2.0".
> +
> + * @event: event whose size is to be calculated.
> + * @event_header: the first event in the event log.
> + *
> + * Returns size of the event. If it is an invalid event, returns 0.
> + */
> +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;
> + void *marker_start;
> + u32 halg_size;
> + size_t size;
> + u16 halg;
> + int i;
> + int j;
> +
> + marker = event;
> + marker_start = marker;
> + marker = marker + sizeof(event->pcr_idx) + sizeof(event->event_type)
> + + sizeof(event->count);
> +
> + efispecid = (struct tcg_efi_specid_event *)event_header->event;
> +
> + /* Check if event is malformed. */
> + if (event->count > efispecid->num_algs)
> + return 0;
> +
> + for (i = 0; i < event->count; i++) {
> + halg_size = sizeof(event->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 +=
> + efispecid->digest_sizes[j].digest_size;
> + break;
> + }
> + }
> + /* Algorithm without known length. Such event is unparseable. */
> + if (j == efispecid->num_algs)
> + return 0;
> + }
> +
> + 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;
> +}
> +EXPORT_SYMBOL(calc_tpm2_event_size);
> --
> 2.14.4
>

2019-02-13 13:50:42

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 3/5] tpm: Use library version of calc_tpm2_event_size in sysfs code

On Mon, Feb 11, 2019 at 03:30:50PM +0100, [email protected] wrote:
> From: Bartosz Szczepanek <[email protected]>
>
> Expect negative values from calc_tpm2_event_size as error codes.
> Pass efispecid instead of event header to calc_tpm2_event_size.
>
> Also, include tpm library in the build.
>
> Signed-off-by: Bartosz Szczepanek <[email protected]>

Event log uses securityfs, not sysfs.

/Jarkko

2019-02-13 13:51:53

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH 4/5] efi/libstub/tpm: Retrieve TPM event log in 2.0 format

On Mon, Feb 11, 2019 at 03:30:51PM +0100, [email protected] wrote:
> From: Bartosz Szczepanek <[email protected]>
>
> Currently, the only way to get TPM 2.0 event log from firmware is to use
> device tree. Introduce efi_retrieve_tpm2_eventlog_2 function to enable
> retrieving it from EFI structures.
>
> Include lib/tpm.c into EFI stub to calculate event sizes using helper
> function.
>
> Signed-off-by: Bartosz Szczepanek <[email protected]>

Collides with Matthew's changes. I want to land those change first
because they are almost production ready.

Maybe you should consider reviewing those changes to make sure that
they make sense to you so that you can build these on top of after
these have landed.

There is not rush as I already made my v5.1 PR. These will be released
earliest in v5.2.

/Jarkko

2019-02-13 15:12:54

by Bartosz Szczepanek

[permalink] [raw]
Subject: Re: [PATCH 4/5] efi/libstub/tpm: Retrieve TPM event log in 2.0 format

On Wed, Feb 13, 2019 at 12:26 PM Jarkko Sakkinen
<[email protected]> wrote:
> Collides with Matthew's changes. I want to land those change first
> because they are almost production ready.
>
> Maybe you should consider reviewing those changes to make sure that
> they make sense to you so that you can build these on top of after
> these have landed.

Yeah, I think so. Actually, I wasn't aware of Matthew's efforts, as it
didn't appear on linux-efi mailing list. (On bad, I haven't checked
linux-integrity.)

At this point, I think it makes more sense to limit this patchset to
5/5 patch, which makes TPM event log initialized on ARM platforms.
Patches 1-4 introduce nothing more than Matthew already did, maybe
except putting calc_tpm2_event_size to a library instead of making it
inline. This function has already grown a bit so it may be a better
approach, but that's nothing to affect functionality.

I'll pull Matthew changes to my tree to confirm operation on ARM
platforms, if that works fine the only thing to merge would be 5/5 +
optionally the library change, if we reach agreement on that.

Bartosz

2019-02-13 15:13:23

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 4/5] efi/libstub/tpm: Retrieve TPM event log in 2.0 format

On Wed, 13 Feb 2019 at 15:21, Bartosz Szczepanek <[email protected]> wrote:
>
> On Wed, Feb 13, 2019 at 12:26 PM Jarkko Sakkinen
> <[email protected]> wrote:
> > Collides with Matthew's changes. I want to land those change first
> > because they are almost production ready.
> >
> > Maybe you should consider reviewing those changes to make sure that
> > they make sense to you so that you can build these on top of after
> > these have landed.
>
> Yeah, I think so. Actually, I wasn't aware of Matthew's efforts, as it
> didn't appear on linux-efi mailing list. (On bad, I haven't checked
> linux-integrity.)
>
> At this point, I think it makes more sense to limit this patchset to
> 5/5 patch, which makes TPM event log initialized on ARM platforms.
> Patches 1-4 introduce nothing more than Matthew already did, maybe
> except putting calc_tpm2_event_size to a library instead of making it
> inline. This function has already grown a bit so it may be a better
> approach, but that's nothing to affect functionality.
>
> I'll pull Matthew changes to my tree to confirm operation on ARM
> platforms, if that works fine the only thing to merge would be 5/5 +
> optionally the library change, if we reach agreement on that.
>

Sounds good, and yes, it would be good to cc patches that affect the
EFI subsystem to linux-efi as well.