| Message ID | 20200722182136.150132-1-alex.hung@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | tpm2: table size can be 76 when "LAML" & "LASA" are used | expand |
On 7/23/20 2:21 AM, Alex Hung wrote: > TCG ACPI Specification released in 2017 added two fields, "Log Area > Minimum Length" and "Log Area Start Address". When they are used, the > table size will be 76. > > Buglink: https://bugs.launchpad.net/bugs/1888189 > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/tpm2/tpm2.c | 35 +++++++++++++++++++---------------- > 1 file changed, 19 insertions(+), 16 deletions(-) > > diff --git a/src/acpi/tpm2/tpm2.c b/src/acpi/tpm2/tpm2.c > index bca5cb8a..56b35a1c 100644 > --- a/src/acpi/tpm2/tpm2.c > +++ b/src/acpi/tpm2/tpm2.c > @@ -72,22 +72,25 @@ static int tpm2_test1(fwts_framework *fw) > tpm2->start_method); > } > > - if (tpm2->start_method == 2 && table->length != sizeof(fwts_acpi_table_tpm2) + 4) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "TPM2BadPlatformParameters", > - "Table length must be 0x%" PRIx32 " if Start method equals 2, got 0x%" PRIx32, > - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 4, > - (uint32_t) table->length); > - } > - > - if (tpm2->start_method == 11 && table->length < sizeof(fwts_acpi_table_tpm2) + 12) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "TPM2BadPlatformParameters", > - "Table length must be atleast 0x%" PRIx32 " if Start method equals 11, got 0x%" PRIx32, > - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 12, > - (uint32_t) table->length); > + /* When TPM2 includes fields "LAML" & "LASA", table size will be fixed to 76. */ > + if (table->length != 76) { From the Spec, we shouldn't check the fixed size 76, the Start Method Specific Parameters is variable, and both LAML and LASA are optional. I think we should check the sizes of Start Method Specific Parameters, LAML and LASA included or not. Cheers, Ivan > + if (tpm2->start_method == 2 && table->length != sizeof(fwts_acpi_table_tpm2) + 4) { > + passed = false; > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "TPM2BadPlatformParameters", > + "Table length must be 0x%" PRIx32 " if Start Method equals 2, " > + "got 0x%" PRIx32, (uint32_t) sizeof(fwts_acpi_table_tpm2) + 4, > + (uint32_t) table->length); > + } > + > + if (tpm2->start_method == 11 && table->length < sizeof(fwts_acpi_table_tpm2) + 12) { > + passed = false; > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "TPM2BadPlatformParameters", > + "Table length must be at least 0x%" PRIx32 " if Start Method equals 11, " > + "got 0x%" PRIx32, (uint32_t) sizeof(fwts_acpi_table_tpm2) + 12, > + (uint32_t) table->length); > + } > } > > if (passed) >
On Wed, Jul 22, 2020 at 8:21 PM ivanhu <ivan.hu@canonical.com> wrote: > > > On 7/23/20 2:21 AM, Alex Hung wrote: > > TCG ACPI Specification released in 2017 added two fields, "Log Area > > Minimum Length" and "Log Area Start Address". When they are used, the > > table size will be 76. > > > > Buglink: https://bugs.launchpad.net/bugs/1888189 > > > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > > --- > > src/acpi/tpm2/tpm2.c | 35 +++++++++++++++++++---------------- > > 1 file changed, 19 insertions(+), 16 deletions(-) > > > > diff --git a/src/acpi/tpm2/tpm2.c b/src/acpi/tpm2/tpm2.c > > index bca5cb8a..56b35a1c 100644 > > --- a/src/acpi/tpm2/tpm2.c > > +++ b/src/acpi/tpm2/tpm2.c > > @@ -72,22 +72,25 @@ static int tpm2_test1(fwts_framework *fw) > > tpm2->start_method); > > } > > > > - if (tpm2->start_method == 2 && table->length != > sizeof(fwts_acpi_table_tpm2) + 4) { > > - passed = false; > > - fwts_failed(fw, LOG_LEVEL_HIGH, > > - "TPM2BadPlatformParameters", > > - "Table length must be 0x%" PRIx32 " if Start > method equals 2, got 0x%" PRIx32, > > - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 4, > > - (uint32_t) table->length); > > - } > > - > > - if (tpm2->start_method == 11 && table->length < > sizeof(fwts_acpi_table_tpm2) + 12) { > > - passed = false; > > - fwts_failed(fw, LOG_LEVEL_HIGH, > > - "TPM2BadPlatformParameters", > > - "Table length must be atleast 0x%" PRIx32 " if > Start method equals 11, got 0x%" PRIx32, > > - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 12, > > - (uint32_t) table->length); > > + /* When TPM2 includes fields "LAML" & "LASA", table size will be > fixed to 76. */ > > + if (table->length != 76) { > > From the Spec, we shouldn't check the fixed size 76, the Start Method > Specific Parameters is variable, and both LAML and LASA are optional. > > I think we should check the sizes of Start Method Specific Parameters, > LAML and LASA included or not. > That's not entirely correct. The document ( https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf) states the length as below: ======================== The length of this table starting from the Signature field up to and including the Start Method specific parameters field. (52 + size of Start Method specific parameters) If the table contains LAML and LASA field, Length is 76. ======================== It also states "Start Method Specific Parameters" can be varying or fixed in size ======================== The content of the Start Method specific parameters is determined by the Start Method.... If LAML and LASA are present, then this field is 12 bytes in size. ======================== I would interpret that the length can be 76 or variable (52 + Specific Parameters). This is not depending on Start Method but on the availability of LAML and LASA. Note: This definition seems to be changed from 2013, but the table revision did not change. > > Cheers, > Ivan > > > + if (tpm2->start_method == 2 && table->length != > sizeof(fwts_acpi_table_tpm2) + 4) { > > + passed = false; > > + fwts_failed(fw, LOG_LEVEL_HIGH, > > + "TPM2BadPlatformParameters", > > + "Table length must be 0x%" PRIx32 " if > Start Method equals 2, " > > + "got 0x%" PRIx32, (uint32_t) > sizeof(fwts_acpi_table_tpm2) + 4, > > + (uint32_t) table->length); > > + } > > + > > + if (tpm2->start_method == 11 && table->length < > sizeof(fwts_acpi_table_tpm2) + 12) { > > + passed = false; > > + fwts_failed(fw, LOG_LEVEL_HIGH, > > + "TPM2BadPlatformParameters", > > + "Table length must be at least 0x%" PRIx32 > " if Start Method equals 11, " > > + "got 0x%" PRIx32, (uint32_t) > sizeof(fwts_acpi_table_tpm2) + 12, > > + (uint32_t) table->length); > > + } > > } > > > > if (passed) > > >
Thanks for clarification. Acked-by: Ivan Hu <ivan.hu@canonical.com> On 7/23/20 12:13 PM, Alex Hung wrote: > > > On Wed, Jul 22, 2020 at 8:21 PM ivanhu <ivan.hu@canonical.com > <mailto:ivan.hu@canonical.com>> wrote: > > > > On 7/23/20 2:21 AM, Alex Hung wrote: > > TCG ACPI Specification released in 2017 added two fields, "Log Area > > Minimum Length" and "Log Area Start Address". When they are > used, the > > table size will be 76. > > > > Buglink: https://bugs.launchpad.net/bugs/1888189 > > > > Signed-off-by: Alex Hung <alex.hung@canonical.com > <mailto:alex.hung@canonical.com>> > > --- > > src/acpi/tpm2/tpm2.c | 35 +++++++++++++++++++---------------- > > 1 file changed, 19 insertions(+), 16 deletions(-) > > > > diff --git a/src/acpi/tpm2/tpm2.c b/src/acpi/tpm2/tpm2.c > > index bca5cb8a..56b35a1c 100644 > > --- a/src/acpi/tpm2/tpm2.c > > +++ b/src/acpi/tpm2/tpm2.c > > @@ -72,22 +72,25 @@ static int tpm2_test1(fwts_framework *fw) > > tpm2->start_method); > > } > > > > - if (tpm2->start_method == 2 && table->length != > sizeof(fwts_acpi_table_tpm2) + 4) { > > - passed = false; > > - fwts_failed(fw, LOG_LEVEL_HIGH, > > - "TPM2BadPlatformParameters", > > - "Table length must be 0x%" PRIx32 " if > Start method equals 2, got 0x%" PRIx32, > > - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 4, > > - (uint32_t) table->length); > > - } > > - > > - if (tpm2->start_method == 11 && table->length < > sizeof(fwts_acpi_table_tpm2) + 12) { > > - passed = false; > > - fwts_failed(fw, LOG_LEVEL_HIGH, > > - "TPM2BadPlatformParameters", > > - "Table length must be atleast 0x%" PRIx32 > " if Start method equals 11, got 0x%" PRIx32, > > - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 12, > > - (uint32_t) table->length); > > + /* When TPM2 includes fields "LAML" & "LASA", table size > will be fixed to 76. */ > > + if (table->length != 76) { > > From the Spec, we shouldn't check the fixed size 76, the Start Method > Specific Parameters is variable, and both LAML and LASA are optional. > > I think we should check the sizes of Start Method Specific Parameters, > LAML and LASA included or not. > > > That's not entirely correct. > > The document > (https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf) > states the length as below: > ======================== > The length of this table starting from the Signature field up to > and including the Start Method specific parameters field. (52 > + size of Start Method specific parameters) > If the table contains LAML and LASA field, Length is 76. > ======================== > > It also states "Start Method Specific Parameters" can be varying or > fixed in size > ======================== > The content of the Start Method specific parameters is > determined by the Start Method.... > > If LAML and LASA are present, then this field is 12 bytes in size. > ======================== > > I would interpret that the length can be 76 or variable (52 + Specific > Parameters). This is not depending on Start Method but on the > availability of LAML and LASA. > > Note: This definition seems to be changed from 2013, but the table > revision did not change. > > > > Cheers, > Ivan > > > + if (tpm2->start_method == 2 && table->length != > sizeof(fwts_acpi_table_tpm2) + 4) { > > + passed = false; > > + fwts_failed(fw, LOG_LEVEL_HIGH, > > + "TPM2BadPlatformParameters", > > + "Table length must be 0x%" PRIx32 > " if Start Method equals 2, " > > + "got 0x%" PRIx32, (uint32_t) > sizeof(fwts_acpi_table_tpm2) + 4, > > + (uint32_t) table->length); > > + } > > + > > + if (tpm2->start_method == 11 && table->length < > sizeof(fwts_acpi_table_tpm2) + 12) { > > + passed = false; > > + fwts_failed(fw, LOG_LEVEL_HIGH, > > + "TPM2BadPlatformParameters", > > + "Table length must be at least > 0x%" PRIx32 " if Start Method equals 11, " > > + "got 0x%" PRIx32, (uint32_t) > sizeof(fwts_acpi_table_tpm2) + 12, > > + (uint32_t) table->length); > > + } > > } > > > > if (passed) > > > > > > -- > Cheers, > Alex Hung
On 22/07/2020 19:21, Alex Hung wrote: > TCG ACPI Specification released in 2017 added two fields, "Log Area > Minimum Length" and "Log Area Start Address". When they are used, the > table size will be 76. > > Buglink: https://bugs.launchpad.net/bugs/1888189 > > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- > src/acpi/tpm2/tpm2.c | 35 +++++++++++++++++++---------------- > 1 file changed, 19 insertions(+), 16 deletions(-) > > diff --git a/src/acpi/tpm2/tpm2.c b/src/acpi/tpm2/tpm2.c > index bca5cb8a..56b35a1c 100644 > --- a/src/acpi/tpm2/tpm2.c > +++ b/src/acpi/tpm2/tpm2.c > @@ -72,22 +72,25 @@ static int tpm2_test1(fwts_framework *fw) > tpm2->start_method); > } > > - if (tpm2->start_method == 2 && table->length != sizeof(fwts_acpi_table_tpm2) + 4) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "TPM2BadPlatformParameters", > - "Table length must be 0x%" PRIx32 " if Start method equals 2, got 0x%" PRIx32, > - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 4, > - (uint32_t) table->length); > - } > - > - if (tpm2->start_method == 11 && table->length < sizeof(fwts_acpi_table_tpm2) + 12) { > - passed = false; > - fwts_failed(fw, LOG_LEVEL_HIGH, > - "TPM2BadPlatformParameters", > - "Table length must be atleast 0x%" PRIx32 " if Start method equals 11, got 0x%" PRIx32, > - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 12, > - (uint32_t) table->length); > + /* When TPM2 includes fields "LAML" & "LASA", table size will be fixed to 76. */ > + if (table->length != 76) { > + if (tpm2->start_method == 2 && table->length != sizeof(fwts_acpi_table_tpm2) + 4) { > + passed = false; > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "TPM2BadPlatformParameters", > + "Table length must be 0x%" PRIx32 " if Start Method equals 2, " > + "got 0x%" PRIx32, (uint32_t) sizeof(fwts_acpi_table_tpm2) + 4, > + (uint32_t) table->length); > + } > + > + if (tpm2->start_method == 11 && table->length < sizeof(fwts_acpi_table_tpm2) + 12) { > + passed = false; > + fwts_failed(fw, LOG_LEVEL_HIGH, > + "TPM2BadPlatformParameters", > + "Table length must be at least 0x%" PRIx32 " if Start Method equals 11, " > + "got 0x%" PRIx32, (uint32_t) sizeof(fwts_acpi_table_tpm2) + 12, > + (uint32_t) table->length); > + } > } > > if (passed) > Acked-by: Colin Ian King <colin.king@canonical.com>
diff --git a/src/acpi/tpm2/tpm2.c b/src/acpi/tpm2/tpm2.c index bca5cb8a..56b35a1c 100644 --- a/src/acpi/tpm2/tpm2.c +++ b/src/acpi/tpm2/tpm2.c @@ -72,22 +72,25 @@ static int tpm2_test1(fwts_framework *fw) tpm2->start_method); } - if (tpm2->start_method == 2 && table->length != sizeof(fwts_acpi_table_tpm2) + 4) { - passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "TPM2BadPlatformParameters", - "Table length must be 0x%" PRIx32 " if Start method equals 2, got 0x%" PRIx32, - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 4, - (uint32_t) table->length); - } - - if (tpm2->start_method == 11 && table->length < sizeof(fwts_acpi_table_tpm2) + 12) { - passed = false; - fwts_failed(fw, LOG_LEVEL_HIGH, - "TPM2BadPlatformParameters", - "Table length must be atleast 0x%" PRIx32 " if Start method equals 11, got 0x%" PRIx32, - (uint32_t) sizeof(fwts_acpi_table_tpm2) + 12, - (uint32_t) table->length); + /* When TPM2 includes fields "LAML" & "LASA", table size will be fixed to 76. */ + if (table->length != 76) { + if (tpm2->start_method == 2 && table->length != sizeof(fwts_acpi_table_tpm2) + 4) { + passed = false; + fwts_failed(fw, LOG_LEVEL_HIGH, + "TPM2BadPlatformParameters", + "Table length must be 0x%" PRIx32 " if Start Method equals 2, " + "got 0x%" PRIx32, (uint32_t) sizeof(fwts_acpi_table_tpm2) + 4, + (uint32_t) table->length); + } + + if (tpm2->start_method == 11 && table->length < sizeof(fwts_acpi_table_tpm2) + 12) { + passed = false; + fwts_failed(fw, LOG_LEVEL_HIGH, + "TPM2BadPlatformParameters", + "Table length must be at least 0x%" PRIx32 " if Start Method equals 11, " + "got 0x%" PRIx32, (uint32_t) sizeof(fwts_acpi_table_tpm2) + 12, + (uint32_t) table->length); + } } if (passed)
TCG ACPI Specification released in 2017 added two fields, "Log Area Minimum Length" and "Log Area Start Address". When they are used, the table size will be 76. Buglink: https://bugs.launchpad.net/bugs/1888189 Signed-off-by: Alex Hung <alex.hung@canonical.com> --- src/acpi/tpm2/tpm2.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)