All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: linux-efi@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	linux-kernel@vger.kernel.org,
	Ben Dooks <ben.dooks@codethink.co.uk>,
	Dave Young <dyoung@redhat.com>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Jerry Snitselaar <jsnitsel@redhat.com>,
	linux-integrity@vger.kernel.org, Lukas Wunner <lukas@wunner.de>,
	Lyude Paul <lyude@redhat.com>, Matthew Garrett <mjg59@google.com>,
	Octavian Purdila <octavian.purdila@intel.com>,
	Peter Jones <pjones@redhat.com>, Scott Talbert <swt@techie.net>
Subject: [PATCH 5/7] efi/tpm: only set efi_tpm_final_log_size after successful event log parsing
Date: Wed,  2 Oct 2019 18:59:02 +0200	[thread overview]
Message-ID: <20191002165904.8819-6-ard.biesheuvel@linaro.org> (raw)
In-Reply-To: <20191002165904.8819-1-ard.biesheuvel@linaro.org>

From: Jerry Snitselaar <jsnitsel@redhat.com>

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 panic like the
the one given below.

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

[    0.774340] BUG: unable to handle page fault for address: ffffbc8fc00866ad
[    0.774788] #PF: supervisor read access in kernel mode
[    0.774788] #PF: error_code(0x0000) - not-present page
[    0.774788] PGD 107d36067 P4D 107d36067 PUD 107d37067 PMD 107d38067 PTE 0
[    0.774788] Oops: 0000 [#1] SMP PTI
[    0.774788] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.3.0-0.rc2.1.elrdy.x86_64 #1
[    0.774788] Hardware name: LENOVO 20HGS22D0W/20HGS22D0W, BIOS N1WET51W (1.30 ) 09/14/2018
[    0.774788] RIP: 0010:memcpy_erms+0x6/0x10
[    0.774788] Code: 90 90 90 90 eb 1e 0f 1f 00 48 89 f8 48 89 d1 48 c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1 <f3> a4 c3 0f 1f 80 00 00 00 00 48 89 f8 48 83 fa 20 72 7e 40 38 fe
[    0.774788] RSP: 0000:ffffbc8fc0073b30 EFLAGS: 00010286
[    0.774788] RAX: ffff9b1fc7c5b367 RBX: ffff9b1fc8390000 RCX: ffffffffffffe962
[    0.774788] RDX: ffffffffffffe962 RSI: ffffbc8fc00866ad RDI: ffff9b1fc7c5b367
[    0.774788] RBP: ffff9b1c10ca7018 R08: ffffbc8fc0085fff R09: 8000000000000063
[    0.774788] R10: 0000000000001000 R11: 000fffffffe00000 R12: 0000000000003367
[    0.774788] R13: ffff9b1fcc47c010 R14: ffffbc8fc0085000 R15: 0000000000000002
[    0.774788] FS:  0000000000000000(0000) GS:ffff9b1fce200000(0000) knlGS:0000000000000000
[    0.774788] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.774788] CR2: ffffbc8fc00866ad CR3: 000000029f60a001 CR4: 00000000003606f0
[    0.774788] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    0.774788] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    0.774788] Call Trace:
[    0.774788]  tpm_read_log_efi+0x156/0x1a0
[    0.774788]  tpm_bios_log_setup+0xc8/0x190
[    0.774788]  tpm_chip_register+0x50/0x1c0
[    0.774788]  tpm_tis_core_init.cold.9+0x28c/0x466
[    0.774788]  tpm_tis_plat_probe+0xcc/0xea
[    0.774788]  platform_drv_probe+0x35/0x80
[    0.774788]  really_probe+0xef/0x390
[    0.774788]  driver_probe_device+0xb4/0x100
[    0.774788]  device_driver_attach+0x4f/0x60
[    0.774788]  __driver_attach+0x86/0x140
[    0.774788]  ? device_driver_attach+0x60/0x60
[    0.774788]  bus_for_each_dev+0x76/0xc0
[    0.774788]  ? klist_add_tail+0x3b/0x70
[    0.774788]  bus_add_driver+0x14a/0x1e0
[    0.774788]  ? tpm_init+0xea/0xea
[    0.774788]  ? do_early_param+0x8e/0x8e
[    0.774788]  driver_register+0x6b/0xb0
[    0.774788]  ? tpm_init+0xea/0xea
[    0.774788]  init_tis+0x86/0xd8
[    0.774788]  ? do_early_param+0x8e/0x8e
[    0.774788]  ? driver_register+0x94/0xb0
[    0.774788]  do_one_initcall+0x46/0x1e4
[    0.774788]  ? do_early_param+0x8e/0x8e
[    0.774788]  kernel_init_freeable+0x199/0x242
[    0.774788]  ? rest_init+0xaa/0xaa
[    0.774788]  kernel_init+0xa/0x106
[    0.774788]  ret_from_fork+0x35/0x40
[    0.774788] Modules linked in:
[    0.774788] CR2: ffffbc8fc00866ad
[    0.774788] ---[ end trace 42930799f8d6eaea ]---
[    0.774788] RIP: 0010:memcpy_erms+0x6/0x10
[    0.774788] Code: 90 90 90 90 eb 1e 0f 1f 00 48 89 f8 48 89 d1 48 c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1 <f3> a4 c3 0f 1f 80 00 00 00 00 48 89 f8 48 83 fa 20 72 7e 40 38 fe
[    0.774788] RSP: 0000:ffffbc8fc0073b30 EFLAGS: 00010286
[    0.774788] RAX: ffff9b1fc7c5b367 RBX: ffff9b1fc8390000 RCX: ffffffffffffe962
[    0.774788] RDX: ffffffffffffe962 RSI: ffffbc8fc00866ad RDI: ffff9b1fc7c5b367
[    0.774788] RBP: ffff9b1c10ca7018 R08: ffffbc8fc0085fff R09: 8000000000000063
[    0.774788] R10: 0000000000001000 R11: 000fffffffe00000 R12: 0000000000003367
[    0.774788] R13: ffff9b1fcc47c010 R14: ffffbc8fc0085000 R15: 0000000000000002
[    0.774788] FS:  0000000000000000(0000) GS:ffff9b1fce200000(0000) knlGS:0000000000000000
[    0.774788] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.774788] CR2: ffffbc8fc00866ad CR3: 000000029f60a001 CR4: 00000000003606f0
[    0.774788] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    0.774788] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    0.774788] Kernel panic - not syncing: Fatal exception
[    0.774788] Kernel Offset: 0x1d000000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[    0.774788] ---[ end Kernel panic - not syncing: Fatal exception ]---

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.

Fixes: c46f3405692de ("tpm: Reserve the TPM final events table")
Cc: linux-efi@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: stable@vger.kernel.org
Cc: Matthew Garrett <mjg59@google.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/tpm.c   | 9 ++++++++-
 include/linux/tpm_eventlog.h | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c
index b9ae5c6f9b9c..703469c1ab8e 100644
--- 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;
diff --git a/include/linux/tpm_eventlog.h b/include/linux/tpm_eventlog.h
index 12584b69a3f3..2dfdd63ac034 100644
--- 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,
-- 
2.20.1


  parent reply	other threads:[~2019-10-02 17:04 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02 16:58 [GIT PULL 0/7] EFI fixes for v5.4 Ard Biesheuvel
2019-10-02 16:58 ` [PATCH 1/7] efi: cper: Fix endianness of PCIe class code Ard Biesheuvel
2019-10-07 14:49   ` [tip: efi/urgent] efi/cper: " tip-bot2 for Lukas Wunner
2019-10-02 16:58 ` [PATCH 2/7] efivar/ssdt: don't iterate over EFI vars if no SSDT override was specified Ard Biesheuvel
2019-10-07 14:49   ` [tip: efi/urgent] efivar/ssdt: Don't " tip-bot2 for Ard Biesheuvel
2019-10-02 16:59 ` [PATCH 3/7] efi/tpm: Don't access event->count when it isn't mapped Ard Biesheuvel
2019-10-07 14:49   ` [tip: efi/urgent] " tip-bot2 for Peter Jones
2019-10-02 16:59 ` [PATCH 4/7] efi/tpm: don't traverse an event log with no events Ard Biesheuvel
2019-10-07 14:49   ` [tip: efi/urgent] efi/tpm: Don't " tip-bot2 for Peter Jones
2019-10-02 16:59 ` Ard Biesheuvel [this message]
2019-10-07 14:49   ` [tip: efi/urgent] efi/tpm: Only set 'efi_tpm_final_log_size' after successful event log parsing tip-bot2 for Jerry Snitselaar
2019-10-02 16:59 ` [PATCH 6/7] efi: make unexported efi_rci2_sysfs_init static Ard Biesheuvel
2019-10-07 14:49   ` [tip: efi/urgent] efi: Make unexported efi_rci2_sysfs_init() static tip-bot2 for Ben Dooks
2019-10-02 16:59 ` [PATCH 7/7] efi/x86: do not clean dummy variable in kexec path Ard Biesheuvel
2019-10-07 14:49   ` [tip: efi/urgent] efi/x86: Do " tip-bot2 for Dave Young

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=20191002165904.8819-6-ard.biesheuvel@linaro.org \
    --to=ard.biesheuvel@linaro.org \
    --cc=ben.dooks@codethink.co.uk \
    --cc=dyoung@redhat.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jsnitsel@redhat.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=lyude@redhat.com \
    --cc=mingo@kernel.org \
    --cc=mjg59@google.com \
    --cc=octavian.purdila@intel.com \
    --cc=pjones@redhat.com \
    --cc=swt@techie.net \
    --cc=tglx@linutronix.de \
    /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.