Message ID | 20200825083621.310249-2-vicamo.yang@canonical.com |
---|---|
State | New |
Headers | show |
Series | Fix system boot hang at efi_tpm_eventlog_init | expand |
On Tue, Aug 25, 2020 at 04:36:21PM +0800, You-Sheng Yang wrote: > From: Fabian Vogt <fvogt@suse.de> > > BugLink: https://bugs.launchpad.net/bugs/1892827 > > [ Upstream commit 7dfc06a0f25b593a9f51992f540c0f80a57f3629 ] Hi, Vicamo. You should have cherry-picked 7dfc06a0f25b593a9f51992f540c0f80a57f3629 instead of ea3cdcaa43b08c9a747639f545b0ed5cca47f87e. Can you send a v2? Cascardo. > > 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> > Signed-off-by: Sasha Levin <sashal@kernel.org> > (cherry picked from commit ea3cdcaa43b08c9a747639f545b0ed5cca47f87e) > Signed-off-by: You-Sheng Yang <vicamo.yang@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
Sorry, resent V2 at https://lists.ubuntu.com/archives/kernel-team/2020-August/112992.html On 2020-08-26 00:28, Thadeu Lima de Souza Cascardo wrote: > On Tue, Aug 25, 2020 at 04:36:21PM +0800, You-Sheng Yang wrote: >> From: Fabian Vogt <fvogt@suse.de> >> >> BugLink: https://bugs.launchpad.net/bugs/1892827 >> >> [ Upstream commit 7dfc06a0f25b593a9f51992f540c0f80a57f3629 ] > > Hi, Vicamo. > > You should have cherry-picked 7dfc06a0f25b593a9f51992f540c0f80a57f3629 instead > of ea3cdcaa43b08c9a747639f545b0ed5cca47f87e. > > Can you send a v2? > > Cascardo. > >> >> 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> >> Signed-off-by: Sasha Levin <sashal@kernel.org> >> (cherry picked from commit ea3cdcaa43b08c9a747639f545b0ed5cca47f87e) >> Signed-off-by: You-Sheng Yang <vicamo.yang@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 --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; }