All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-arm-kernel@lists.infradead.org
Cc: linux-efi@vger.kernel.org, will@kernel.org,
	catalin.marinas@arm.com, Ard Biesheuvel <ardb@kernel.org>,
	stable@vger.kernel.org, Peter Jones <pjones@redhat.com>,
	Jarkko Sakkinen <jarkko@kernel.org>,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	Nathan Chancellor <nathan@kernel.org>
Subject: [PATCH] efi: tpm: Avoid READ_ONCE() for accessing the event log
Date: Mon,  9 Jan 2023 10:59:48 +0100	[thread overview]
Message-ID: <20230109095948.2471205-1-ardb@kernel.org> (raw)

Nathan reports that recent kernels built with LTO will crash when doing
EFI boot using Fedora's GRUB and SHIM. The culprit turns out to be a
misaligned load from the TPM event log, which is annotated with
READ_ONCE(), and under LTO, this gets translated into a LDAR instruction
which does not tolerate misaligned accesses.

Interestingly, this does not happen when booting the same kernel
straight from the UEFI shell, and so the fact that the event log may
appear misaligned in memory may be caused by a bug in GRUB or SHIM.

However, using READ_ONCE() to access firmware tables is slightly unusual
in any case, and here, we only need to ensure that 'event' is not
dereferenced again after it gets unmapped, so a compiler barrier should
be sufficient, and works around the reported issue.

Cc: <stable@vger.kernel.org>
Cc: Peter Jones <pjones@redhat.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Reported-by: Nathan Chancellor <nathan@kernel.org>
Link: https://github.com/ClangBuiltLinux/linux/issues/1782
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 include/linux/tpm_eventlog.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 20c0ff54b7a0d313..0abcc85904cba874 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -198,8 +198,10 @@ static __always_inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
 	 * The loop below will unmap these fields if the log is larger than
 	 * one page, so save them here for reference:
 	 */
-	count = READ_ONCE(event->count);
-	event_type = READ_ONCE(event->event_type);
+	count = event->count;
+	event_type = event->event_type;
+
+	barrier();
 
 	/* Verify that it's the log header */
 	if (event_header->pcr_idx != 0 ||
-- 
2.39.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-arm-kernel@lists.infradead.org
Cc: linux-efi@vger.kernel.org, will@kernel.org,
	catalin.marinas@arm.com, Ard Biesheuvel <ardb@kernel.org>,
	stable@vger.kernel.org, Peter Jones <pjones@redhat.com>,
	Jarkko Sakkinen <jarkko@kernel.org>,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	Nathan Chancellor <nathan@kernel.org>
Subject: [PATCH] efi: tpm: Avoid READ_ONCE() for accessing the event log
Date: Mon,  9 Jan 2023 10:59:48 +0100	[thread overview]
Message-ID: <20230109095948.2471205-1-ardb@kernel.org> (raw)

Nathan reports that recent kernels built with LTO will crash when doing
EFI boot using Fedora's GRUB and SHIM. The culprit turns out to be a
misaligned load from the TPM event log, which is annotated with
READ_ONCE(), and under LTO, this gets translated into a LDAR instruction
which does not tolerate misaligned accesses.

Interestingly, this does not happen when booting the same kernel
straight from the UEFI shell, and so the fact that the event log may
appear misaligned in memory may be caused by a bug in GRUB or SHIM.

However, using READ_ONCE() to access firmware tables is slightly unusual
in any case, and here, we only need to ensure that 'event' is not
dereferenced again after it gets unmapped, so a compiler barrier should
be sufficient, and works around the reported issue.

Cc: <stable@vger.kernel.org>
Cc: Peter Jones <pjones@redhat.com>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Reported-by: Nathan Chancellor <nathan@kernel.org>
Link: https://github.com/ClangBuiltLinux/linux/issues/1782
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 include/linux/tpm_eventlog.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 20c0ff54b7a0d313..0abcc85904cba874 100644
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -198,8 +198,10 @@ static __always_inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *ev
 	 * The loop below will unmap these fields if the log is larger than
 	 * one page, so save them here for reference:
 	 */
-	count = READ_ONCE(event->count);
-	event_type = READ_ONCE(event->event_type);
+	count = event->count;
+	event_type = event->event_type;
+
+	barrier();
 
 	/* Verify that it's the log header */
 	if (event_header->pcr_idx != 0 ||
-- 
2.39.0


             reply	other threads:[~2023-01-09 10:01 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-09  9:59 Ard Biesheuvel [this message]
2023-01-09  9:59 ` [PATCH] efi: tpm: Avoid READ_ONCE() for accessing the event log Ard Biesheuvel
2023-01-09 15:10 ` Will Deacon
2023-01-09 15:10   ` Will Deacon
2023-01-09 15:20   ` Ard Biesheuvel
2023-01-09 15:20     ` Ard Biesheuvel
2023-01-09 15:34     ` Will Deacon
2023-01-09 15:34       ` Will Deacon
2023-01-09 15:43       ` Ard Biesheuvel
2023-01-09 15:43         ` Ard Biesheuvel
2023-01-09 17:48 ` Nathan Chancellor
2023-01-09 17:48   ` Nathan Chancellor
2023-01-09 17:50   ` Ard Biesheuvel
2023-01-09 17:50     ` Ard Biesheuvel
2023-01-20 23:22 ` Jarkko Sakkinen
2023-01-20 23:22   ` Jarkko Sakkinen

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=20230109095948.2471205-1-ardb@kernel.org \
    --to=ardb@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=jarkko@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=mjg59@srcf.ucam.org \
    --cc=nathan@kernel.org \
    --cc=pjones@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=will@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.