2023-02-15 09:44:26

by Morten Linderud

[permalink] [raw]
Subject: [PATCH v3] tpm/eventlog: Don't abort tpm_read_log on faulty ACPI address

tpm_read_log_acpi() should return -ENODEV when no eventlog from the ACPI
table is found. If the firmware vendor includes an invalid log address
we are unable to map from the ACPI memory and tpm_read_log() returns -EIO
which would abort discovery of the eventlog.

Change the return value from -EIO to -ENODEV when acpi_os_map_iomem()
fails to map the event log.

The following hardware was used to test this issue:
Framework Laptop (Pre-production)
BIOS: INSYDE Corp, Revision: 3.2
TPM Device: NTC, Firmware Revision: 7.2

Dump of the faulty ACPI TPM2 table:
[000h 0000 4] Signature : "TPM2" [Trusted Platform Module hardware interface Table]
[004h 0004 4] Table Length : 0000004C
[008h 0008 1] Revision : 04
[009h 0009 1] Checksum : 2B
[00Ah 0010 6] Oem ID : "INSYDE"
[010h 0016 8] Oem Table ID : "TGL-ULT"
[018h 0024 4] Oem Revision : 00000002
[01Ch 0028 4] Asl Compiler ID : "ACPI"
[020h 0032 4] Asl Compiler Revision : 00040000

[024h 0036 2] Platform Class : 0000
[026h 0038 2] Reserved : 0000
[028h 0040 8] Control Address : 0000000000000000
[030h 0048 4] Start Method : 06 [Memory Mapped I/O]

[034h 0052 12] Method Parameters : 00 00 00 00 00 00 00 00 00 00 00 00
[040h 0064 4] Minimum Log Length : 00010000
[044h 0068 8] Log Address : 000000004053D000

Fixes: 0cf577a03f21 ("tpm: Fix handling of missing event log")
Tested-By: Erkki Eilonen <[email protected]>
Signed-off-by: Morten Linderud <[email protected]>

---

v2: Tweak commit message and opt to return -ENODEV instead of loosening up the
if condition in tpm_read_log()

v3: Mention function name instead of "this" in the commit log. Added Tested-By
and Fixes tags

---
drivers/char/tpm/eventlog/acpi.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
index 0913d3eb8d51..cd266021d010 100644
--- a/drivers/char/tpm/eventlog/acpi.c
+++ b/drivers/char/tpm/eventlog/acpi.c
@@ -143,8 +143,12 @@ int tpm_read_log_acpi(struct tpm_chip *chip)

ret = -EIO;
virt = acpi_os_map_iomem(start, len);
- if (!virt)
+ if (!virt) {
+ dev_warn(&chip->dev, "%s: Failed to map ACPI memory\n", __func__);
+ /* try EFI log next */
+ ret = -ENODEV;
goto err;
+ }

memcpy_fromio(log->bios_event_log, virt, len);

--
2.39.1


2023-02-17 22:30:28

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v3] tpm/eventlog: Don't abort tpm_read_log on faulty ACPI address

On Wed, Feb 15, 2023 at 10:25:52AM +0100, Morten Linderud wrote:
> tpm_read_log_acpi() should return -ENODEV when no eventlog from the ACPI
> table is found. If the firmware vendor includes an invalid log address
> we are unable to map from the ACPI memory and tpm_read_log() returns -EIO
> which would abort discovery of the eventlog.
>
> Change the return value from -EIO to -ENODEV when acpi_os_map_iomem()
> fails to map the event log.
>
> The following hardware was used to test this issue:
> Framework Laptop (Pre-production)
> BIOS: INSYDE Corp, Revision: 3.2
> TPM Device: NTC, Firmware Revision: 7.2
>
> Dump of the faulty ACPI TPM2 table:
> [000h 0000 4] Signature : "TPM2" [Trusted Platform Module hardware interface Table]
> [004h 0004 4] Table Length : 0000004C
> [008h 0008 1] Revision : 04
> [009h 0009 1] Checksum : 2B
> [00Ah 0010 6] Oem ID : "INSYDE"
> [010h 0016 8] Oem Table ID : "TGL-ULT"
> [018h 0024 4] Oem Revision : 00000002
> [01Ch 0028 4] Asl Compiler ID : "ACPI"
> [020h 0032 4] Asl Compiler Revision : 00040000
>
> [024h 0036 2] Platform Class : 0000
> [026h 0038 2] Reserved : 0000
> [028h 0040 8] Control Address : 0000000000000000
> [030h 0048 4] Start Method : 06 [Memory Mapped I/O]
>
> [034h 0052 12] Method Parameters : 00 00 00 00 00 00 00 00 00 00 00 00
> [040h 0064 4] Minimum Log Length : 00010000
> [044h 0068 8] Log Address : 000000004053D000
>
> Fixes: 0cf577a03f21 ("tpm: Fix handling of missing event log")
> Tested-By: Erkki Eilonen <[email protected]>
> Signed-off-by: Morten Linderud <[email protected]>
>
> ---
>
> v2: Tweak commit message and opt to return -ENODEV instead of loosening up the
> if condition in tpm_read_log()
>
> v3: Mention function name instead of "this" in the commit log. Added Tested-By
> and Fixes tags
>
> ---
> drivers/char/tpm/eventlog/acpi.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c
> index 0913d3eb8d51..cd266021d010 100644
> --- a/drivers/char/tpm/eventlog/acpi.c
> +++ b/drivers/char/tpm/eventlog/acpi.c
> @@ -143,8 +143,12 @@ int tpm_read_log_acpi(struct tpm_chip *chip)
>
> ret = -EIO;
> virt = acpi_os_map_iomem(start, len);
> - if (!virt)
> + if (!virt) {
> + dev_warn(&chip->dev, "%s: Failed to map ACPI memory\n", __func__);
> + /* try EFI log next */
> + ret = -ENODEV;
> goto err;
> + }
>
> memcpy_fromio(log->bios_event_log, virt, len);
>
> --
> 2.39.1

Reviewed-by: Jarkko Sakkinen <[email protected]>

I'll apply this soon, thanks.

BR, Jarkko