linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 5.3 062/112] efivar/ssdt: Dont iterate over EFI vars if no SSDT override was specified
       [not found] <20191016214844.038848564@linuxfoundation.org>
@ 2019-10-16 21:50 ` Greg Kroah-Hartman
  2019-10-16 21:50 ` [PATCH 5.3 063/112] efi/tpm: Dont access event->count when it isnt mapped Greg Kroah-Hartman
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-16 21:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Scott Talbert, Ard Biesheuvel,
	Ben Dooks, Dave Young, Jarkko Sakkinen, Jerry Snitselaar,
	Linus Torvalds, Lukas Wunner, Lyude Paul, Matthew Garrett,
	Octavian Purdila, Peter Jones, Peter Zijlstra, Thomas Gleixner,
	linux-efi, linux-integrity, Ingo Molnar

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

commit c05f8f92b701576b615f30aac31fabdc0648649b upstream.

The kernel command line option efivar_ssdt= allows the name to be
specified of an EFI variable containing an ACPI SSDT table that should
be loaded into memory by the OS, and treated as if it was provided by
the firmware.

Currently, that code will always iterate over the EFI variables and
compare each name with the provided name, even if the command line
option wasn't set to begin with.

So bail early when no variable name was provided. This works around a
boot regression on the 2012 Mac Pro, as reported by Scott.

Tested-by: Scott Talbert <swt@techie.net>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: <stable@vger.kernel.org> # v4.9+
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Dave Young <dyoung@redhat.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Jerry Snitselaar <jsnitsel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Matthew Garrett <mjg59@google.com>
Cc: Octavian Purdila <octavian.purdila@intel.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Fixes: 475fb4e8b2f4 ("efi / ACPI: load SSTDs from EFI variables")
Link: https://lkml.kernel.org/r/20191002165904.8819-3-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/firmware/efi/efi.c |    3 +++
 1 file changed, 3 insertions(+)

--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -282,6 +282,9 @@ static __init int efivar_ssdt_load(void)
 	void *data;
 	int ret;
 
+	if (!efivar_ssdt[0])
+		return 0;
+
 	ret = efivar_init(efivar_ssdt_iter, &entries, true, &entries);
 
 	list_for_each_entry_safe(entry, aux, &entries, list) {



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 5.3 063/112] efi/tpm: Dont access event->count when it isnt mapped
       [not found] <20191016214844.038848564@linuxfoundation.org>
  2019-10-16 21:50 ` [PATCH 5.3 062/112] efivar/ssdt: Dont iterate over EFI vars if no SSDT override was specified Greg Kroah-Hartman
@ 2019-10-16 21:50 ` Greg Kroah-Hartman
  2019-10-16 21:50 ` [PATCH 5.3 064/112] efi/tpm: Dont traverse an event log with no events Greg Kroah-Hartman
  2019-10-16 21:50 ` [PATCH 5.3 065/112] efi/tpm: Only set efi_tpm_final_log_size after successful event log parsing Greg Kroah-Hartman
  3 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-16 21:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Lyude Paul, Peter Jones,
	Jarkko Sakkinen, Ard Biesheuvel, Matthew Garrett, Ben Dooks,
	Dave Young, Jerry Snitselaar, Linus Torvalds, Lukas Wunner,
	Octavian Purdila, Peter Zijlstra, Scott Talbert, Thomas Gleixner,
	linux-efi, linux-integrity, Ingo Molnar

From: Peter Jones <pjones@redhat.com>

commit 047d50aee341d940350897c85799e56ae57c3849 upstream.

Some machines generate a lot of event log entries.  When we're
iterating over them, the code removes the old mapping and adds a
new one, so once we cross the page boundary we're unmapping the page
with the count on it.  Hilarity ensues.

This patch keeps the info from the header in local variables so we don't
need to access that page again or keep track of if it's mapped.

Tested-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Acked-by: Matthew Garrett <mjg59@google.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Dave Young <dyoung@redhat.com>
Cc: Jerry Snitselaar <jsnitsel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Octavian Purdila <octavian.purdila@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Scott Talbert <swt@techie.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: stable@vger.kernel.org
Fixes: 44038bc514a2 ("tpm: Abstract crypto agile event size calculations")
Link: https://lkml.kernel.org/r/20191002165904.8819-4-ard.biesheuvel@linaro.org
[ Minor edits. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 include/linux/tpm_eventlog.h |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -170,6 +170,7 @@ static inline int __calc_tpm2_event_size
 	u16 halg;
 	int i;
 	int j;
+	u32 count, event_type;
 
 	marker = event;
 	marker_start = marker;
@@ -190,16 +191,22 @@ static inline int __calc_tpm2_event_size
 	}
 
 	event = (struct tcg_pcr_event2_head *)mapping;
+	/*
+	 * 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);
 
 	efispecid = (struct tcg_efi_specid_event_head *)event_header->event;
 
 	/* Check if event is malformed. */
-	if (event->count > efispecid->num_algs) {
+	if (count > efispecid->num_algs) {
 		size = 0;
 		goto out;
 	}
 
-	for (i = 0; i < event->count; i++) {
+	for (i = 0; i < count; i++) {
 		halg_size = sizeof(event->digests[i].alg_id);
 
 		/* Map the digest's algorithm identifier */
@@ -256,8 +263,9 @@ static inline int __calc_tpm2_event_size
 		+ event_field->event_size;
 	size = marker - marker_start;
 
-	if ((event->event_type == 0) && (event_field->event_size == 0))
+	if (event_type == 0 && event_field->event_size == 0)
 		size = 0;
+
 out:
 	if (do_mapping)
 		TPM_MEMUNMAP(mapping, mapping_size);



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 5.3 064/112] efi/tpm: Dont traverse an event log with no events
       [not found] <20191016214844.038848564@linuxfoundation.org>
  2019-10-16 21:50 ` [PATCH 5.3 062/112] efivar/ssdt: Dont iterate over EFI vars if no SSDT override was specified Greg Kroah-Hartman
  2019-10-16 21:50 ` [PATCH 5.3 063/112] efi/tpm: Dont access event->count when it isnt mapped Greg Kroah-Hartman
@ 2019-10-16 21:50 ` Greg Kroah-Hartman
  2019-10-16 21:50 ` [PATCH 5.3 065/112] efi/tpm: Only set efi_tpm_final_log_size after successful event log parsing Greg Kroah-Hartman
  3 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-16 21:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Lyude Paul, Peter Jones,
	Jarkko Sakkinen, Ard Biesheuvel, Matthew Garrett, Ben Dooks,
	Dave Young, Jerry Snitselaar, Linus Torvalds, Lukas Wunner,
	Octavian Purdila, Peter Zijlstra, Scott Talbert, Thomas Gleixner,
	linux-efi, linux-integrity, Ingo Molnar

From: Peter Jones <pjones@redhat.com>

commit 05c8c1ff81ed2eb9bad7c27cf92e55c864c16df8 upstream.

When there are no entries to put into the final event log, some machines
will return the template they would have populated anyway.  In this case
the nr_events field is 0, but the rest of the log is just garbage.

This patch stops us from trying to iterate the table with
__calc_tpm2_event_size() when the number of events in the table is 0.

Tested-by: Lyude Paul <lyude@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Acked-by: Matthew Garrett <mjg59@google.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Dave Young <dyoung@redhat.com>
Cc: Jerry Snitselaar <jsnitsel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Octavian Purdila <octavian.purdila@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Scott Talbert <swt@techie.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: stable@vger.kernel.org
Fixes: c46f3405692d ("tpm: Reserve the TPM final events table")
Link: https://lkml.kernel.org/r/20191002165904.8819-5-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/firmware/efi/tpm.c |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -75,11 +75,16 @@ int __init efi_tpm_eventlog_init(void)
 		goto out;
 	}
 
-	tbl_size = tpm2_calc_event_log_size((void *)efi.tpm_final_log
-					    + sizeof(final_tbl->version)
-					    + sizeof(final_tbl->nr_events),
-					    final_tbl->nr_events,
-					    log_tbl->log);
+	tbl_size = 0;
+	if (final_tbl->nr_events != 0) {
+		void *events = (void *)efi.tpm_final_log
+				+ sizeof(final_tbl->version)
+				+ sizeof(final_tbl->nr_events);
+
+		tbl_size = tpm2_calc_event_log_size(events,
+						    final_tbl->nr_events,
+						    log_tbl->log);
+	}
 	memblock_reserve((unsigned long)final_tbl,
 			 tbl_size + sizeof(*final_tbl));
 	early_memunmap(final_tbl, sizeof(*final_tbl));



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 5.3 065/112] efi/tpm: Only set efi_tpm_final_log_size after successful event log parsing
       [not found] <20191016214844.038848564@linuxfoundation.org>
                   ` (2 preceding siblings ...)
  2019-10-16 21:50 ` [PATCH 5.3 064/112] efi/tpm: Dont traverse an event log with no events Greg Kroah-Hartman
@ 2019-10-16 21:50 ` Greg Kroah-Hartman
  3 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-16 21:50 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, stable, Jerry Snitselaar, Ard Biesheuvel,
	Ben Dooks, Dave Young, Jarkko Sakkinen, Linus Torvalds,
	Lukas Wunner, Lyude Paul, Matthew Garrett, Octavian Purdila,
	Peter Jones, Peter Zijlstra, Scott Talbert, Thomas Gleixner,
	linux-efi, linux-integrity, Ingo Molnar

From: Jerry Snitselaar <jsnitsel@redhat.com>

commit e658c82be5561412c5e83b5e74e9da4830593f3e upstream.

If __calc_tpm2_event_size() fails to parse an event it will return 0,
resulting tpm2_calc_event_log_size() returning -1. Currently there is
no check of this return value, and 'efi_tpm_final_log_size' can end up
being set to this negative value resulting in a crash like this one:

  BUG: unable to handle page fault for address: ffffbc8fc00866ad
  #PF: supervisor read access in kernel mode
  #PF: error_code(0x0000) - not-present page

  RIP: 0010:memcpy_erms+0x6/0x10
  Call Trace:
   tpm_read_log_efi()
   tpm_bios_log_setup()
   tpm_chip_register()
   tpm_tis_core_init.cold.9+0x28c/0x466
   tpm_tis_plat_probe()
   platform_drv_probe()
   ...

Also __calc_tpm2_event_size() returns a size of 0 when it fails
to parse an event, so update function documentation to reflect this.

The root cause of the issue that caused the failure of event parsing
in this case is resolved by Peter Jone's patchset dealing with large
event logs where crossing over a page boundary causes the page with
the event count to be unmapped.

Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ben Dooks <ben.dooks@codethink.co.uk>
Cc: Dave Young <dyoung@redhat.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Matthew Garrett <mjg59@google.com>
Cc: Octavian Purdila <octavian.purdila@intel.com>
Cc: Peter Jones <pjones@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Scott Talbert <swt@techie.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: stable@vger.kernel.org
Fixes: c46f3405692de ("tpm: Reserve the TPM final events table")
Link: https://lkml.kernel.org/r/20191002165904.8819-6-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/firmware/efi/tpm.c   |    9 ++++++++-
 include/linux/tpm_eventlog.h |    2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

--- a/drivers/firmware/efi/tpm.c
+++ b/drivers/firmware/efi/tpm.c
@@ -85,11 +85,18 @@ int __init efi_tpm_eventlog_init(void)
 						    final_tbl->nr_events,
 						    log_tbl->log);
 	}
+
+	if (tbl_size < 0) {
+		pr_err(FW_BUG "Failed to parse event in TPM Final Events Log\n");
+		goto out_calc;
+	}
+
 	memblock_reserve((unsigned long)final_tbl,
 			 tbl_size + sizeof(*final_tbl));
-	early_memunmap(final_tbl, sizeof(*final_tbl));
 	efi_tpm_final_log_size = tbl_size;
 
+out_calc:
+	early_memunmap(final_tbl, sizeof(*final_tbl));
 out:
 	early_memunmap(log_tbl, sizeof(*log_tbl));
 	return ret;
--- a/include/linux/tpm_eventlog.h
+++ b/include/linux/tpm_eventlog.h
@@ -152,7 +152,7 @@ struct tcg_algorithm_info {
  * total. Once we've done this we know the offset of the data length field,
  * and can calculate the total size of the event.
  *
- * Return: size of the event on success, <0 on failure
+ * Return: size of the event on success, 0 on failure
  */
 
 static inline int __calc_tpm2_event_size(struct tcg_pcr_event2_head *event,



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-10-16 22:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20191016214844.038848564@linuxfoundation.org>
2019-10-16 21:50 ` [PATCH 5.3 062/112] efivar/ssdt: Dont iterate over EFI vars if no SSDT override was specified Greg Kroah-Hartman
2019-10-16 21:50 ` [PATCH 5.3 063/112] efi/tpm: Dont access event->count when it isnt mapped Greg Kroah-Hartman
2019-10-16 21:50 ` [PATCH 5.3 064/112] efi/tpm: Dont traverse an event log with no events Greg Kroah-Hartman
2019-10-16 21:50 ` [PATCH 5.3 065/112] efi/tpm: Only set efi_tpm_final_log_size after successful event log parsing Greg Kroah-Hartman

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