Re: [PATCH 3/3] Core TPM support

[Javier Martinez Canillas]

Hello Matthew,

On 06/16/2017 03:51 PM, Javier Martinez Canillas wrote:
> 
> I've tested your patches (plus the verify framework) today on a Lenovo
> Thinkpad X1 Carbon with 2 different setups:
> 

I've tested today building your patches --with-platform=pc instead of efi,
and found a build error due the grub_tpm_init() function being defined twice.
I'm using the following patch [0] on top of yours to avoid this issue.

But I noticed that you have a new verifier_tpm_module branch [1], that instead
of having the TPM support in-kernel, has it as an external module. And is only
enabled for efi, so this problem won't happen on that new branch.

You should remove though the grub_tpm_init() prototype, stub and in fact all
the GRUB_MACHINE_EFI #ifdefery in that new branch since it's unused now IIUC.

[0]:
>From 8e8ae3d0fb3b9b5a914359b48bb9e893b6113aa4 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <address@hidden>
Date: Wed, 21 Jun 2017 12:29:38 +0200
Subject: [PATCH 1/1] Only build TPM support for x86 EFI platforms
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There's only TPM support for x86 EFI platforms for now. So instead of
unconditionally add the TPM headers and source files, add it on a per
platform basis if TPM is supported. This also prevents the following
compile error when building a x86 pc platform:

In file included from kern/tpm.c:5:0:
../include/grub/tpm.h: In function ‘grub_tpm_init’:
../include/grub/tpm.h:76:1: warning: no return statement in function returning 
non-void [-Wreturn-type]
 static inline grub_err_t grub_tpm_init(void) {};
 ^~~~~~
kern/tpm.c: At top level:
kern/tpm.c:77:1: error: redefinition of ‘grub_tpm_init’
 grub_tpm_init (void)
 ^~~~~~~~~~~~~
In file included from kern/tpm.c:5:0:
../include/grub/tpm.h:76:26: note: previous definition of ‘grub_tpm_init’ was 
here
 static inline grub_err_t grub_tpm_init(void) {};

Signed-off-by: Javier Martinez Canillas <address@hidden>
---
 grub-core/Makefile.am       |  3 ++-
 grub-core/Makefile.core.def |  2 +-
 include/grub/tpm.h          | 20 --------------------
 3 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
index cc641bc36a0f..36f006f135a8 100644
--- a/grub-core/Makefile.am
+++ b/grub-core/Makefile.am
@@ -92,7 +92,6 @@ KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/term.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/time.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/mm_private.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/net.h
-KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/tpm.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/memory.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/verify.h
 
@@ -110,6 +109,7 @@ KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/acpi.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/pci.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/pmtimer.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/tpm.h
 endif
 
 if COND_i386_coreboot
@@ -166,6 +166,7 @@ KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/pci.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/acpi.h
 KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/pmtimer.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/tpm.h
 endif
 
 if COND_ia64_efi
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 2bdb6cafde2d..bfbe7c97a6cb 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -129,7 +129,6 @@ kernel = {
   common = kern/rescue_parser.c;
   common = kern/rescue_reader.c;
   common = kern/term.c;
-  common = kern/tpm.c;
   common = kern/verifiers.c;
 
   noemu = kern/compiler-rt.c;
@@ -194,6 +193,7 @@ kernel = {
   efi = kern/acpi.c;
   efi = kern/efi/acpi.c;
   efi = kern/efi/tpm.c;
+  efi = kern/tpm.c;
   i386_coreboot = kern/i386/pc/acpi.c;
   i386_multiboot = kern/i386/pc/acpi.c;
   i386_coreboot = kern/acpi.c;
diff --git a/include/grub/tpm.h b/include/grub/tpm.h
index dd905b8299d9..320b6d0a5897 100644
--- a/include/grub/tpm.h
+++ b/include/grub/tpm.h
@@ -66,29 +66,9 @@ typedef struct {
 grub_err_t EXPORT_FUNC(grub_tpm_measure) (unsigned char *buf, grub_size_t size,
                                          grub_uint8_t pcr,
                                          const char *description);
-#if defined (GRUB_MACHINE_EFI)
 grub_err_t grub_tpm_init(void);
 grub_err_t grub_tpm_execute(PassThroughToTPM_InputParamBlock *inbuf,
                            PassThroughToTPM_OutputParamBlock *outbuf);
 grub_err_t grub_tpm_log_event(unsigned char *buf, grub_size_t size,
                              grub_uint8_t pcr, const char *description);
-#else
-static inline grub_err_t grub_tpm_init(void) {};
-
-static inline grub_err_t grub_tpm_execute(
-       PassThroughToTPM_InputParamBlock *inbuf __attribute__ ((unused)),
-       PassThroughToTPM_OutputParamBlock *outbuf __attribute__ ((unused)))
-{
-       return 0;
-};
-static inline grub_err_t grub_tpm_log_event(
-       unsigned char *buf __attribute__ ((unused)),
-       grub_size_t size __attribute__ ((unused)),
-       grub_uint8_t pcr __attribute__ ((unused)),
-       const char *description __attribute__ ((unused)))
-{
-       return 0;
-};
-#endif
-
 #endif
-- 
2.13.0

[1]: https://github.com/mjg59/grub/commits/verifier_tpm_module

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat