Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
From: Stefan Berger <stefanb@linux.ibm.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Stefan Berger <stefanb@linux.vnet.ibm.com>,
	Jiandi An <anjiandi@codeaurora.org>
Cc: linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-acpi@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>
Subject: Re: [PATCH v5 1/2] acpi: Extend TPM2 ACPI table with missing log fields
Date: Wed, 24 Jun 2020 20:38:25 -0400
Message-ID: <9d94c704-5774-ceeb-e4f3-010f74ffe37b@linux.ibm.com> (raw)
In-Reply-To: <20200625000021.GC21758@linux.intel.com>

On 6/24/20 8:00 PM, Jarkko Sakkinen wrote:
> On Tue, Jun 23, 2020 at 08:06:35AM -0400, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Recent extensions of the TPM2 ACPI table added 3 more fields
>> including 12 bytes of start method specific parameters and Log Area
>> Minimum Length (u32) and Log Area Start Address (u64). So, we extend
>> the existing structure with these fields to allow non-UEFI systems
>> to access the TPM2's log.
>>
>> The specification that has the new fields is the following:
>>    TCG ACPI Specification
>>    Family "1.2" and "2.0"
>>    Version 1.2, Revision 8
>>
>> Adapt all existing table size calculations to use
>> offsetof(struct acpi_table_tpm2, start_method_specific)
>> [where start_method_specific is a newly added field]
>> rather than sizeof(struct acpi_table_tpm2) so that the addition
>> of the new fields does not affect current systems that may not
>> have them.
>>
> I found at least one regression from this patch. Please remove my
> reviewed-by comment form the next version.
>
> Should have:
>
>    Link: https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf
>
> Please, add this.
>
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> Cc: linux-acpi@vger.kernel.org
>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>> ---
>>   drivers/char/tpm/tpm_crb.c | 13 ++++++++++---
>>   drivers/char/tpm/tpm_tis.c |  4 +++-
>>   include/acpi/actbl3.h      |  5 +++--
>>   3 files changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
>> index a9dcf31eadd2..0565aa5482f9 100644
>> --- a/drivers/char/tpm/tpm_crb.c
>> +++ b/drivers/char/tpm/tpm_crb.c
>> @@ -669,7 +669,9 @@ static int crb_acpi_add(struct acpi_device *device)
>>   
>>   	status = acpi_get_table(ACPI_SIG_TPM2, 1,
>>   				(struct acpi_table_header **) &buf);
>> -	if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
>> +	if (ACPI_FAILURE(status) || buf->header.length <
>> +			offsetof(struct acpi_table_tpm2,
>> +				 start_method_specific)) {
>>   		dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
>>   		return -EINVAL;
>>   	}
>> @@ -684,14 +686,19 @@ static int crb_acpi_add(struct acpi_device *device)
>>   		return -ENOMEM;
>>   
>>   	if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_ARM_SMC) {
>> -		if (buf->header.length < (sizeof(*buf) + sizeof(*crb_smc))) {
>> +		if (buf->header.length <
>> +			(offsetof(struct acpi_table_tpm2,
>> +				  start_method_specific) +
> Should be
>
>    offsetof(struct acpti_table_tpm2, log_area_minimum_length)


The old code had sizeof(*buf) with buf being 'struct acpi_table_tpm2' 
and that was equivalent to offsetof(struct acpi_table_tpm2, 
start_method_specific) since 'start_method_specific' is the first new 
field that we are adding right here. Also see 3rd paragraph in the patch 
description. The replacement rule described there should apply to all 
sizeof() calculations on 'struct acpi_table_tpm2.'

I'll address the other issues.


    Stefan



  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23 12:06 [PATCH v5 0/2] tpm2: Make TPM2 logs accessible for non-UEFI firmware Stefan Berger
2020-06-23 12:06 ` [PATCH v5 1/2] acpi: Extend TPM2 ACPI table with missing log fields Stefan Berger
2020-06-25  0:00   ` Jarkko Sakkinen
2020-06-25  0:38     ` Stefan Berger [this message]
2020-06-25  2:34       ` Jarkko Sakkinen
2020-06-25  2:52         ` Jarkko Sakkinen
2020-06-25  2:54           ` Jarkko Sakkinen
2020-06-25 12:10         ` Stefan Berger
2020-06-23 12:06 ` [PATCH v5 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table Stefan Berger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9d94c704-5774-ceeb-e4f3-010f74ffe37b@linux.ibm.com \
    --to=stefanb@linux.ibm.com \
    --cc=anjiandi@codeaurora.org \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=stefanb@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org
	public-inbox-index linux-integrity

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git