From: Ard Biesheuvel <ardb@kernel.org>
To: Fabian Vogt <fvogt@suse.de>
Cc: linux-efi <linux-efi@vger.kernel.org>,
"Lee, Chun-Yi" <jlee@suse.com>,
"Loïc Yhuel" <loic.yhuel@gmail.com>,
"Matthew Garrett" <matthewgarrett@google.com>,
"Jarkko Sakkinen" <jarkko.sakkinen@linux.intel.com>
Subject: Re: [PATCH] tpm: verify that it's actually an event log header before parsing
Date: Fri, 12 Jun 2020 13:01:33 +0200 [thread overview]
Message-ID: <CAMj1kXHErV-tx+8GvvLmb-L0wXq1cVwzY+OEr4PEHDZWrB7Dpg@mail.gmail.com> (raw)
In-Reply-To: <1894249.ujS34B1uSo@linux-e202.suse.de>
On Fri, 5 Jun 2020 at 16:22, Fabian Vogt <fvogt@suse.de> wrote:
>
> 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
> Signed-off-by: Fabian Vogt <fvogt@suse.de>
> ---
> 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 4f8c90c93c29..b913faeedcb5 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 (READ_ONCE(event_header->pcr_idx) != 0 ||
> + READ_ONCE(event_header->event_type) != NO_ACTION ||
Is the READ_ONCE() necessary here?
> + 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) {
So is it guaranteed that every well formed event starts with TCG_SPECID_SIG?
> size = 0;
> goto out;
> }
> --
> 2.25.1
>
>
>
>
next prev parent reply other threads:[~2020-06-12 11:01 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-05 14:21 [PATCH] tpm: verify that it's actually an event log header before parsing Fabian Vogt
2020-06-12 11:01 ` Ard Biesheuvel [this message]
2020-06-12 11:58 ` Fabian Vogt
2020-06-12 12:16 ` Fabian Vogt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMj1kXHErV-tx+8GvvLmb-L0wXq1cVwzY+OEr4PEHDZWrB7Dpg@mail.gmail.com \
--to=ardb@kernel.org \
--cc=fvogt@suse.de \
--cc=jarkko.sakkinen@linux.intel.com \
--cc=jlee@suse.com \
--cc=linux-efi@vger.kernel.org \
--cc=loic.yhuel@gmail.com \
--cc=matthewgarrett@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).