TPM/Verifiers testing bug?

TPM/Verifiers testing bug?

[Max Tottenham]

Hi Folks

I was curious about the upstream work on the verifiers framework (and
the TPM patches). I have both a TPM 2.0 based system and a QEMU + swtpm
setup with which to test. I compiled the head of the master branch, if I
boot into the grub shell and run the following commands:

grub> insmod verifiers
grub> insmod tpm
grub> normal

I get a machine crash:

qemu-system-x86_64: Trying to execute code outside RAM or ROM at
0x00000000000b0000
This usually means one of the following happened:

(1) You told QEMU to execute a kernel for the wrong machine type, and it
crashed on startup (eg trying to run a raspberry pi kernel on a
versatilepb QEMU machine)
(2) You didn't give QEMU a kernel or BIOS filename at all, and QEMU
executed a ROM full of no-op instructions until it fell off the end
(3) Your guest kernel has a bug and crashed by jumping off into nowhere

This is almost always one of the first two, so check your command line
and that you are using the right type of kernel for this machine.
If you think option (3) is likely then you can try debugging your guest
with the -d debug options; in particular -d guest_errors will cause the
log to include a dump of the guest register state at this point.

Execution cannot continue; stopping here.


Software versions:
Qemu version:  v2.11.0           (0a0dc59)
OVMF git tag:  edk2-stable201811 (8558838)
swtpm git tag: tpm2-preview.v2   (f98592c)


Running the same on real hardware also produces a crash, any thoughts?


-- 
Max Tottenham       | mtottenh@akamai.com
Senior Software Engineer, Server Platform Engineering
/(* Akamai Technologies

Re: TPM/Verifiers testing bug?

[Daniel Kiper]

On Wed, Jan 09, 2019 at 02:16:16PM +0000, Max Tottenham wrote:
> Hi Folks
>
> I was curious about the upstream work on the verifiers framework (and
> the TPM patches). I have both a TPM 2.0 based system and a QEMU + swtpm
> setup with which to test. I compiled the head of the master branch, if I
> boot into the grub shell and run the following commands:
>
> grub> insmod verifiers
> grub> insmod tpm
> grub> normal
>
> I get a machine crash:
>
> qemu-system-x86_64: Trying to execute code outside RAM or ROM at
> 0x00000000000b0000
> This usually means one of the following happened:
>
> (1) You told QEMU to execute a kernel for the wrong machine type, and it
> crashed on startup (eg trying to run a raspberry pi kernel on a
> versatilepb QEMU machine)
> (2) You didn't give QEMU a kernel or BIOS filename at all, and QEMU
> executed a ROM full of no-op instructions until it fell off the end
> (3) Your guest kernel has a bug and crashed by jumping off into nowhere
>
> This is almost always one of the first two, so check your command line
> and that you are using the right type of kernel for this machine.
> If you think option (3) is likely then you can try debugging your guest
> with the -d debug options; in particular -d guest_errors will cause the
> log to include a dump of the guest register state at this point.
>
> Execution cannot continue; stopping here.
>
>
> Software versions:
> Qemu version:  v2.11.0           (0a0dc59)
> OVMF git tag:  edk2-stable201811 (8558838)
> swtpm git tag: tpm2-preview.v2   (f98592c)
>
>
> Running the same on real hardware also produces a crash, any thoughts?

Adding Matthew who is the author of the patch series.

Daniel

Re: TPM/Verifiers testing bug?

[Max Tottenham]

On 01/14, Daniel Kiper wrote:
> On Wed, Jan 09, 2019 at 02:16:16PM +0000, Max Tottenham wrote:
> > Hi Folks
> >
> > I was curious about the upstream work on the verifiers framework (and
> > the TPM patches). I have both a TPM 2.0 based system and a QEMU + swtpm
> > setup with which to test. I compiled the head of the master branch, if I
> > boot into the grub shell and run the following commands:
> >
> > grub> insmod verifiers
> > grub> insmod tpm
> > grub> normal
> >
> > I get a machine crash:
> >
> > qemu-system-x86_64: Trying to execute code outside RAM or ROM at
> > 0x00000000000b0000
> > This usually means one of the following happened:
> >
> > (1) You told QEMU to execute a kernel for the wrong machine type, and it
> > crashed on startup (eg trying to run a raspberry pi kernel on a
> > versatilepb QEMU machine)
> > (2) You didn't give QEMU a kernel or BIOS filename at all, and QEMU
> > executed a ROM full of no-op instructions until it fell off the end
> > (3) Your guest kernel has a bug and crashed by jumping off into nowhere
> >
> > This is almost always one of the first two, so check your command line
> > and that you are using the right type of kernel for this machine.
> > If you think option (3) is likely then you can try debugging your guest
> > with the -d debug options; in particular -d guest_errors will cause the
> > log to include a dump of the guest register state at this point.
> >
> > Execution cannot continue; stopping here.
> >
> >
> > Software versions:
> > Qemu version:  v2.11.0           (0a0dc59)
> > OVMF git tag:  edk2-stable201811 (8558838)
> > swtpm git tag: tpm2-preview.v2   (f98592c)
> >
> >
> > Running the same on real hardware also produces a crash, any thoughts?
> 
> Adding Matthew who is the author of the patch series.
> 
> Daniel

I went ahead and did some debugging. Below is a patch that seems to fix
my problem. Although those calls to grub_efi_open_protocol() in the tpm
module should probably check their return value and do something sane if
0x0 is returned.

-- 
Max Tottenham       | mtottenh@akamai.com
Senior Software Engineer, Server Platform Engineering
/(* Akamai Technologies

From 023be8fe8a4f77dbcbf94015bef81385a7681679 Mon Sep 17 00:00:00 2001
From: Max Tottenham <mtottenh@akamai.com>
Date: Mon, 14 Jan 2019 14:03:29 +0000
Subject: [PATCH] Fix bug in GRUB2 TPM module

  The value of tpm_handle changes between successive calls to
  tpm_handle_find(), as instead of simply copying the stored pointer we end up
  taking the address of said pointer when using the cached value of grub_tpm_handle.

  This causes grub_efi_open_protocol() to return a nullptr in
  grub_tpm2_execute() and grub_tpm2_log_event(). Said nullptr goes unchecked
  and efi_call_5(tpm->hash_log_extend_event,...) ends up jumping to 0x0, Qemu
  crashes once video ROM is reached at 0xb0000.

  This patch seems to do the trick of fixing that bug, but we should
  also ensure that all calls to grub_efi_open_protocol() are checked so
  that we don't start executing low memory.
---
 grub-core/commands/efi/tpm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grub-core/commands/efi/tpm.c b/grub-core/commands/efi/tpm.c
index 563ceba7a..7475fd87b 100644
--- a/grub-core/commands/efi/tpm.c
+++ b/grub-core/commands/efi/tpm.c
@@ -89,7 +89,7 @@ grub_tpm_handle_find (grub_efi_handle_t *tpm_handle,
 
   if (grub_tpm_handle != NULL)
     {
-      *tpm_handle = &grub_tpm_handle;
+      *tpm_handle = grub_tpm_handle;
       *protocol_version = grub_tpm_version;
       return 1;
     }
-- 
2.17.0

Re: TPM/Verifiers testing bug?

[Matthew Garrett]

On Mon, Jan 14, 2019 at 6:09 AM 'Max Tottenham' via mjg59
<mjg59@google.com> wrote:

> I went ahead and did some debugging. Below is a patch that seems to fix
> my problem. Although those calls to grub_efi_open_protocol() in the tpm
> module should probably check their return value and do something sane if
> 0x0 is returned.

LGTM.

Re: TPM/Verifiers testing bug?

[Daniel Kiper]

On Mon, Jan 14, 2019 at 11:42:21AM -0800, Matthew Garrett wrote:
> On Mon, Jan 14, 2019 at 6:09 AM 'Max Tottenham' via mjg59
> <mjg59@google.com> wrote:
>
> > I went ahead and did some debugging. Below is a patch that seems to fix
> > my problem. Although those calls to grub_efi_open_protocol() in the tpm
> > module should probably check their return value and do something sane if
> > 0x0 is returned.
>
> LGTM.

May I treat this as Reviewed-by: Matthew Garrett <mjg59@google.com>?

Daniel

Re: TPM/Verifiers testing bug?

[Matthew Garrett]

On Tue, Jan 15, 2019 at 3:58 AM Daniel Kiper <dkiper@net-space.pl> wrote:
>
> On Mon, Jan 14, 2019 at 11:42:21AM -0800, Matthew Garrett wrote:
> > On Mon, Jan 14, 2019 at 6:09 AM 'Max Tottenham' via mjg59
> > <mjg59@google.com> wrote:
> >
> > > I went ahead and did some debugging. Below is a patch that seems to fix
> > > my problem. Although those calls to grub_efi_open_protocol() in the tpm
> > > module should probably check their return value and do something sane if
> > > 0x0 is returned.
> >
> > LGTM.
>
> May I treat this as Reviewed-by: Matthew Garrett <mjg59@google.com>?

Sorry, yes.

Re: TPM/Verifiers testing bug?

[Daniel Kiper]

On Mon, Jan 14, 2019 at 02:09:45PM +0000, Max Tottenham wrote:
> On 01/14, Daniel Kiper wrote:
> > On Wed, Jan 09, 2019 at 02:16:16PM +0000, Max Tottenham wrote:
> > > Hi Folks
> > >
> > > I was curious about the upstream work on the verifiers framework (and
> > > the TPM patches). I have both a TPM 2.0 based system and a QEMU + swtpm
> > > setup with which to test. I compiled the head of the master branch, if I
> > > boot into the grub shell and run the following commands:
> > >
> > > grub> insmod verifiers
> > > grub> insmod tpm
> > > grub> normal
> > >
> > > I get a machine crash:
> > >
> > > qemu-system-x86_64: Trying to execute code outside RAM or ROM at
> > > 0x00000000000b0000
> > > This usually means one of the following happened:
> > >
> > > (1) You told QEMU to execute a kernel for the wrong machine type, and it
> > > crashed on startup (eg trying to run a raspberry pi kernel on a
> > > versatilepb QEMU machine)
> > > (2) You didn't give QEMU a kernel or BIOS filename at all, and QEMU
> > > executed a ROM full of no-op instructions until it fell off the end
> > > (3) Your guest kernel has a bug and crashed by jumping off into nowhere
> > >
> > > This is almost always one of the first two, so check your command line
> > > and that you are using the right type of kernel for this machine.
> > > If you think option (3) is likely then you can try debugging your guest
> > > with the -d debug options; in particular -d guest_errors will cause the
> > > log to include a dump of the guest register state at this point.
> > >
> > > Execution cannot continue; stopping here.
> > >
> > >
> > > Software versions:
> > > Qemu version:  v2.11.0           (0a0dc59)
> > > OVMF git tag:  edk2-stable201811 (8558838)
> > > swtpm git tag: tpm2-preview.v2   (f98592c)
> > >
> > >
> > > Running the same on real hardware also produces a crash, any thoughts?
> >
> > Adding Matthew who is the author of the patch series.
> >
> > Daniel
>
> I went ahead and did some debugging. Below is a patch that seems to fix
> my problem. Although those calls to grub_efi_open_protocol() in the tpm
> module should probably check their return value and do something sane if
> 0x0 is returned.
>
> --
> Max Tottenham       | mtottenh@akamai.com
> Senior Software Engineer, Server Platform Engineering
> /(* Akamai Technologies
>
> >From 023be8fe8a4f77dbcbf94015bef81385a7681679 Mon Sep 17 00:00:00 2001
> From: Max Tottenham <mtottenh@akamai.com>
> Date: Mon, 14 Jan 2019 14:03:29 +0000
> Subject: [PATCH] Fix bug in GRUB2 TPM module
>
>   The value of tpm_handle changes between successive calls to
>   tpm_handle_find(), as instead of simply copying the stored pointer we end up
>   taking the address of said pointer when using the cached value of grub_tpm_handle.
>
>   This causes grub_efi_open_protocol() to return a nullptr in
>   grub_tpm2_execute() and grub_tpm2_log_event(). Said nullptr goes unchecked
>   and efi_call_5(tpm->hash_log_extend_event,...) ends up jumping to 0x0, Qemu
>   crashes once video ROM is reached at 0xb0000.
>
>   This patch seems to do the trick of fixing that bug, but we should
>   also ensure that all calls to grub_efi_open_protocol() are checked so
>   that we don't start executing low memory.

I have added your SOB and pushed. I hope that you do not mind.

Daniel