Skip to contentSkip to conversation

tpm: verify that it's actually an event log header before parsing

PATCH Applied Fabian Vogt 3 participants 3 messages Jun 15, 2020 – Jun 19, 2020

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 tpm: verify that it's actually an event log header before parsing

It's possible that the first event in the 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 claims that the TPM is present and active and 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")

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1165773

---
v2: Drop unneeded READ_ONCE around event_header derefs.

 include/linux/tpm_eventlog.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
include/linux/tpm_eventlog.h +13 -1
index 4f8c90c93c29..64356b199e94 100644
@@ -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;
 	}
Signature · 1 line
2.25.1
maintainer Applied
#1 lore raw
Quoted 17 lines · Fabian Vogt

It's possible that the first event in the 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 claims that the TPM is present and active and 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.

Quoted 6 lines

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")

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1165773

Queued in efi/urgent, thanks.

Quoted 54 lines

---
v2: Drop unneeded READ_ONCE around event_header derefs.

 include/linux/tpm_eventlog.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
include/linux/tpm_eventlog.h +13 -2
index 4f8c90c93c29..64356b199e94 100644
@@ -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.25.1

merge bot Applied
#2 lore raw

[tip: efi/urgent] efi/tpm: Verify event log header before parsing

In reply to Fabian Vogt
The following commit has been merged into the efi/urgent branch of tip:

The following commit has been merged into the efi/urgent branch of tip:

Commit-ID:     7dfc06a0f25b593a9f51992f540c0f80a57f3629
Gitweb:        https://git.kernel.org/tip/7dfc06a0f25b593a9f51992f540c0f80a57f3629
Author:        Fabian Vogt <[email protected]>
AuthorDate:    Mon, 15 Jun 2020 09:16:36 +02:00
Committer:     Ard Biesheuvel <[email protected]>
CommitterDate: Mon, 15 Jun 2020 14:37:02 +02:00

efi/tpm: Verify event log header before parsing

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")

Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1165773

---
 include/linux/tpm_eventlog.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
include/linux/tpm_eventlog.h +13 -1
index 4f8c90c..64356b1 100644
@@ -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;
 	}