From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750846AbdAXKLx (ORCPT ); Tue, 24 Jan 2017 05:11:53 -0500 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:38359 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750734AbdAXKLv (ORCPT ); Tue, 24 Jan 2017 05:11:51 -0500 Subject: Re: [PATCH v9 2/2] tpm: add securityfs support for TPM 2.0 firmware event log To: Jarkko Sakkinen References: <1485156387-7735-1-git-send-email-nayna@linux.vnet.ibm.com> <1485156387-7735-3-git-send-email-nayna@linux.vnet.ibm.com> <20170123151350.staamgwzcbcfg6b6@intel.com> Cc: tpmdd-devel@lists.sourceforge.net, peterhuewe@gmx.de, tpmdd@selhorst.net, jgunthorpe@obsidianresearch.com, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org From: Nayna Date: Tue, 24 Jan 2017 15:41:22 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20170123151350.staamgwzcbcfg6b6@intel.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17012410-8235-0000-0000-00000AC88F05 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00006489; HX=3.00000240; KW=3.00000007; PH=3.00000004; SC=3.00000200; SDB=6.00811949; UDB=6.00395924; IPR=6.00589365; BA=6.00005084; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00014022; XFM=3.00000011; UTC=2017-01-24 10:11:48 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17012410-8236-0000-0000-000038FE28A1 Message-Id: <5887284A.20809@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-01-24_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1701240082 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/23/2017 08:43 PM, Jarkko Sakkinen wrote: > On Mon, Jan 23, 2017 at 02:26:27AM -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 >> --- >> drivers/char/tpm/Makefile | 2 +- >> .../char/tpm/{tpm_eventlog.c => tpm1_eventlog.c} | 35 ++-- >> drivers/char/tpm/tpm2_eventlog.c | 203 +++++++++++++++++++++ >> drivers/char/tpm/tpm_acpi.c | 3 + >> drivers/char/tpm/tpm_eventlog.h | 59 ++++++ >> 5 files changed, 287 insertions(+), 15 deletions(-) >> rename drivers/char/tpm/{tpm_eventlog.c => tpm1_eventlog.c} (95%) >> create mode 100644 drivers/char/tpm/tpm2_eventlog.c >> >> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile >> index a05b1eb..3d386a8 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 >> + tpm1_eventlog.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/tpm_eventlog.c b/drivers/char/tpm/tpm1_eventlog.c >> similarity index 95% >> rename from drivers/char/tpm/tpm_eventlog.c >> rename to drivers/char/tpm/tpm1_eventlog.c >> index 11bb113..9a8605e 100644 >> --- a/drivers/char/tpm/tpm_eventlog.c >> +++ b/drivers/char/tpm/tpm1_eventlog.c >> @@ -390,9 +390,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; >> @@ -407,7 +404,13 @@ 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", >> @@ -418,17 +421,21 @@ int tpm_bios_log_setup(struct tpm_chip *chip) >> goto err; >> cnt++; >> >> - chip->ascii_log_seqops.chip = chip; >> - chip->ascii_log_seqops.seqops = &tpm_ascii_b_measurements_seqops; >> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { >> >> - 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_ERR(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_ERR(chip->bios_dir[cnt])) >> + goto err; >> + cnt++; >> + } >> >> return 0; >> >> diff --git a/drivers/char/tpm/tpm2_eventlog.c b/drivers/char/tpm/tpm2_eventlog.c >> new file mode 100644 >> index 0000000..513897c >> --- /dev/null >> +++ b/drivers/char/tpm/tpm2_eventlog.c >> @@ -0,0 +1,203 @@ >> +/* >> + * Copyright (C) 2016 IBM Corporation >> + * >> + * Authors: >> + * Nayna Jain >> + * >> + * Access to TPM 2.0 event log as written by Firmware. >> + * It assumes that writer of event log has followed TCG Specification >> + * for Family "2.0" and written the event 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 >> +#include >> +#include >> +#include >> +#include >> + >> +#include "tpm.h" >> +#include "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. >> + */ >> +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; >> + >> + for (i = 0; (i < event->count) && (i < TPM2_ACTIVE_PCR_BANKS); >> + 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 = 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; >> + size_t size; >> + int i; >> + >> + 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; >> + } >> + >> + 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; >> + size_t event_size; >> + void *marker; >> + >> + 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; >> + >> + 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_acpi.c b/drivers/char/tpm/tpm_acpi.c >> index b7718c9..169edf3 100644 >> --- a/drivers/char/tpm/tpm_acpi.c >> +++ b/drivers/char/tpm/tpm_acpi.c >> @@ -54,6 +54,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip) >> u64 len, start; >> struct tpm_bios_log *log; >> >> + if (chip->flags & TPM_CHIP_FLAG_TPM2) >> + return -ENODEV; >> + >> log = &chip->log; >> >> /* Unfortuntely ACPI does not associate the event log with a specific >> diff --git a/drivers/char/tpm/tpm_eventlog.h b/drivers/char/tpm/tpm_eventlog.h >> index 1660d74..8d43328 100644 >> --- a/drivers/char/tpm/tpm_eventlog.h >> +++ b/drivers/char/tpm/tpm_eventlog.h >> @@ -2,9 +2,12 @@ >> #ifndef __TPM_EVENTLOG_H__ >> #define __TPM_EVENTLOG_H__ >> >> +#include >> + >> #define TCG_EVENT_NAME_LEN_MAX 255 >> #define MAX_TEXT_EVENT 1000 /* Max event string length */ >> #define ACPI_TCPA_SIG "TCPA" /* 0x41504354 /'TCPA' */ >> +#define TPM2_ACTIVE_PCR_BANKS 3 >> >> #ifdef CONFIG_PPC64 >> #define do_endian_conversion(x) be32_to_cpu(x) >> @@ -73,6 +76,62 @@ enum tcpa_pc_event_ids { >> HOST_TABLE_OF_DEVICES, >> }; >> >> +/* >> + * All the structures related to TPM 2.0 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 TPM 2.0 are defined in document >> + * Trusted Platform Module Library Part 2: Structures, Family "2.0". >> + */ > > I think I asked you remove this comment before. Only useful part is the > link. Everything else in it serves no purpose of any kind. > > If you really want to comment in some way this is a version that > contains the useful information from above: > > /* Even Log structures: > * http://www.trustedcomputinggroup.org/tcg-efi-protocol-specification/ > */ > > Because of this you don't need to send anything. I'm just complaining > because it is still there :-) Yeah..Sure.. Sorry, I guess I understood part of your previous comment. I have removed only the single line comments but kept this. Thanks!! Thanks & Regards, - Nayna > >> + >> +struct tcg_efi_specid_event_algs { >> + u16 alg_id; >> + u16 digest_size; >> +} __packed; >> + >> +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[TPM2_ACTIVE_PCR_BANKS]; >> + u8 vendor_info_size; >> + u8 vendor_info[0]; >> +} __packed; >> + >> +struct tcg_pcr_event { >> + u32 pcr_idx; >> + u32 event_type; >> + u8 digest[20]; >> + u32 event_size; >> + u8 event[0]; >> +} __packed; >> + >> +struct tpm2_digest { >> + u16 alg_id; >> + u8 digest[SHA512_DIGEST_SIZE]; >> +} __packed; >> + >> +struct tcg_event_field { >> + u32 event_size; >> + u8 event[0]; >> +} __packed; >> + >> +struct tcg_pcr_event2 { >> + u32 pcr_idx; >> + u32 event_type; >> + u32 count; >> + struct tpm2_digest digests[TPM2_ACTIVE_PCR_BANKS]; >> + struct tcg_event_field event; >> +} __packed; >> + >> +extern const struct seq_operations tpm2_binary_b_measurements_seqops; >> + >> #if defined(CONFIG_ACPI) >> int tpm_read_log_acpi(struct tpm_chip *chip); >> #else >> -- >> 2.5.0 >> > > I guess that this is OK but since the code isn't peer tested I cannot > apply. > > /Jarkko >