linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: Tyler Hicks <tyhicks@linux.microsoft.com>
Cc: "Peter Huewe" <peterhuewe@gmx.de>,
	"Jason Gunthorpe" <jgg@ziepe.ca>,
	linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-efi@vger.kernel.org,
	"Kai-Heng Feng" <kai.heng.feng@canonical.com>,
	"Kenneth R . Crudup" <kenny@panix.com>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Thiébaud Weksteen" <tweek@google.com>,
	"Ard Biesheuvel" <ardb@kernel.org>
Subject: Re: [PATCH] tpm: efi: Don't create binary_bios_measurements file for an empty log
Date: Fri, 30 Oct 2020 06:46:26 +0200	[thread overview]
Message-ID: <20201030044626.GA36779@kernel.org> (raw)
In-Reply-To: <20201028154102.9595-1-tyhicks@linux.microsoft.com>

On Wed, Oct 28, 2020 at 10:41:02AM -0500, Tyler Hicks wrote:
> Mimic the pre-existing ACPI and Device Tree event log behavior by not
> creating the binary_bios_measurements file when the EFI TPM event log is
> empty.
> 
> This fixes the following NULL pointer dereference that can occur when
> reading /sys/kernel/security/tpm0/binary_bios_measurements after the
> kernel received an empty event log from the firmware:
> 
>  BUG: kernel NULL pointer dereference, address: 000000000000002c
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 0 P4D 0
>  Oops: 0000 [#1] SMP PTI
>  CPU: 2 PID: 3932 Comm: fwupdtpmevlog Not tainted 5.9.0-00003-g629990edad62 #17
>  Hardware name: LENOVO 20LCS03L00/20LCS03L00, BIOS N27ET38W (1.24 ) 11/28/2019
>  RIP: 0010:tpm2_bios_measurements_start+0x3a/0x550
>  Code: 54 53 48 83 ec 68 48 8b 57 70 48 8b 1e 65 48 8b 04 25 28 00 00 00 48 89 45 d0 31 c0 48 8b 82 c0 06 00 00 48 8b 8a c8 06 00 00 <44> 8b 60 1c 48 89 4d a0 4c 89 e2 49 83 c4 20 48 83 fb 00 75 2a 49
>  RSP: 0018:ffffa9c901203db0 EFLAGS: 00010246
>  RAX: 0000000000000010 RBX: 0000000000000000 RCX: 0000000000000010
>  RDX: ffff8ba1eb99c000 RSI: ffff8ba1e4ce8280 RDI: ffff8ba1e4ce8258
>  RBP: ffffa9c901203e40 R08: ffffa9c901203dd8 R09: ffff8ba1ec443300
>  R10: ffffa9c901203e50 R11: 0000000000000000 R12: ffff8ba1e4ce8280
>  R13: ffffa9c901203ef0 R14: ffffa9c901203ef0 R15: ffff8ba1e4ce8258
>  FS:  00007f6595460880(0000) GS:ffff8ba1ef880000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 000000000000002c CR3: 00000007d8d18003 CR4: 00000000003706e0
>  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  Call Trace:
>   ? __kmalloc_node+0x113/0x320
>   ? kvmalloc_node+0x31/0x80
>   seq_read+0x94/0x420
>   vfs_read+0xa7/0x190
>   ksys_read+0xa7/0xe0
>   __x64_sys_read+0x1a/0x20
>   do_syscall_64+0x37/0x80
>   entry_SYSCALL_64_after_hwframe+0x44/0xa9
> 
> In this situation, the bios_event_log pointer in the tpm_bios_log struct
> was not NULL but was equal to the ZERO_SIZE_PTR (0x10) value. This was
> due to the following kmemdup() in tpm_read_log_efi():
> 
> int tpm_read_log_efi(struct tpm_chip *chip)
> {
> ...
> 	/* malloc EventLog space */
> 	log->bios_event_log = kmemdup(log_tbl->log, log_size, GFP_KERNEL);
> 	if (!log->bios_event_log) {
> 		ret = -ENOMEM;
> 		goto out;
> 	}
> ...
> }
> 
> When log_size is zero, due to an empty event log from firmware,
> ZERO_SIZE_PTR is returned from kmemdup(). Upon a read of the
> binary_bios_measurements file, the tpm2_bios_measurements_start()
> function does not perform a ZERO_OR_NULL_PTR() check on the
> bios_event_log pointer before dereferencing it.
> 
> Rather than add a ZERO_OR_NULL_PTR() check in functions that make use of
> the bios_event_log pointer, simply avoid creating the
> binary_bios_measurements_file as is done in other event log retrieval
> backends.
> 
> Explicitly ignore all of the events in the final event log when the main
> event log is empty. The list of events in the final event log cannot be
> accurately parsed without referring to the first event in the main event
> log (the event log header) so the final event log is useless in such a
> situation.
> 
> Fixes: 58cc1e4faf10 ("tpm: parse TPM event logs based on EFI table")
> Link: https://lore.kernel.org/linux-integrity/E1FDCCCB-CA51-4AEE-AC83-9CDE995EAE52@canonical.com/
> Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Reported-by: Kenneth R. Crudup <kenny@panix.com>
> Reported-by: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Thiébaud Weksteen <tweek@google.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> ---

Applied, thanks.

>  drivers/char/tpm/eventlog/efi.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/char/tpm/eventlog/efi.c b/drivers/char/tpm/eventlog/efi.c
> index 6bb023de17f1..35229e5143ca 100644
> --- a/drivers/char/tpm/eventlog/efi.c
> +++ b/drivers/char/tpm/eventlog/efi.c
> @@ -41,6 +41,11 @@ int tpm_read_log_efi(struct tpm_chip *chip)
>  	log_size = log_tbl->size;
>  	memunmap(log_tbl);
>  
> +	if (!log_size) {
> +		pr_warn("UEFI TPM log area empty\n");
> +		return -EIO;
> +	}
> +
>  	log_tbl = memremap(efi.tpm_log, sizeof(*log_tbl) + log_size,
>  			   MEMREMAP_WB);
>  	if (!log_tbl) {
> 
> base-commit: ed8780e3f2ecc82645342d070c6b4e530532e680
> -- 
> 2.25.1
> 
> 

/Jarkko

      parent reply	other threads:[~2020-10-30  4:46 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-28 12:31 [Regression] "tpm: Require that all digests are present in TCG_PCR_EVENT2 structures" causes null pointer dereference Kai-Heng Feng
2020-09-28 14:06 ` Jarkko Sakkinen
2020-09-28 14:16   ` Kai-Heng Feng
     [not found]     ` <20200928155215.GA92669@linux.intel.com>
2020-09-28 16:15       ` Ard Biesheuvel
2020-09-28 16:26         ` Ard Biesheuvel
2020-09-28 17:12         ` Jarkko Sakkinen
2020-09-28 18:05           ` Kenneth R. Crudup
2020-09-29 17:52     ` Mimi Zohar
2020-09-30  2:20       ` Jarkko Sakkinen
2020-10-08  9:09         ` Kai-Heng Feng
2020-10-09 16:06           ` Jarkko Sakkinen
2020-10-13 14:31             ` Tyler Hicks
2020-10-13 14:33               ` Jarkko Sakkinen
2020-10-20 21:07       ` Mimi Zohar
2020-10-21  5:48         ` Tyler Hicks
2020-10-26  5:49           ` Kai-Heng Feng
2020-10-27  5:08             ` Tyler Hicks
2020-10-28 15:41 ` [PATCH] tpm: efi: Don't create binary_bios_measurements file for an empty log Tyler Hicks
2020-10-28 16:30   ` Tyler Hicks
2020-10-28 17:39     ` Tyler Hicks
2020-10-30  6:41       ` Kai-Heng Feng
2020-11-04  2:12       ` Kenneth R. Crudup
2020-10-30  4:46   ` Jarkko Sakkinen [this message]

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=20201030044626.GA36779@kernel.org \
    --to=jarkko@kernel.org \
    --cc=ardb@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=kai.heng.feng@canonical.com \
    --cc=kenny@panix.com \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterhuewe@gmx.de \
    --cc=tweek@google.com \
    --cc=tyhicks@linux.microsoft.com \
    --cc=zohar@linux.ibm.com \
    --subject='Re: [PATCH] tpm: efi: Don'\''t create binary_bios_measurements file for an empty log' \
    /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

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