linux-kernel - Re: [PATCH v2 3/3] tpm: Add support for ACPI logs from TPM2 ACPI table

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Mar 2020 22:54:16 +0300
From:   Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>
To:     Stefan Berger <stefanb@...ux.vnet.ibm.com>
Cc:     linux-integrity@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org, linux-security-module@...r.kernel.org,
        Stefan Berger <stefanb@...ux.ibm.com>
Subject: Re: [PATCH v2 3/3] tpm: Add support for ACPI logs from TPM2 ACPI
 table

On Mon, Mar 30, 2020 at 11:15:36AM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@...ux.ibm.com>
> 
> In case of a TPM2 search for a TPM2 ACPI table and if found use it to
> find start and length of the log area.
> 
> Signed-off-by: Stefan Berger <stefanb@...ux.ibm.com>

Please use "tpm/eventlog".

There's a typo in the short summary: I guess you meant event log when
you speak of "ACPI log".

Also, I'm not getting the long description e.g. "TPM2 search" is an
alien term.

> ---
>  drivers/char/tpm/eventlog/acpi.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
> index 1ef2267c9378..2447f21e4a23 100644
> --- a/drivers/char/tpm/eventlog/acpi.c
> +++ b/drivers/char/tpm/eventlog/acpi.c
> @@ -49,6 +49,8 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  	void __iomem *virt;
>  	u64 len, start;
>  	struct tpm_bios_log *log;
> +	struct acpi_table_tpm2 *tbl;
> +	int format;
>  
>  	log = &chip->log;
>  
> @@ -59,7 +61,15 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  		return -ENODEV;
>  
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> -		return -ENODEV;
> +		status = acpi_get_table("TPM2", 1,
> +					(struct acpi_table_header **)&tbl);
> +		if (ACPI_FAILURE(status))
> +			return -ENODEV;
> +		if (tbl->header.length < sizeof(*tbl))
> +			return -ENODEV;
> +		len = tbl->log_area_minimum_length;
> +		start = tbl->log_area_start_address;
> +		format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2;
>  	} else {
>  		/* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
>  		status = acpi_get_table(ACPI_SIG_TCPA, 1,
> @@ -79,6 +89,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  			start = buff->client.log_start_addr;
>  			break;
>  		}
> +		format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
>  	}
>  	if (!len) {
>  		dev_warn(&chip->dev, "%s: TCPA log area empty\n", __func__);
> @@ -99,7 +110,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>  	memcpy_fromio(log->bios_event_log, virt, len);
>  
>  	acpi_os_unmap_iomem(virt, len);
> -	return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2;
> +	return format;
>  
>  err:
>  	kfree(log->bios_event_log);
> -- 
> 2.14.5
> 

The code change looks fine but need a review after squashed with 2/3.

/Jarkko