linux-kernel - Re: [PATCH v3] tpm/eventlog: Don't abort tpm_read

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] [day] [month] [year] [list]

Message-ID: <Y+//+pFcdluslFJj@kernel.org>
Date:   Sat, 18 Feb 2023 00:30:18 +0200
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Morten Linderud <morten@...derud.pw>
Cc:     linux-kernel@...r.kernel.org, linux-integrity@...r.kernel.org,
        jgg@...pe.ca, peterhuewe@....de, Erkki Eilonen <erkki@...rmetal.eu>
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 <erkki@...rmetal.eu>
> Signed-off-by: Morten Linderud <morten@...derud.pw>
> 
> ---
> 
> 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 <jarkko@...nel.org>

I'll apply this soon, thanks.

BR, Jarkko