diff mbox series

[v2,1/1,SRU,OEM-5.6] efi/tpm: Verify event log header before parsing

Message ID 20200826055929.122134-2-vicamo.yang@canonical.com
State New
Headers show
Series Fix system boot hang at efi_tpm_eventlog_init | expand

Commit Message

You-Sheng Yang Aug. 26, 2020, 5:59 a.m. UTC 
From: Fabian Vogt <fvogt@suse.de>

BugLink: https://bugs.launchpad.net/bugs/1892827

It is possible that the first event in the event log is not actually a
log header at all, but rather a normal event. This leads to the cast in
__calc_tpm2_event_size being an invalid conversion, which means that
the values read are effectively garbage. Depending on the first event's
contents, this leads either to apparently normal behaviour, a crash or
a freeze.

While this behaviour of the firmware is not in accordance with the
TCG Client EFI Specification, this happens on a Dell Precision 5510
with the TPM enabled but hidden from the OS ("TPM On" disabled, state
otherwise untouched). The EFI firmware claims that the TPM is present
and active and that it supports the TCG 2.0 event log format.

Fortunately, this can be worked around by simply checking the header
of the first event and the event log header signature itself.

Commit b4f1874c6216 ("tpm: check event log version before reading final
events") addressed a similar issue also found on Dell models.

Fixes: 6b0326190205 ("efi: Attempt to get the TCG2 event log in the boot stub")
Signed-off-by: Fabian Vogt <fvogt@suse.de>
Link: https://lore.kernel.org/r/1927248.evlx2EsYKh@linux-e202.suse.de
Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1165773
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
(cherry picked from commit 7dfc06a0f25b593a9f51992f540c0f80a57f3629)
Signed-off-by: You-Sheng Yang <vicamo.yang@canonical.com>
---
 include/linux/tpm_eventlog.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Thadeu Lima de Souza Cascardo Aug. 26, 2020, 10:19 a.m. UTC | #1 
On Wed, Aug 26, 2020 at 01:59:29PM +0800, You-Sheng Yang wrote:
> From: Fabian Vogt <fvogt@suse.de>
> 
> BugLink: https://bugs.launchpad.net/bugs/1892827
> 
> It is possible that the first event in the event log is not actually a
> log header at all, but rather a normal event. This leads to the cast in
> __calc_tpm2_event_size being an invalid conversion, which means that
> the values read are effectively garbage. Depending on the first event's
> contents, this leads either to apparently normal behaviour, a crash or
> a freeze.
> 
> While this behaviour of the firmware is not in accordance with the
> TCG Client EFI Specification, this happens on a Dell Precision 5510
> with the TPM enabled but hidden from the OS ("TPM On" disabled, state
> otherwise untouched). The EFI firmware claims that the TPM is present
> and active and that it supports the TCG 2.0 event log format.
> 
> Fortunately, this can be worked around by simply checking the header
> of the first event and the event log header signature itself.
> 
> Commit b4f1874c6216 ("tpm: check event log version before reading final
> events") addressed a similar issue also found on Dell models.
> 
> Fixes: 6b0326190205 ("efi: Attempt to get the TCG2 event log in the boot stub")
> Signed-off-by: Fabian Vogt <fvogt@suse.de>
> Link: https://lore.kernel.org/r/1927248.evlx2EsYKh@linux-e202.suse.de
> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1165773
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> (cherry picked from commit 7dfc06a0f25b593a9f51992f540c0f80a57f3629)
> Signed-off-by: You-Sheng Yang <vicamo.yang@canonical.com>


Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>

> ---
>  include/linux/tpm_eventlog.h | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
> index 131ea1bad458..eccfd3a4e4c8 100644
> --- a/include/linux/tpm_eventlog.h
> +++ b/include/linux/tpm_eventlog.h
> @@ -81,6 +81,8 @@ struct tcg_efi_specid_event_algs {
>  	u16 digest_size;
>  } __packed;
>  
> +#define TCG_SPECID_SIG "Spec ID Event03"
> +
>  struct tcg_efi_specid_event_head {
>  	u8 signature[16];
>  	u32 platform_class;
> @@ -171,6 +173,7 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
>  	int i;
>  	int j;
>  	u32 count, event_type;
> +	const u8 zero_digest[sizeof(event_header->digest)] = {0};
>  
>  	marker = event;
>  	marker_start = marker;
> @@ -198,10 +201,19 @@ static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
>  	count = READ_ONCE(event->count);
>  	event_type = READ_ONCE(event->event_type);
>  
> +	/* Verify that it's the log header */
> +	if (event_header->pcr_idx != 0 ||
> +	    event_header->event_type != NO_ACTION ||
> +	    memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
> +		size = 0;
> +		goto out;
> +	}
> +
>  	efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
>  
>  	/* Check if event is malformed. */
> -	if (count > efispecid->num_algs) {
> +	if (memcmp(efispecid->signature, TCG_SPECID_SIG,
> +		   sizeof(TCG_SPECID_SIG)) || count > efispecid->num_algs) {
>  		size = 0;
>  		goto out;
>  	}
> -- 
> 2.27.0
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 131ea1bad458..eccfd3a4e4c8 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -81,6 +81,8 @@  struct tcg_efi_specid_event_algs {
 	u16 digest_size;
 } __packed;
 
+#define TCG_SPECID_SIG "Spec ID Event03"
+
 struct tcg_efi_specid_event_head {
 	u8 signature[16];
 	u32 platform_class;
@@ -171,6 +173,7 @@  static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	int i;
 	int j;
 	u32 count, event_type;
+	const u8 zero_digest[sizeof(event_header->digest)] = {0};
 
 	marker = event;
 	marker_start = marker;
@@ -198,10 +201,19 @@  static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,
 	count = READ_ONCE(event->count);
 	event_type = READ_ONCE(event->event_type);
 
+	/* Verify that it's the log header */
+	if (event_header->pcr_idx != 0 ||
+	    event_header->event_type != NO_ACTION ||
+	    memcmp(event_header->digest, zero_digest, sizeof(zero_digest))) {
+		size = 0;
+		goto out;
+	}
+
 	efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
 
 	/* Check if event is malformed. */
-	if (count > efispecid->num_algs) {
+	if (memcmp(efispecid->signature, TCG_SPECID_SIG,
+		   sizeof(TCG_SPECID_SIG)) || count > efispecid->num_algs) {
 		size = 0;
 		goto out;
 	}